Hi Ard,

śr., 11 sie 2021 o 13:57 Ard Biesheuvel <a...@kernel.org> napisał(a):
>
> On Wed, 11 Aug 2021 at 13:57, Ard Biesheuvel <a...@kernel.org> wrote:
> >
> > 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:
>
> 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 <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 (#79109): https://edk2.groups.io/g/devel/message/79109
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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to