On Tue, 10 Aug 2021 at 17:23, Sunny Wang <sunny.w...@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 <a...@kernel.org>
> Sent: Friday, August 6, 2021 9:45 PM
> To: Grzegorz Bernacki <g...@semihalf.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm
> <l...@nuviainc.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; Samer
> El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Sunny Wang
> <sunny.w...@arm.com>; Marcin Wojtas <m...@semihalf.com>; upstr...@semihalf.com
> Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package.
>
> On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <g...@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 <g...@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.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79107): https://edk2.groups.io/g/devel/message/79107
Mute This Topic: https://groups.io/mt/84704033/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
