* [PATCH] ArmPkg: Enable boot discovery policy for ARM package. @ 2021-08-06 8:30 Grzegorz Bernacki 2021-08-06 9:02 ` Sunny Wang 2021-08-06 13:44 ` Ard Biesheuvel 0 siblings, 2 replies; 7+ messages in thread From: Grzegorz Bernacki @ 2021-08-06 8:30 UTC (permalink / raw) To: devel Cc: leif, ardb+tianocore, Samer.El-Haj-Mahmoud, sunny.Wang, mw, upstream, Grzegorz Bernacki This commit adds code which check BootDiscoveryPolicy variable and calls Boot Policy Manager Protocol to connect device specified by the variable. To enable that mechanism for platform EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be added to DSC file and BootDiscoveryPolicyUiLib should be added to UiApp libraries. Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> --- ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf index 353d7a967b..86751b45f8 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -65,11 +65,15 @@ [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy [Guids] + gBootDiscoveryPolicyMgrFormsetGuid gEdkiiNonDiscoverableEhciDeviceGuid gEdkiiNonDiscoverableUhciDeviceGuid gEdkiiNonDiscoverableXhciDeviceGuid + gEfiBootManagerPolicyNetworkGuid + gEfiBootManagerPolicyConnectAllGuid gEfiFileInfoGuid gEfiFileSystemInfoGuid gEfiFileSystemVolumeLabelInfoIdGuid @@ -79,6 +83,7 @@ [Protocols] gEdkiiNonDiscoverableDeviceProtocolGuid + gEfiBootManagerPolicyProtocolGuid gEfiDevicePathProtocolGuid gEfiGraphicsOutputProtocolGuid gEfiLoadedImageProtocolGuid diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c index 5ceb23d822..4332c45bb7 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c @@ -2,9 +2,10 @@ Implementation for PlatformBootManagerLib library class interfaces. Copyright (C) 2015-2016, Red Hat, Inc. - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> + Copyright (c) 2021, Semihalf All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -19,6 +20,7 @@ #include <Library/UefiBootManagerLib.h> #include <Library/UefiLib.h> #include <Library/UefiRuntimeServicesTableLib.h> +#include <Protocol/BootManagerPolicy.h> #include <Protocol/DevicePath.h> #include <Protocol/EsrtManagement.h> #include <Protocol/GraphicsOutput.h> @@ -27,6 +29,7 @@ #include <Protocol/PciIo.h> #include <Protocol/PciRootBridgeIo.h> #include <Protocol/PlatformBootManager.h> +#include <Guid/BootDiscoveryPolicy.h> #include <Guid/EventGroup.h> #include <Guid/NonDiscoverableDevice.h> #include <Guid/TtyTerm.h> @@ -703,6 +706,91 @@ HandleCapsules ( #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " +/** + This functions checks the value of BootDiscoverPolicy variable and + connect devices of class specified by that variable. Then it refreshes + Boot order for newly discovered boot device. + + @retval EFI_SUCCESS Devices connected succesfully or connection + not required. + @retval others Return values from GetVariable(), LocateProtocol() + and ConnectDeviceClass(). +--*/ +STATIC +EFI_STATUS +BootDiscoveryPolicyHandler ( + VOID + ) +{ + EFI_STATUS Status; + UINT32 DiscoveryPolicy; + UINTN Size; + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; + EFI_GUID *Class; + + Size = sizeof (DiscoveryPolicy); + Status = gRT->GetVariable ( + BOOT_DISCOVERY_POLICY_VAR, + &gBootDiscoveryPolicyMgrFormsetGuid, + NULL, + &Size, + &DiscoveryPolicy + ); + if (Status == EFI_NOT_FOUND) { + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); + if (Status == EFI_NOT_FOUND) { + return EFI_SUCCESS; + } else if (EFI_ERROR (Status)) { + return Status; + } + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); + } else if (EFI_ERROR (Status)) { + return Status; + } + + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { + return EFI_SUCCESS; + } + + switch (DiscoveryPolicy) { + case BDP_CONNECT_NET: + Class = &gEfiBootManagerPolicyNetworkGuid; + break; + case BDP_CONNECT_ALL: + Class = &gEfiBootManagerPolicyConnectAllGuid; + break; + default: + DEBUG (( + DEBUG_INFO, + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", + __FUNCTION__, + DiscoveryPolicy + )); + return EFI_SUCCESS; + } + + Status = gBS->LocateProtocol ( + &gEfiBootManagerPolicyProtocolGuid, + NULL, + (VOID **)&BMPolicy + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." + "Driver connect will be skipped.\n", __FUNCTION__)); + return Status; + } + + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); + if (EFI_ERROR (Status)){ + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); + return Status; + } + + EfiBootManagerRefreshAllBootOption(); + + return EFI_SUCCESS; +} + /** Do the platform specific action after the console is ready Possible things that can be done in PlatformBootManagerAfterConsole: @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( } } + // + // Connect device specified by BootDiscoverPolicy variable and + // refresh Boot order for newly discovered boot devices + // + BootDiscoveryPolicyHandler (); + // // On ARM, there is currently no reason to use the phased capsule // update approach where some capsules are dispatched before EndOfDxe -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-06 8:30 [PATCH] ArmPkg: Enable boot discovery policy for ARM package Grzegorz Bernacki @ 2021-08-06 9:02 ` Sunny Wang 2021-08-06 13:44 ` Ard Biesheuvel 1 sibling, 0 replies; 7+ messages in thread From: Sunny Wang @ 2021-08-06 9:02 UTC (permalink / raw) To: Grzegorz Bernacki, devel@edk2.groups.io Cc: leif@nuviainc.com, ardb+tianocore@kernel.org, Samer El-Haj-Mahmoud, mw@semihalf.com, upstream@semihalf.com, Sunny Wang Looks good. Thanks, Greg. Reviewed-by: Sunny Wang <sunny.wang@arm.com> For others' reference, this patch is similar to the one below for Rasberry Pi that just got merged. - https://edk2.groups.io/g/devel/message/78514 After merging this change, we should be able to easily enable "boot discovery policy" on other ARM platforms. For the platform that doesn't include BootManagerPolicyDxe driver but using ArmPkg PlatformBootManagerLib, it would just do nothing (the behavior will be kept the same as before). Best Regards, Sunny Wang -----Original Message----- From: Grzegorz Bernacki <gjb@semihalf.com> Sent: Friday, August 6, 2021 4:30 PM To: devel@edk2.groups.io Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; mw@semihalf.com; upstream@semihalf.com; Grzegorz Bernacki <gjb@semihalf.com> Subject: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. This commit adds code which check BootDiscoveryPolicy variable and calls Boot Policy Manager Protocol to connect device specified by the variable. To enable that mechanism for platform EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be added to DSC file and BootDiscoveryPolicyUiLib should be added to UiApp libraries. Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> --- ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf index 353d7a967b..86751b45f8 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -65,11 +65,15 @@ [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy [Guids] + gBootDiscoveryPolicyMgrFormsetGuid gEdkiiNonDiscoverableEhciDeviceGuid gEdkiiNonDiscoverableUhciDeviceGuid gEdkiiNonDiscoverableXhciDeviceGuid + gEfiBootManagerPolicyNetworkGuid + gEfiBootManagerPolicyConnectAllGuid gEfiFileInfoGuid gEfiFileSystemInfoGuid gEfiFileSystemVolumeLabelInfoIdGuid @@ -79,6 +83,7 @@ [Protocols] gEdkiiNonDiscoverableDeviceProtocolGuid + gEfiBootManagerPolicyProtocolGuid gEfiDevicePathProtocolGuid gEfiGraphicsOutputProtocolGuid gEfiLoadedImageProtocolGuid diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c index 5ceb23d822..4332c45bb7 100644 --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c @@ -2,9 +2,10 @@ Implementation for PlatformBootManagerLib library class interfaces. Copyright (C) 2015-2016, Red Hat, Inc. - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> + Copyright (c) 2021, Semihalf All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -19,6 +20,7 @@ #include <Library/UefiBootManagerLib.h> #include <Library/UefiLib.h> #include <Library/UefiRuntimeServicesTableLib.h> +#include <Protocol/BootManagerPolicy.h> #include <Protocol/DevicePath.h> #include <Protocol/EsrtManagement.h> #include <Protocol/GraphicsOutput.h> @@ -27,6 +29,7 @@ #include <Protocol/PciIo.h> #include <Protocol/PciRootBridgeIo.h> #include <Protocol/PlatformBootManager.h> +#include <Guid/BootDiscoveryPolicy.h> #include <Guid/EventGroup.h> #include <Guid/NonDiscoverableDevice.h> #include <Guid/TtyTerm.h> @@ -703,6 +706,91 @@ HandleCapsules ( #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " +/** + This functions checks the value of BootDiscoverPolicy variable and + connect devices of class specified by that variable. Then it refreshes + Boot order for newly discovered boot device. + + @retval EFI_SUCCESS Devices connected succesfully or connection + not required. + @retval others Return values from GetVariable(), LocateProtocol() + and ConnectDeviceClass(). +--*/ +STATIC +EFI_STATUS +BootDiscoveryPolicyHandler ( + VOID + ) +{ + EFI_STATUS Status; + UINT32 DiscoveryPolicy; + UINTN Size; + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; + EFI_GUID *Class; + + Size = sizeof (DiscoveryPolicy); + Status = gRT->GetVariable ( + BOOT_DISCOVERY_POLICY_VAR, + &gBootDiscoveryPolicyMgrFormsetGuid, + NULL, + &Size, + &DiscoveryPolicy + ); + if (Status == EFI_NOT_FOUND) { + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); + if (Status == EFI_NOT_FOUND) { + return EFI_SUCCESS; + } else if (EFI_ERROR (Status)) { + return Status; + } + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); + } else if (EFI_ERROR (Status)) { + return Status; + } + + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { + return EFI_SUCCESS; + } + + switch (DiscoveryPolicy) { + case BDP_CONNECT_NET: + Class = &gEfiBootManagerPolicyNetworkGuid; + break; + case BDP_CONNECT_ALL: + Class = &gEfiBootManagerPolicyConnectAllGuid; + break; + default: + DEBUG (( + DEBUG_INFO, + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", + __FUNCTION__, + DiscoveryPolicy + )); + return EFI_SUCCESS; + } + + Status = gBS->LocateProtocol ( + &gEfiBootManagerPolicyProtocolGuid, + NULL, + (VOID **)&BMPolicy + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." + "Driver connect will be skipped.\n", __FUNCTION__)); + return Status; + } + + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); + if (EFI_ERROR (Status)){ + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); + return Status; + } + + EfiBootManagerRefreshAllBootOption(); + + return EFI_SUCCESS; +} + /** Do the platform specific action after the console is ready Possible things that can be done in PlatformBootManagerAfterConsole: @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( } } + // + // Connect device specified by BootDiscoverPolicy variable and + // refresh Boot order for newly discovered boot devices + // + BootDiscoveryPolicyHandler (); + // // On ARM, there is currently no reason to use the phased capsule // update approach where some capsules are dispatched before EndOfDxe -- 2.25.1 IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-06 8:30 [PATCH] ArmPkg: Enable boot discovery policy for ARM package Grzegorz Bernacki 2021-08-06 9:02 ` Sunny Wang @ 2021-08-06 13:44 ` Ard Biesheuvel 2021-08-10 15:23 ` Sunny Wang 1 sibling, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2021-08-06 13:44 UTC (permalink / raw) To: Grzegorz Bernacki Cc: edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Samer El-Haj-Mahmoud, Sunny Wang, Marcin Wojtas, upstream On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote: > > This commit adds code which check BootDiscoveryPolicy variable and > calls Boot Policy Manager Protocol to connect device specified by > the variable. To enable that mechanism for platform > EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be > added to DSC file and BootDiscoveryPolicyUiLib should be added to > UiApp libraries. > ... or the platform will be broken once we apply this patch, right? If so, please propose patches for all platforms in edk2-platforms that use this library - we can't just break them. > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> > --- > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- > 2 files changed, 100 insertions(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 353d7a967b..86751b45f8 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -65,11 +65,15 @@ > > [Pcd] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > [Guids] > + gBootDiscoveryPolicyMgrFormsetGuid > gEdkiiNonDiscoverableEhciDeviceGuid > gEdkiiNonDiscoverableUhciDeviceGuid > gEdkiiNonDiscoverableXhciDeviceGuid > + gEfiBootManagerPolicyNetworkGuid > + gEfiBootManagerPolicyConnectAllGuid > gEfiFileInfoGuid > gEfiFileSystemInfoGuid > gEfiFileSystemVolumeLabelInfoIdGuid > @@ -79,6 +83,7 @@ > > [Protocols] > gEdkiiNonDiscoverableDeviceProtocolGuid > + gEfiBootManagerPolicyProtocolGuid > gEfiDevicePathProtocolGuid > gEfiGraphicsOutputProtocolGuid > gEfiLoadedImageProtocolGuid > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > index 5ceb23d822..4332c45bb7 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > @@ -2,9 +2,10 @@ > Implementation for PlatformBootManagerLib library class interfaces. > > Copyright (C) 2015-2016, Red Hat, Inc. > - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> > + Copyright (c) 2021, Semihalf All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -19,6 +20,7 @@ > #include <Library/UefiBootManagerLib.h> > #include <Library/UefiLib.h> > #include <Library/UefiRuntimeServicesTableLib.h> > +#include <Protocol/BootManagerPolicy.h> > #include <Protocol/DevicePath.h> > #include <Protocol/EsrtManagement.h> > #include <Protocol/GraphicsOutput.h> > @@ -27,6 +29,7 @@ > #include <Protocol/PciIo.h> > #include <Protocol/PciRootBridgeIo.h> > #include <Protocol/PlatformBootManager.h> > +#include <Guid/BootDiscoveryPolicy.h> > #include <Guid/EventGroup.h> > #include <Guid/NonDiscoverableDevice.h> > #include <Guid/TtyTerm.h> > @@ -703,6 +706,91 @@ HandleCapsules ( > > #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " > > +/** > + This functions checks the value of BootDiscoverPolicy variable and > + connect devices of class specified by that variable. Then it refreshes > + Boot order for newly discovered boot device. > + > + @retval EFI_SUCCESS Devices connected succesfully or connection > + not required. > + @retval others Return values from GetVariable(), LocateProtocol() > + and ConnectDeviceClass(). > +--*/ > +STATIC > +EFI_STATUS > +BootDiscoveryPolicyHandler ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINT32 DiscoveryPolicy; > + UINTN Size; > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > + EFI_GUID *Class; > + > + Size = sizeof (DiscoveryPolicy); > + Status = gRT->GetVariable ( > + BOOT_DISCOVERY_POLICY_VAR, > + &gBootDiscoveryPolicyMgrFormsetGuid, > + NULL, > + &Size, > + &DiscoveryPolicy > + ); > + if (Status == EFI_NOT_FOUND) { > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); > + if (Status == EFI_NOT_FOUND) { > + return EFI_SUCCESS; > + } else if (EFI_ERROR (Status)) { > + return Status; > + } > + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); > + } else if (EFI_ERROR (Status)) { > + return Status; > + } > + > + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { > + return EFI_SUCCESS; > + } > + > + switch (DiscoveryPolicy) { > + case BDP_CONNECT_NET: > + Class = &gEfiBootManagerPolicyNetworkGuid; > + break; > + case BDP_CONNECT_ALL: > + Class = &gEfiBootManagerPolicyConnectAllGuid; > + break; > + default: > + DEBUG (( > + DEBUG_INFO, > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", > + __FUNCTION__, > + DiscoveryPolicy > + )); > + return EFI_SUCCESS; > + } > + > + Status = gBS->LocateProtocol ( > + &gEfiBootManagerPolicyProtocolGuid, > + NULL, > + (VOID **)&BMPolicy > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." > + "Driver connect will be skipped.\n", __FUNCTION__)); > + return Status; > + } > + > + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); > + if (EFI_ERROR (Status)){ > + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); > + return Status; > + } > + > + EfiBootManagerRefreshAllBootOption(); > + > + return EFI_SUCCESS; > +} > + > /** > Do the platform specific action after the console is ready > Possible things that can be done in PlatformBootManagerAfterConsole: > @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( > } > } > > + // > + // Connect device specified by BootDiscoverPolicy variable and > + // refresh Boot order for newly discovered boot devices > + // > + BootDiscoveryPolicyHandler (); > + > // > // On ARM, there is currently no reason to use the phased capsule > // update approach where some capsules are dispatched before EndOfDxe > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-06 13:44 ` Ard Biesheuvel @ 2021-08-10 15:23 ` Sunny Wang 2021-08-11 11:57 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Sunny Wang @ 2021-08-10 15:23 UTC (permalink / raw) To: Ard Biesheuvel, Grzegorz Bernacki Cc: edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Samer El-Haj-Mahmoud, Marcin Wojtas, upstream@semihalf.com, Sunny Wang Hi Ard, Yeah, this is a good point. Greg and I offline discussed this as well. If we don't miss anything, only applying this patch without platform changes should be fine. There should be no behavior change. I added some details below for your reference. - if the platform doesn’t add PcdBootDiscoveryPolicy to platform's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPolicyHandler() would just get PcdBootDiscoveryPolicy default value and do nothing. - if the platform doesn't include BootManagerPolicyDxe driver or produce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just return with only printing a message to remind users/developers that Boot Discovery Policy doesn't work (driver connect will be skipped). Best Regards, Sunny Wang -----Original Message----- From: Ard Biesheuvel <ardb@kernel.org> Sent: Friday, August 6, 2021 9:45 PM To: Grzegorz Bernacki <gjb@semihalf.com> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Marcin Wojtas <mw@semihalf.com>; upstream@semihalf.com Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote: > > This commit adds code which check BootDiscoveryPolicy variable and > calls Boot Policy Manager Protocol to connect device specified by > the variable. To enable that mechanism for platform > EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be > added to DSC file and BootDiscoveryPolicyUiLib should be added to > UiApp libraries. > ... or the platform will be broken once we apply this patch, right? If so, please propose patches for all platforms in edk2-platforms that use this library - we can't just break them. > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> > --- > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- > 2 files changed, 100 insertions(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 353d7a967b..86751b45f8 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -65,11 +65,15 @@ > > [Pcd] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > [Guids] > + gBootDiscoveryPolicyMgrFormsetGuid > gEdkiiNonDiscoverableEhciDeviceGuid > gEdkiiNonDiscoverableUhciDeviceGuid > gEdkiiNonDiscoverableXhciDeviceGuid > + gEfiBootManagerPolicyNetworkGuid > + gEfiBootManagerPolicyConnectAllGuid > gEfiFileInfoGuid > gEfiFileSystemInfoGuid > gEfiFileSystemVolumeLabelInfoIdGuid > @@ -79,6 +83,7 @@ > > [Protocols] > gEdkiiNonDiscoverableDeviceProtocolGuid > + gEfiBootManagerPolicyProtocolGuid > gEfiDevicePathProtocolGuid > gEfiGraphicsOutputProtocolGuid > gEfiLoadedImageProtocolGuid > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > index 5ceb23d822..4332c45bb7 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > @@ -2,9 +2,10 @@ > Implementation for PlatformBootManagerLib library class interfaces. > > Copyright (C) 2015-2016, Red Hat, Inc. > - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> > + Copyright (c) 2021, Semihalf All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -19,6 +20,7 @@ > #include <Library/UefiBootManagerLib.h> > #include <Library/UefiLib.h> > #include <Library/UefiRuntimeServicesTableLib.h> > +#include <Protocol/BootManagerPolicy.h> > #include <Protocol/DevicePath.h> > #include <Protocol/EsrtManagement.h> > #include <Protocol/GraphicsOutput.h> > @@ -27,6 +29,7 @@ > #include <Protocol/PciIo.h> > #include <Protocol/PciRootBridgeIo.h> > #include <Protocol/PlatformBootManager.h> > +#include <Guid/BootDiscoveryPolicy.h> > #include <Guid/EventGroup.h> > #include <Guid/NonDiscoverableDevice.h> > #include <Guid/TtyTerm.h> > @@ -703,6 +706,91 @@ HandleCapsules ( > > #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " > > +/** > + This functions checks the value of BootDiscoverPolicy variable and > + connect devices of class specified by that variable. Then it refreshes > + Boot order for newly discovered boot device. > + > + @retval EFI_SUCCESS Devices connected succesfully or connection > + not required. > + @retval others Return values from GetVariable(), LocateProtocol() > + and ConnectDeviceClass(). > +--*/ > +STATIC > +EFI_STATUS > +BootDiscoveryPolicyHandler ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINT32 DiscoveryPolicy; > + UINTN Size; > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > + EFI_GUID *Class; > + > + Size = sizeof (DiscoveryPolicy); > + Status = gRT->GetVariable ( > + BOOT_DISCOVERY_POLICY_VAR, > + &gBootDiscoveryPolicyMgrFormsetGuid, > + NULL, > + &Size, > + &DiscoveryPolicy > + ); > + if (Status == EFI_NOT_FOUND) { > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); > + if (Status == EFI_NOT_FOUND) { > + return EFI_SUCCESS; > + } else if (EFI_ERROR (Status)) { > + return Status; > + } > + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); > + } else if (EFI_ERROR (Status)) { > + return Status; > + } > + > + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { > + return EFI_SUCCESS; > + } > + > + switch (DiscoveryPolicy) { > + case BDP_CONNECT_NET: > + Class = &gEfiBootManagerPolicyNetworkGuid; > + break; > + case BDP_CONNECT_ALL: > + Class = &gEfiBootManagerPolicyConnectAllGuid; > + break; > + default: > + DEBUG (( > + DEBUG_INFO, > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", > + __FUNCTION__, > + DiscoveryPolicy > + )); > + return EFI_SUCCESS; > + } > + > + Status = gBS->LocateProtocol ( > + &gEfiBootManagerPolicyProtocolGuid, > + NULL, > + (VOID **)&BMPolicy > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." > + "Driver connect will be skipped.\n", __FUNCTION__)); > + return Status; > + } > + > + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); > + if (EFI_ERROR (Status)){ > + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); > + return Status; > + } > + > + EfiBootManagerRefreshAllBootOption(); > + > + return EFI_SUCCESS; > +} > + > /** > Do the platform specific action after the console is ready > Possible things that can be done in PlatformBootManagerAfterConsole: > @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( > } > } > > + // > + // Connect device specified by BootDiscoverPolicy variable and > + // refresh Boot order for newly discovered boot devices > + // > + BootDiscoveryPolicyHandler (); > + > // > // On ARM, there is currently no reason to use the phased capsule > // update approach where some capsules are dispatched before EndOfDxe > -- > 2.25.1 > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-10 15:23 ` Sunny Wang @ 2021-08-11 11:57 ` Ard Biesheuvel 2021-08-11 11:57 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2021-08-11 11:57 UTC (permalink / raw) To: Sunny Wang Cc: Grzegorz Bernacki, edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Samer El-Haj-Mahmoud, Marcin Wojtas, upstream@semihalf.com On Tue, 10 Aug 2021 at 17:23, Sunny Wang <Sunny.Wang@arm.com> wrote: > > Hi Ard, > > Yeah, this is a good point. Greg and I offline discussed this as well. > If we don't miss anything, only applying this patch without platform changes should be fine. There should be no behavior change. I added some details below for your reference. > - if the platform doesn’t add PcdBootDiscoveryPolicy to platform's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPolicyHandler() would just get PcdBootDiscoveryPolicy default value and do nothing. > - if the platform doesn't include BootManagerPolicyDxe driver or produce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just return with only printing a message to remind users/developers that Boot Discovery Policy doesn't work (driver connect will be skipped). > OK, thanks for double checking. I tried to submit this but I get CI errors: > > -----Original Message----- > From: Ard Biesheuvel <ardb@kernel.org> > Sent: Friday, August 6, 2021 9:45 PM > To: Grzegorz Bernacki <gjb@semihalf.com> > Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Marcin Wojtas <mw@semihalf.com>; upstream@semihalf.com > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. > > On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote: > > > > This commit adds code which check BootDiscoveryPolicy variable and > > calls Boot Policy Manager Protocol to connect device specified by > > the variable. To enable that mechanism for platform > > EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be > > added to DSC file and BootDiscoveryPolicyUiLib should be added to > > UiApp libraries. > > > > ... or the platform will be broken once we apply this patch, right? If > so, please propose patches for all platforms in edk2-platforms that > use this library - we can't just break them. > > > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> > > --- > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- > > 2 files changed, 100 insertions(+), 1 deletion(-) > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > index 353d7a967b..86751b45f8 100644 > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > @@ -65,11 +65,15 @@ > > > > [Pcd] > > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > > > [Guids] > > + gBootDiscoveryPolicyMgrFormsetGuid > > gEdkiiNonDiscoverableEhciDeviceGuid > > gEdkiiNonDiscoverableUhciDeviceGuid > > gEdkiiNonDiscoverableXhciDeviceGuid > > + gEfiBootManagerPolicyNetworkGuid > > + gEfiBootManagerPolicyConnectAllGuid > > gEfiFileInfoGuid > > gEfiFileSystemInfoGuid > > gEfiFileSystemVolumeLabelInfoIdGuid > > @@ -79,6 +83,7 @@ > > > > [Protocols] > > gEdkiiNonDiscoverableDeviceProtocolGuid > > + gEfiBootManagerPolicyProtocolGuid > > gEfiDevicePathProtocolGuid > > gEfiGraphicsOutputProtocolGuid > > gEfiLoadedImageProtocolGuid > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > index 5ceb23d822..4332c45bb7 100644 > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > @@ -2,9 +2,10 @@ > > Implementation for PlatformBootManagerLib library class interfaces. > > > > Copyright (C) 2015-2016, Red Hat, Inc. > > - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> > > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> > > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > > Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> > > + Copyright (c) 2021, Semihalf All rights reserved.<BR> > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -19,6 +20,7 @@ > > #include <Library/UefiBootManagerLib.h> > > #include <Library/UefiLib.h> > > #include <Library/UefiRuntimeServicesTableLib.h> > > +#include <Protocol/BootManagerPolicy.h> > > #include <Protocol/DevicePath.h> > > #include <Protocol/EsrtManagement.h> > > #include <Protocol/GraphicsOutput.h> > > @@ -27,6 +29,7 @@ > > #include <Protocol/PciIo.h> > > #include <Protocol/PciRootBridgeIo.h> > > #include <Protocol/PlatformBootManager.h> > > +#include <Guid/BootDiscoveryPolicy.h> > > #include <Guid/EventGroup.h> > > #include <Guid/NonDiscoverableDevice.h> > > #include <Guid/TtyTerm.h> > > @@ -703,6 +706,91 @@ HandleCapsules ( > > > > #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " > > > > +/** > > + This functions checks the value of BootDiscoverPolicy variable and > > + connect devices of class specified by that variable. Then it refreshes > > + Boot order for newly discovered boot device. > > + > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > + not required. > > + @retval others Return values from GetVariable(), LocateProtocol() > > + and ConnectDeviceClass(). > > +--*/ > > +STATIC > > +EFI_STATUS > > +BootDiscoveryPolicyHandler ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + UINT32 DiscoveryPolicy; > > + UINTN Size; > > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > > + EFI_GUID *Class; > > + > > + Size = sizeof (DiscoveryPolicy); > > + Status = gRT->GetVariable ( > > + BOOT_DISCOVERY_POLICY_VAR, > > + &gBootDiscoveryPolicyMgrFormsetGuid, > > + NULL, > > + &Size, > > + &DiscoveryPolicy > > + ); > > + if (Status == EFI_NOT_FOUND) { > > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); > > + if (Status == EFI_NOT_FOUND) { > > + return EFI_SUCCESS; > > + } else if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); > > + } else if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { > > + return EFI_SUCCESS; > > + } > > + > > + switch (DiscoveryPolicy) { > > + case BDP_CONNECT_NET: > > + Class = &gEfiBootManagerPolicyNetworkGuid; > > + break; > > + case BDP_CONNECT_ALL: > > + Class = &gEfiBootManagerPolicyConnectAllGuid; > > + break; > > + default: > > + DEBUG (( > > + DEBUG_INFO, > > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", > > + __FUNCTION__, > > + DiscoveryPolicy > > + )); > > + return EFI_SUCCESS; > > + } > > + > > + Status = gBS->LocateProtocol ( > > + &gEfiBootManagerPolicyProtocolGuid, > > + NULL, > > + (VOID **)&BMPolicy > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." > > + "Driver connect will be skipped.\n", __FUNCTION__)); > > + return Status; > > + } > > + > > + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); > > + if (EFI_ERROR (Status)){ > > + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); > > + return Status; > > + } > > + > > + EfiBootManagerRefreshAllBootOption(); > > + > > + return EFI_SUCCESS; > > +} > > + > > /** > > Do the platform specific action after the console is ready > > Possible things that can be done in PlatformBootManagerAfterConsole: > > @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( > > } > > } > > > > + // > > + // Connect device specified by BootDiscoverPolicy variable and > > + // refresh Boot order for newly discovered boot devices > > + // > > + BootDiscoveryPolicyHandler (); > > + > > // > > // On ARM, there is currently no reason to use the phased capsule > > // update approach where some capsules are dispatched before EndOfDxe > > -- > > 2.25.1 > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-11 11:57 ` Ard Biesheuvel @ 2021-08-11 11:57 ` Ard Biesheuvel 2021-08-11 12:07 ` Marcin Wojtas 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2021-08-11 11:57 UTC (permalink / raw) To: Sunny Wang Cc: Grzegorz Bernacki, edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Samer El-Haj-Mahmoud, Marcin Wojtas, upstream@semihalf.com On Wed, 11 Aug 2021 at 13:57, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 10 Aug 2021 at 17:23, Sunny Wang <Sunny.Wang@arm.com> wrote: > > > > Hi Ard, > > > > Yeah, this is a good point. Greg and I offline discussed this as well. > > If we don't miss anything, only applying this patch without platform changes should be fine. There should be no behavior change. I added some details below for your reference. > > - if the platform doesn’t add PcdBootDiscoveryPolicy to platform's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPolicyHandler() would just get PcdBootDiscoveryPolicy default value and do nothing. > > - if the platform doesn't include BootManagerPolicyDxe driver or produce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just return with only printing a message to remind users/developers that Boot Discovery Policy doesn't work (driver connect will be skipped). > > > > OK, thanks for double checking. > > I tried to submit this but I get CI errors: https://github.com/tianocore/edk2/pull/1895 > > > > > > > -----Original Message----- > > From: Ard Biesheuvel <ardb@kernel.org> > > Sent: Friday, August 6, 2021 9:45 PM > > To: Grzegorz Bernacki <gjb@semihalf.com> > > Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Marcin Wojtas <mw@semihalf.com>; upstream@semihalf.com > > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. > > > > On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote: > > > > > > This commit adds code which check BootDiscoveryPolicy variable and > > > calls Boot Policy Manager Protocol to connect device specified by > > > the variable. To enable that mechanism for platform > > > EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be > > > added to DSC file and BootDiscoveryPolicyUiLib should be added to > > > UiApp libraries. > > > > > > > ... or the platform will be broken once we apply this patch, right? If > > so, please propose patches for all platforms in edk2-platforms that > > use this library - we can't just break them. > > > > > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> > > > --- > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- > > > 2 files changed, 100 insertions(+), 1 deletion(-) > > > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > index 353d7a967b..86751b45f8 100644 > > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > @@ -65,11 +65,15 @@ > > > > > > [Pcd] > > > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > > > > > [Guids] > > > + gBootDiscoveryPolicyMgrFormsetGuid > > > gEdkiiNonDiscoverableEhciDeviceGuid > > > gEdkiiNonDiscoverableUhciDeviceGuid > > > gEdkiiNonDiscoverableXhciDeviceGuid > > > + gEfiBootManagerPolicyNetworkGuid > > > + gEfiBootManagerPolicyConnectAllGuid > > > gEfiFileInfoGuid > > > gEfiFileSystemInfoGuid > > > gEfiFileSystemVolumeLabelInfoIdGuid > > > @@ -79,6 +83,7 @@ > > > > > > [Protocols] > > > gEdkiiNonDiscoverableDeviceProtocolGuid > > > + gEfiBootManagerPolicyProtocolGuid > > > gEfiDevicePathProtocolGuid > > > gEfiGraphicsOutputProtocolGuid > > > gEfiLoadedImageProtocolGuid > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > index 5ceb23d822..4332c45bb7 100644 > > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > @@ -2,9 +2,10 @@ > > > Implementation for PlatformBootManagerLib library class interfaces. > > > > > > Copyright (C) 2015-2016, Red Hat, Inc. > > > - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> > > > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> > > > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > > > Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> > > > + Copyright (c) 2021, Semihalf All rights reserved.<BR> > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > @@ -19,6 +20,7 @@ > > > #include <Library/UefiBootManagerLib.h> > > > #include <Library/UefiLib.h> > > > #include <Library/UefiRuntimeServicesTableLib.h> > > > +#include <Protocol/BootManagerPolicy.h> > > > #include <Protocol/DevicePath.h> > > > #include <Protocol/EsrtManagement.h> > > > #include <Protocol/GraphicsOutput.h> > > > @@ -27,6 +29,7 @@ > > > #include <Protocol/PciIo.h> > > > #include <Protocol/PciRootBridgeIo.h> > > > #include <Protocol/PlatformBootManager.h> > > > +#include <Guid/BootDiscoveryPolicy.h> > > > #include <Guid/EventGroup.h> > > > #include <Guid/NonDiscoverableDevice.h> > > > #include <Guid/TtyTerm.h> > > > @@ -703,6 +706,91 @@ HandleCapsules ( > > > > > > #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " > > > > > > +/** > > > + This functions checks the value of BootDiscoverPolicy variable and > > > + connect devices of class specified by that variable. Then it refreshes > > > + Boot order for newly discovered boot device. > > > + > > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > > + not required. > > > + @retval others Return values from GetVariable(), LocateProtocol() > > > + and ConnectDeviceClass(). > > > +--*/ > > > +STATIC > > > +EFI_STATUS > > > +BootDiscoveryPolicyHandler ( > > > + VOID > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + UINT32 DiscoveryPolicy; > > > + UINTN Size; > > > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > > > + EFI_GUID *Class; > > > + > > > + Size = sizeof (DiscoveryPolicy); > > > + Status = gRT->GetVariable ( > > > + BOOT_DISCOVERY_POLICY_VAR, > > > + &gBootDiscoveryPolicyMgrFormsetGuid, > > > + NULL, > > > + &Size, > > > + &DiscoveryPolicy > > > + ); > > > + if (Status == EFI_NOT_FOUND) { > > > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); > > > + if (Status == EFI_NOT_FOUND) { > > > + return EFI_SUCCESS; > > > + } else if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); > > > + } else if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + > > > + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { > > > + return EFI_SUCCESS; > > > + } > > > + > > > + switch (DiscoveryPolicy) { > > > + case BDP_CONNECT_NET: > > > + Class = &gEfiBootManagerPolicyNetworkGuid; > > > + break; > > > + case BDP_CONNECT_ALL: > > > + Class = &gEfiBootManagerPolicyConnectAllGuid; > > > + break; > > > + default: > > > + DEBUG (( > > > + DEBUG_INFO, > > > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", > > > + __FUNCTION__, > > > + DiscoveryPolicy > > > + )); > > > + return EFI_SUCCESS; > > > + } > > > + > > > + Status = gBS->LocateProtocol ( > > > + &gEfiBootManagerPolicyProtocolGuid, > > > + NULL, > > > + (VOID **)&BMPolicy > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." > > > + "Driver connect will be skipped.\n", __FUNCTION__)); > > > + return Status; > > > + } > > > + > > > + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); > > > + if (EFI_ERROR (Status)){ > > > + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); > > > + return Status; > > > + } > > > + > > > + EfiBootManagerRefreshAllBootOption(); > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > /** > > > Do the platform specific action after the console is ready > > > Possible things that can be done in PlatformBootManagerAfterConsole: > > > @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( > > > } > > > } > > > > > > + // > > > + // Connect device specified by BootDiscoverPolicy variable and > > > + // refresh Boot order for newly discovered boot devices > > > + // > > > + BootDiscoveryPolicyHandler (); > > > + > > > // > > > // On ARM, there is currently no reason to use the phased capsule > > > // update approach where some capsules are dispatched before EndOfDxe > > > -- > > > 2.25.1 > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. 2021-08-11 11:57 ` Ard Biesheuvel @ 2021-08-11 12:07 ` Marcin Wojtas 0 siblings, 0 replies; 7+ messages in thread From: Marcin Wojtas @ 2021-08-11 12:07 UTC (permalink / raw) To: Ard Biesheuvel Cc: Sunny Wang, Grzegorz Bernacki, edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Samer El-Haj-Mahmoud, upstream@semihalf.com Hi Ard, śr., 11 sie 2021 o 13:57 Ard Biesheuvel <ardb@kernel.org> napisał(a): > > On Wed, 11 Aug 2021 at 13:57, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Tue, 10 Aug 2021 at 17:23, Sunny Wang <Sunny.Wang@arm.com> wrote: > > > > > > Hi Ard, > > > > > > Yeah, this is a good point. Greg and I offline discussed this as well. > > > If we don't miss anything, only applying this patch without platform changes should be fine. There should be no behavior change. I added some details below for your reference. > > > - if the platform doesn’t add PcdBootDiscoveryPolicy to platform's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPolicyHandler() would just get PcdBootDiscoveryPolicy default value and do nothing. > > > - if the platform doesn't include BootManagerPolicyDxe driver or produce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just return with only printing a message to remind users/developers that Boot Discovery Policy doesn't work (driver connect will be skipped). > > > > > > > OK, thanks for double checking. > > > > I tried to submit this but I get CI errors: > > https://github.com/tianocore/edk2/pull/1895 > > Thanks, Greg will take care of it after the weekend. Best regards, Marcin > > > > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel <ardb@kernel.org> > > > Sent: Friday, August 6, 2021 9:45 PM > > > To: Grzegorz Bernacki <gjb@semihalf.com> > > > Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Marcin Wojtas <mw@semihalf.com>; upstream@semihalf.com > > > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. > > > > > > On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote: > > > > > > > > This commit adds code which check BootDiscoveryPolicy variable and > > > > calls Boot Policy Manager Protocol to connect device specified by > > > > the variable. To enable that mechanism for platform > > > > EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be > > > > added to DSC file and BootDiscoveryPolicyUiLib should be added to > > > > UiApp libraries. > > > > > > > > > > ... or the platform will be broken once we apply this patch, right? If > > > so, please propose patches for all platforms in edk2-platforms that > > > use this library - we can't just break them. > > > > > > > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com> > > > > --- > > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 + > > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 96 +++++++++++++++++++- > > > > 2 files changed, 100 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > > index 353d7a967b..86751b45f8 100644 > > > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > > @@ -65,11 +65,15 @@ > > > > > > > > [Pcd] > > > > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > > > > > > > [Guids] > > > > + gBootDiscoveryPolicyMgrFormsetGuid > > > > gEdkiiNonDiscoverableEhciDeviceGuid > > > > gEdkiiNonDiscoverableUhciDeviceGuid > > > > gEdkiiNonDiscoverableXhciDeviceGuid > > > > + gEfiBootManagerPolicyNetworkGuid > > > > + gEfiBootManagerPolicyConnectAllGuid > > > > gEfiFileInfoGuid > > > > gEfiFileSystemInfoGuid > > > > gEfiFileSystemVolumeLabelInfoIdGuid > > > > @@ -79,6 +83,7 @@ > > > > > > > > [Protocols] > > > > gEdkiiNonDiscoverableDeviceProtocolGuid > > > > + gEfiBootManagerPolicyProtocolGuid > > > > gEfiDevicePathProtocolGuid > > > > gEfiGraphicsOutputProtocolGuid > > > > gEfiLoadedImageProtocolGuid > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > > index 5ceb23d822..4332c45bb7 100644 > > > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > > > @@ -2,9 +2,10 @@ > > > > Implementation for PlatformBootManagerLib library class interfaces. > > > > > > > > Copyright (C) 2015-2016, Red Hat, Inc. > > > > - Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR> > > > > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR> > > > > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > > > > Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR> > > > > + Copyright (c) 2021, Semihalf All rights reserved.<BR> > > > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > @@ -19,6 +20,7 @@ > > > > #include <Library/UefiBootManagerLib.h> > > > > #include <Library/UefiLib.h> > > > > #include <Library/UefiRuntimeServicesTableLib.h> > > > > +#include <Protocol/BootManagerPolicy.h> > > > > #include <Protocol/DevicePath.h> > > > > #include <Protocol/EsrtManagement.h> > > > > #include <Protocol/GraphicsOutput.h> > > > > @@ -27,6 +29,7 @@ > > > > #include <Protocol/PciIo.h> > > > > #include <Protocol/PciRootBridgeIo.h> > > > > #include <Protocol/PlatformBootManager.h> > > > > +#include <Guid/BootDiscoveryPolicy.h> > > > > #include <Guid/EventGroup.h> > > > > #include <Guid/NonDiscoverableDevice.h> > > > > #include <Guid/TtyTerm.h> > > > > @@ -703,6 +706,91 @@ HandleCapsules ( > > > > > > > > #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " > > > > > > > > +/** > > > > + This functions checks the value of BootDiscoverPolicy variable and > > > > + connect devices of class specified by that variable. Then it refreshes > > > > + Boot order for newly discovered boot device. > > > > + > > > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > > > + not required. > > > > + @retval others Return values from GetVariable(), LocateProtocol() > > > > + and ConnectDeviceClass(). > > > > +--*/ > > > > +STATIC > > > > +EFI_STATUS > > > > +BootDiscoveryPolicyHandler ( > > > > + VOID > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + UINT32 DiscoveryPolicy; > > > > + UINTN Size; > > > > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > > > > + EFI_GUID *Class; > > > > + > > > > + Size = sizeof (DiscoveryPolicy); > > > > + Status = gRT->GetVariable ( > > > > + BOOT_DISCOVERY_POLICY_VAR, > > > > + &gBootDiscoveryPolicyMgrFormsetGuid, > > > > + NULL, > > > > + &Size, > > > > + &DiscoveryPolicy > > > > + ); > > > > + if (Status == EFI_NOT_FOUND) { > > > > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy)); > > > > + if (Status == EFI_NOT_FOUND) { > > > > + return EFI_SUCCESS; > > > > + } else if (EFI_ERROR (Status)) { > > > > + return Status; > > > > + } > > > > + DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy); > > > > + } else if (EFI_ERROR (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) { > > > > + return EFI_SUCCESS; > > > > + } > > > > + > > > > + switch (DiscoveryPolicy) { > > > > + case BDP_CONNECT_NET: > > > > + Class = &gEfiBootManagerPolicyNetworkGuid; > > > > + break; > > > > + case BDP_CONNECT_ALL: > > > > + Class = &gEfiBootManagerPolicyConnectAllGuid; > > > > + break; > > > > + default: > > > > + DEBUG (( > > > > + DEBUG_INFO, > > > > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n", > > > > + __FUNCTION__, > > > > + DiscoveryPolicy > > > > + )); > > > > + return EFI_SUCCESS; > > > > + } > > > > + > > > > + Status = gBS->LocateProtocol ( > > > > + &gEfiBootManagerPolicyProtocolGuid, > > > > + NULL, > > > > + (VOID **)&BMPolicy > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid." > > > > + "Driver connect will be skipped.\n", __FUNCTION__)); > > > > + return Status; > > > > + } > > > > + > > > > + Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class); > > > > + if (EFI_ERROR (Status)){ > > > > + DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status)); > > > > + return Status; > > > > + } > > > > + > > > > + EfiBootManagerRefreshAllBootOption(); > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > /** > > > > Do the platform specific action after the console is ready > > > > Possible things that can be done in PlatformBootManagerAfterConsole: > > > > @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole ( > > > > } > > > > } > > > > > > > > + // > > > > + // Connect device specified by BootDiscoverPolicy variable and > > > > + // refresh Boot order for newly discovered boot devices > > > > + // > > > > + BootDiscoveryPolicyHandler (); > > > > + > > > > // > > > > // On ARM, there is currently no reason to use the phased capsule > > > > // update approach where some capsules are dispatched before EndOfDxe > > > > -- > > > > 2.25.1 > > > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-11 12:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-08-06 8:30 [PATCH] ArmPkg: Enable boot discovery policy for ARM package Grzegorz Bernacki 2021-08-06 9:02 ` Sunny Wang 2021-08-06 13:44 ` Ard Biesheuvel 2021-08-10 15:23 ` Sunny Wang 2021-08-11 11:57 ` Ard Biesheuvel 2021-08-11 11:57 ` Ard Biesheuvel 2021-08-11 12:07 ` Marcin Wojtas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox