Re: [edk2-devel] GitHub PR Code Review process now active

2024-06-03 Thread Neal Gompa
Hmm, I don't see a setting for it anymore, maybe that's not a thing anymore?

I seemingly recall that draft PRs didn't get CI runs, but if that's
not a thing anymore, then that's fine.

That said, draft PRs cannot be reviewed, so we should not be telling
people to make draft PRs.


On Mon, Jun 3, 2024 at 12:26 PM Michael D Kinney via groups.io

 wrote:
>
> CI jobs are dispatched to both GitHub Actions and Azure Pipelines.
>
> For Draft PRs, I see both GitHub Actions and Azure Pipelines jobs running.
>
> This must imply that edk2 repo allows this.  Do you happen to know where
> this is configurable or a link to GitHub docs for configuration?
>
> Mike
>
> > -Original Message-
> > From: Neal Gompa 
> > Sent: Monday, June 3, 2024 9:13 AM
> > To: devel@edk2.groups.io; Kinney, Michael D 
> > Subject: Re: [edk2-devel] GitHub PR Code Review process now active
> >
> > On Tue, May 28, 2024 at 2:53 PM Michael D Kinney via groups.io
> >  wrote:
> > >
> > > Hello,
> > >
> > > The GitHub PR code review process is now active.  Please
> > > use the new PR based code review process for all new
> > > submissions starting today.
> > >
> > > * The Wiki has been updated with the process changes.
> > >
> > >   
> > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-
> > Process
> > >
> > >   Big thanks to Michael Kubacki for writing up all the
> > >   changes based on the RFC proposal and community discussions.
> > >
> > >   We will learn by using, so if you see anything missing or
> > >   incorrect or clarifications needed, please send feedback
> > >   here so the Wiki pages can be updated quickly for everyone.
> > >
> > > * The edk2 repo settings have been updated to require
> > >   a GitHub PR code review approval before merging and
> > >   all conversations must be resolved before merging.
> > >
> > > * A PR has been opened that removes the requirement for
> > >   Cc: tags in the commit messages and is the first PR
> > >   that will use the new process. This PR needs to be
> > >   reviewed and merged to support the revised commit
> > >   message format.
> > >
> > >   https://github.com/tianocore/edk2/pull/5688
> > >
> > >   https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
> > Format
> > >
> > > * Please use "Draft" PRs to run CI without any reviews.
> > >   Once ready for reviews, convert from "Draft" to
> > >   "Ready for Review".
> > >
> >
> > Generally GitHub doesn't allow CI to run on PRs created as draft pull
> > requests. Was this changed for edk2?
> >
> >
> > --
> > 真実はいつも一つ!/ Always, there's only one truth!
>
>
> 
>
>


--
真実はいつも一つ!/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119433): https://edk2.groups.io/g/devel/message/119433
Mute This Topic: https://groups.io/mt/106355103/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] GitHub PR Code Review process now active

2024-06-03 Thread Neal Gompa
On Tue, May 28, 2024 at 2:53 PM Michael D Kinney via groups.io
 wrote:
>
> Hello,
>
> The GitHub PR code review process is now active.  Please
> use the new PR based code review process for all new
> submissions starting today.
>
> * The Wiki has been updated with the process changes.
>
>   
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
>
>   Big thanks to Michael Kubacki for writing up all the
>   changes based on the RFC proposal and community discussions.
>
>   We will learn by using, so if you see anything missing or
>   incorrect or clarifications needed, please send feedback
>   here so the Wiki pages can be updated quickly for everyone.
>
> * The edk2 repo settings have been updated to require
>   a GitHub PR code review approval before merging and
>   all conversations must be resolved before merging.
>
> * A PR has been opened that removes the requirement for
>   Cc: tags in the commit messages and is the first PR
>   that will use the new process. This PR needs to be
>   reviewed and merged to support the revised commit
>   message format.
>
>   https://github.com/tianocore/edk2/pull/5688
>
>   https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
>
> * Please use "Draft" PRs to run CI without any reviews.
>   Once ready for reviews, convert from "Draft" to
>   "Ready for Review".
>

Generally GitHub doesn't allow CI to run on PRs created as draft pull
requests. Was this changed for edk2?


-- 
真実はいつも一つ!/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119429): https://edk2.groups.io/g/devel/message/119429
Mute This Topic: https://groups.io/mt/106355103/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-12-21 Thread Neal Gompa
On Tue, Dec 19, 2023 at 9:11 AM Samer El-Haj-Mahmoud
 wrote:
>
> Thank you all!
>

Thank you everyone, I see that it has landed in the repository now:
https://github.com/tianocore/edk2/commit/8c1e9f9c6fa7b5137003b0cfa6d54a6bada16d8e



-- 
真実はいつも一つ!/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112828): https://edk2.groups.io/g/devel/message/112828
Mute This Topic: https://groups.io/mt/103262405/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-12-06 Thread Neal Gompa
On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa  wrote:
>
> On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek  wrote:
> >
> > On 10/31/23 23:27, Jeremy Linton wrote:
> > > On 10/31/23 12:37, Neal Gompa via groups.io wrote:
> > >> From: Neal Gompa 
> > >>
> > >> Currently, the ReadyToBoot event is only signaled when a formal Boot
> > >> Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).
> > >>
> > >> However, the introduction of Platform Recovery in UEFI 2.5 makes it
> > >> necessary to signal ReadyToBoot when a Platform Recovery boot loader
> > >> runs because otherwise it may lead to the execution of a boot loader
> > >> that has similar requirements to a regular one that is not launched
> > >> as a Boot Manager option.
> > >>
> > >> This is especially critical to ensuring that the graphical console
> > >> is actually usable during platform recovery, as some platforms do
> > >> rely on the ConsolePrefDxe driver, which only performs console
> > >> initialization after ReadyToBoot is triggered.
> > >>
> > >> This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
> > >> in EfiBootManagerProcessLoadOption () when invoking platform recovery,
> > >> which is the function that sets up the platform recovery boot process.
> > >>
> > >> The expected behavior has been clarified in the UEFI 2.10 specification
> > >> to explicitly indicate this behavior is required for correct operation.
> > >>
> > >> This is a rebased version of the patch originally written by Pete Batard.
> > >
> > > Took me a bit to swap in that whole conversation again, and recheck the
> > > spec's and code paths, but this all looks fine to me and should allow
> > > the PFTF build to drop the similar patch from Pete that has been carried
> > > downstream for the past couple years.
> > >
> > > As for testing the previous patch has been in the field for a couple
> > > years now and i'm not aware of it causing any issues. The additional
> > > restriction of limiting it to platform recovery logically makes sense,
> > > and as far as I can see shouldn't cause any problems.
> > >
> > > So,
> > >
> > > Reviewed-by: Jeremy Linton 
> > >
> > >
> > > As a PS: I also went to check the ready to boot behavior for OsRecovery
> > > and realized that apparently none of that support was ever merged?
> >
> > What else is there, outside of this patch, in need of upstreaming?
> >
> > > That seems a bit of an oversight since its been in the spec for a few 
> > > years now.
> >
> > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > commit message), which is quite recent ("Aug 29, 2022").
> >
> > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> >
>
> Is there anything else holding up committing this patch? Jeremy and
> you reviewed it earlier in the month...
>

Friendly ping again? It's been a month now...



-- 
真実はいつも一つ!/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112157): https://edk2.groups.io/g/devel/message/112157
Mute This Topic: https://groups.io/mt/102302654/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-11-24 Thread Neal Gompa
On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek  wrote:
>
> On 10/31/23 23:27, Jeremy Linton wrote:
> > On 10/31/23 12:37, Neal Gompa via groups.io wrote:
> >> From: Neal Gompa 
> >>
> >> Currently, the ReadyToBoot event is only signaled when a formal Boot
> >> Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).
> >>
> >> However, the introduction of Platform Recovery in UEFI 2.5 makes it
> >> necessary to signal ReadyToBoot when a Platform Recovery boot loader
> >> runs because otherwise it may lead to the execution of a boot loader
> >> that has similar requirements to a regular one that is not launched
> >> as a Boot Manager option.
> >>
> >> This is especially critical to ensuring that the graphical console
> >> is actually usable during platform recovery, as some platforms do
> >> rely on the ConsolePrefDxe driver, which only performs console
> >> initialization after ReadyToBoot is triggered.
> >>
> >> This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
> >> in EfiBootManagerProcessLoadOption () when invoking platform recovery,
> >> which is the function that sets up the platform recovery boot process.
> >>
> >> The expected behavior has been clarified in the UEFI 2.10 specification
> >> to explicitly indicate this behavior is required for correct operation.
> >>
> >> This is a rebased version of the patch originally written by Pete Batard.
> >
> > Took me a bit to swap in that whole conversation again, and recheck the
> > spec's and code paths, but this all looks fine to me and should allow
> > the PFTF build to drop the similar patch from Pete that has been carried
> > downstream for the past couple years.
> >
> > As for testing the previous patch has been in the field for a couple
> > years now and i'm not aware of it causing any issues. The additional
> > restriction of limiting it to platform recovery logically makes sense,
> > and as far as I can see shouldn't cause any problems.
> >
> > So,
> >
> > Reviewed-by: Jeremy Linton 
> >
> >
> > As a PS: I also went to check the ready to boot behavior for OsRecovery
> > and realized that apparently none of that support was ever merged?
>
> What else is there, outside of this patch, in need of upstreaming?
>
> > That seems a bit of an oversight since its been in the spec for a few years 
> > now.
>
> "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> commit message), which is quite recent ("Aug 29, 2022").
>
> I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
>

Is there anything else holding up committing this patch? Jeremy and
you reviewed it earlier in the month...



--
真実はいつも一つ!/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111703): https://edk2.groups.io/g/devel/message/111703
Mute This Topic: https://groups.io/mt/102302654/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-10-31 Thread Neal Gompa
On Thu, Oct 26, 2023 at 12:39 PM Laszlo Ersek  wrote:
>
> On 10/26/23 18:19, Laszlo Ersek wrote:
> > On 10/26/23 15:53, Neal Gompa wrote:
> >> From: Neal Gompa 
> >>
> >> Currently, the ReadyToBoot event is only signaled when a formal Boot
> >> Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).
> >>
> >> However, the introduction of Platform Recovery in UEFI 2.5 makes it
> >> necessary to signal ReadyToBoot when a Platform Recovery boot loader
> >> runs because otherwise it may lead to the execution of a boot loader
> >> that has similar requirements to a regular one that is not launched
> >> as a Boot Manager option.
> >>
> >> This is especially critical to ensuring that the graphical console
> >> is actually usable during platform recovery, as some platforms do
> >> rely on the ConsolePrefDxe driver, which only performs console
> >> initialization after ReadyToBoot is triggered.
> >>
> >> This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
> >> in EfiBootManagerProcessLoadOption (), which is the function that sets
> >> up the platform recovery boot process.
> >>
> >> The expected behavior has been clarified in the UEFI 2.10 specification
> >> to explicitly indicate this behavior is required for correct operation.
> >>
> >> This is a rebased version of the patch originally written by Pete Batard.
> >>
> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831
> >>
> >> Cc: Pete Batard 
> >> Cc: Daniel P. Berrangé 
> >> Cc: Gerd Hoffmann 
> >> Cc: Samer El-Haj-Mahmoud 
> >>
> >> Co-authored-by: Pete Batard 
> >> Signed-off-by: Neal Gompa 
> >> ---
> >>  MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
> >> b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> >> index 2087f0b91d..31ed608817 100644
> >> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> >> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> >> @@ -1416,6 +1416,15 @@ EfiBootManagerProcessLoadOption (
> >>  return EFI_SUCCESS;
> >>}
> >>
> >> +  //
> >> +  // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load 
> >> and execute the boot option.
> >> +  //
> >> +  EfiSignalEventReadyToBoot ();
> >> +  //
> >> +  // Report Status Code to indicate ReadyToBoot was signalled
> >> +  //
> >> +  REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | 
> >> EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
> >> +
> >>//
> >>// Load and start the load option.
> >>//
> >
> > While the patch does the right thing for the latest UEFI spec language,
> > it does a bit too much.
> >
> > The spec says (v2.10), under 7.1.2 "EFI_BOOT_SERVICES.CreateEventEx()":
> >
> > ---
> > EFI_EVENT_GROUP_READY_TO_BOOT
> >
> > This event group is notified by the system right before notifying
> > EFI_EVENT_GROUP_AFTER_READY_TO_BOOT event group when the Boot Manager is
> > about to load and execute a boot option or a platform or OS recovery
> > option. The event group presents the last chance to modify device or
> > system configuration prior to passing control to a boot option.
> > ---
> >
> > NB "boot option", or "platform or OS recovery option".
> >
> > However, EfiBootManagerProcessLoadOption() is also used for launching
> > Driver and SysPrep options.
> >
> > EfiBootManagerProcessLoadOption() has two call sites:
> >
> > (1) in ProcessLoadOptions() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]
> >
> > (2) near the end of BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]
> >
> > Call site (2) bears the comment "When platform recovery is not enabled,
> > still boot to platform default file path", so I figure signaling
> > ready-to-boot at that point is fine.
> >
> > Call site (1) requires further investigation. Namely,
> > ProcessLoadOptions() itself is called from three locations, all inside
> > BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]:
> >
> > (1.1) under comment "Execute Driver Options", for "LoadOptionTypeDriver"
> > type load options
> >
> > (1.2) und

[edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-10-31 Thread Neal Gompa
From: Neal Gompa 

Currently, the ReadyToBoot event is only signaled when a formal Boot
Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).

However, the introduction of Platform Recovery in UEFI 2.5 makes it
necessary to signal ReadyToBoot when a Platform Recovery boot loader
runs because otherwise it may lead to the execution of a boot loader
that has similar requirements to a regular one that is not launched
as a Boot Manager option.

This is especially critical to ensuring that the graphical console
is actually usable during platform recovery, as some platforms do
rely on the ConsolePrefDxe driver, which only performs console
initialization after ReadyToBoot is triggered.

This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
in EfiBootManagerProcessLoadOption () when invoking platform recovery,
which is the function that sets up the platform recovery boot process.

The expected behavior has been clarified in the UEFI 2.10 specification
to explicitly indicate this behavior is required for correct operation.

This is a rebased version of the patch originally written by Pete Batard.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831

Cc: Pete Batard 
Cc: Daniel P. Berrangé 
Cc: Gerd Hoffmann 
Cc: Samer El-Haj-Mahmoud 
Cc: Laszlo Ersek 

Co-authored-by: Pete Batard 
Signed-off-by: Neal Gompa 
---
 .../Library/UefiBootManagerLib/BmLoadOption.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 2087f0b91d..83a2f893e4 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1416,6 +1416,17 @@ EfiBootManagerProcessLoadOption (
 return EFI_SUCCESS;
   }
 
+  if (LoadOption->OptionType == LoadOptionTypePlatformRecovery) {
+//
+// Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and 
execute the boot option.
+//
+EfiSignalEventReadyToBoot ();
+//
+// Report Status Code to indicate ReadyToBoot was signaled
+//
+REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | 
EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+  }
+
   //
   // Load and start the load option.
   //
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110438): https://edk2.groups.io/g/devel/message/110438
Mute This Topic: https://groups.io/mt/102302654/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2023-10-26 Thread Neal Gompa
From: Neal Gompa 

Currently, the ReadyToBoot event is only signaled when a formal Boot
Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).

However, the introduction of Platform Recovery in UEFI 2.5 makes it
necessary to signal ReadyToBoot when a Platform Recovery boot loader
runs because otherwise it may lead to the execution of a boot loader
that has similar requirements to a regular one that is not launched
as a Boot Manager option.

This is especially critical to ensuring that the graphical console
is actually usable during platform recovery, as some platforms do
rely on the ConsolePrefDxe driver, which only performs console
initialization after ReadyToBoot is triggered.

This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
in EfiBootManagerProcessLoadOption (), which is the function that sets
up the platform recovery boot process.

The expected behavior has been clarified in the UEFI 2.10 specification
to explicitly indicate this behavior is required for correct operation.

This is a rebased version of the patch originally written by Pete Batard.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831

Cc: Pete Batard 
Cc: Daniel P. Berrangé 
Cc: Gerd Hoffmann 
Cc: Samer El-Haj-Mahmoud 

Co-authored-by: Pete Batard 
Signed-off-by: Neal Gompa 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 2087f0b91d..31ed608817 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1416,6 +1416,15 @@ EfiBootManagerProcessLoadOption (
 return EFI_SUCCESS;
   }
 
+  //
+  // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and 
execute the boot option.
+  //
+  EfiSignalEventReadyToBoot ();
+  //
+  // Report Status Code to indicate ReadyToBoot was signalled
+  //
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | 
EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   //
   // Load and start the load option.
   //
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110104): https://edk2.groups.io/g/devel/message/110104
Mute This Topic: https://groups.io/mt/102200076/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Ignore PMBR BootIndicator per UEFI spec

2021-07-05 Thread Neal Gompa
Per UEFI Spec 2.8 (UEFI_Spec_2_8_final.pdf, page 114)
5.2.3 Protective MBR
Table 20. Protective MBR Partition Record protecting the entire disk

The description for BootIndicator states the following:

> Set to 0x00 to indicate a non-bootable partition. If set to any
> value other than 0x00 the behavior of this flag on non-UEFI
> systems is undefined. Must be ignored by UEFI implementations.

Unfortunately, we have been incorrectly assuming that the
BootIndicator value must be 0x00, which leads to problems
when the 'pmbr_boot' flag is set on a disk containing a GPT
(such as with GNU parted). When the flag is set, the value
changes to 0x01, causing this check to fail and the system
is rendered unbootable despite it being valid from the
perspective of the UEFI spec.

To resolve this, we drop the check for the BootIndicator
so that we stop caring about the value set there, which
restores the capability to boot such disks.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3474

Cc: Chris Murphy 
Cc: David Duncan 
Cc: Lazlo Ersek 
Cc: Hao A Wu 
Cc: Ray Ni 
Cc: Zhichao Gao 

Signed-off-by: Neal Gompa 
---
 MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c 
b/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
index aefb2d6ecb3f..efaff5e0808f 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
@@ -264,8 +264,7 @@ PartitionInstallGptChildHandles (
   // Verify that the Protective MBR is valid
   //
   for (Index = 0; Index < MAX_MBR_PARTITIONS; Index++) {
-if (ProtectiveMbr->Partition[Index].BootIndicator == 0x00 &&
-ProtectiveMbr->Partition[Index].OSIndicator == PMBR_GPT_PARTITION &&
+if (ProtectiveMbr->Partition[Index].OSIndicator == PMBR_GPT_PARTITION &&
 UNPACK_UINT32 (ProtectiveMbr->Partition[Index].StartingLBA) == 1
 ) {
   break;
-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77477): https://edk2.groups.io/g/devel/message/77477
Mute This Topic: https://groups.io/mt/83993973/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-