From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web09.6532.1628683070761575946 for ; Wed, 11 Aug 2021 04:57:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=G3hreCzg; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 3F98960F56 for ; Wed, 11 Aug 2021 11:57:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628683070; bh=Y+eKTSeVnaRAs2xgQi0GtCB4OOv9m/KlgMio+j3BdOQ=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=G3hreCzgnZqIsn9ep2dzFTxzaVTapB0SYETsO/fG7JH7lGUKKGmw/TymoS5k8JkxH +0YJDftrVTLjJBYtMNhe/t+aHIZkMsROsrjv+Tdf9uC0LZerHtiQ0jwBdaU/yavTkO eRsTM7JWNS8r9Ui3Brn5RweVn+EZzazpvnKPJZ9EYV8f4bs+Lu1X1IaMLyf7tI4aJW R9MB3NTxK7IrrmEeVWYSCe1Uj3fQlgcBze06Ac8s0TqqfXAyzMcFGJpP0vOX2mrj6I uChHHSJLOHY0nC218TlDx6AlAwW/GfFTOgvm6tvo6PrToJrepD4Sk2Hsmwc97LZIVE 0rACci/N48ZSg== Received: by mail-oi1-f169.google.com with SMTP id be20so4148109oib.8 for ; Wed, 11 Aug 2021 04:57:50 -0700 (PDT) X-Gm-Message-State: AOAM533eCEKgkOoXJtH7RFaryBAfdYEfU+5krH6SL0a4tY/FSPoEC4Bg i2TYG1U13m78TsucVq0qnT4w+bsj+zq8GN6Cz7s= X-Google-Smtp-Source: ABdhPJx3/22HPa7TUW9lymfvSPVRQP1aLHUKJX7C8enq07Zz+CpjNivgPXACTMaq4vjmGfV0yek2NuvREWBdjkrmhIQ= X-Received: by 2002:aca:dd89:: with SMTP id u131mr7213442oig.47.1628683069651; Wed, 11 Aug 2021 04:57:49 -0700 (PDT) MIME-Version: 1.0 References: <20210806083026.3295056-1-gjb@semihalf.com> In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 11 Aug 2021 13:57:38 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. To: Sunny Wang Cc: Grzegorz Bernacki , edk2-devel-groups-io , Leif Lindholm , Ard Biesheuvel , Samer El-Haj-Mahmoud , Marcin Wojtas , "upstream@semihalf.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 11 Aug 2021 at 13:57, Ard Biesheuvel wrote: > > On Tue, 10 Aug 2021 at 17:23, Sunny Wang 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 ch= anges should be fine. There should be no behavior change. I added some deta= ils below for your reference. > > - if the platform doesn=E2=80=99t add PcdBootDiscoveryPolicy to platf= orm's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPoli= cyHandler() would just get PcdBootDiscoveryPolicy default value and do noth= ing. > > - if the platform doesn't include BootManagerPolicyDxe driver or prod= uce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just r= eturn with only printing a message to remind users/developers that Boot Dis= covery 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 > > Sent: Friday, August 6, 2021 9:45 PM > > To: Grzegorz Bernacki > > Cc: edk2-devel-groups-io ; Leif Lindholm ; Ard Biesheuvel ; Samer El-Haj-Mahmo= ud ; Sunny Wang ; Marcin = Wojtas ; upstream@semihalf.com > > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM packa= ge. > > > > On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki 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 > > > --- > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | = 5 + > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 9= 6 +++++++++++++++++++- > > > 2 files changed, 100 insertions(+), 1 deletion(-) > > > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManage= rLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > index 353d7a967b..86751b45f8 100644 > > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.in= f > > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.in= f > > > @@ -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/Arm= Pkg/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.
> > > + Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.
> > > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.=
> > > Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > + Copyright (c) 2021, Semihalf All rights reserved.
> > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > @@ -19,6 +20,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -27,6 +29,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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 refre= shes > > > + Boot order for newly discovered boot device. > > > + > > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > > + not required. > > > + @retval others Return values from GetVariable(), LocateProt= ocol() > > > + and ConnectDeviceClass(). > > > +--*/ > > > +STATIC > > > +EFI_STATUS > > > +BootDiscoveryPolicyHandler ( > > > + VOID > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + UINT32 DiscoveryPolicy; > > > + UINTN Size; > > > + EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy; > > > + EFI_GUID *Class; > > > + > > > + Size =3D sizeof (DiscoveryPolicy); > > > + Status =3D gRT->GetVariable ( > > > + BOOT_DISCOVERY_POLICY_VAR, > > > + &gBootDiscoveryPolicyMgrFormsetGuid, > > > + NULL, > > > + &Size, > > > + &DiscoveryPolicy > > > + ); > > > + if (Status =3D=3D EFI_NOT_FOUND) { > > > + Status =3D PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootD= iscoveryPolicy)); > > > + if (Status =3D=3D EFI_NOT_FOUND) { > > > + return EFI_SUCCESS; > > > + } else if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + DiscoveryPolicy =3D PcdGet32 (PcdBootDiscoveryPolicy); > > > + } else if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + > > > + if (DiscoveryPolicy =3D=3D BDP_CONNECT_MINIMAL) { > > > + return EFI_SUCCESS; > > > + } > > > + > > > + switch (DiscoveryPolicy) { > > > + case BDP_CONNECT_NET: > > > + Class =3D &gEfiBootManagerPolicyNetworkGuid; > > > + break; > > > + case BDP_CONNECT_ALL: > > > + Class =3D &gEfiBootManagerPolicyConnectAllGuid; > > > + break; > > > + default: > > > + DEBUG (( > > > + DEBUG_INFO, > > > + "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discove= ry Policy\n", > > > + __FUNCTION__, > > > + DiscoveryPolicy > > > + )); > > > + return EFI_SUCCESS; > > > + } > > > + > > > + Status =3D gBS->LocateProtocol ( > > > + &gEfiBootManagerPolicyProtocolGuid, > > > + NULL, > > > + (VOID **)&BMPolicy > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicy= ProtocolGuid." > > > + "Driver connect will be skipped.\n", __FUNCTION__)); > > > + return Status; > > > + } > > > + > > > + Status =3D 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 PlatformBootManagerAfterConsol= e: > > > @@ -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 EndOf= Dxe > > > -- > > > 2.25.1 > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are co= nfidential and may also be privileged. If you are not the intended recipien= t, 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.