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.6526.1628683049737313985 for ; Wed, 11 Aug 2021 04:57:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=rTv8Qu3s; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id D730860FDA for ; Wed, 11 Aug 2021 11:57:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628683048; bh=LxexB5RA5v9xBCmtdz3KyH0fyqYcwhcq+j8CdoxoTyc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=rTv8Qu3sJk2aS/W3AmBHreB25Shug6rf3HLv5+ZYRQqMrUn+acxDT51aUbZhhCUvu Pj9MHuI1ztuigIsZ1SXG0NiDDQo/NpuqYwuhHelyIlnp9q0ujOJ0kekpHPIPrtL8r5 k505z6XfOyRFnt1Hu2usp4FEz3yejs/0xDbT54fwQ8Xavap32FToWxq4kc4HifHXuL YFqKgsh4C9a8MC1gT5vGzE+wefT6PI++NgeFClo3If2K/5UseaZkSA5diLP4NK31ck Hoqt5jRzPFkJrSjk9186Dj511aNgMdFZQz28iwR/bQjVdCNlOK0c3uMCIqFJhGbUlU 7a9/SslZZx6gQ== Received: by mail-oi1-f173.google.com with SMTP id o20so4109977oiw.12 for ; Wed, 11 Aug 2021 04:57:28 -0700 (PDT) X-Gm-Message-State: AOAM5311BY/qpd3HX82ARBBk49NXI14ZHrlO8Ap6tVCIREzyO/gj/xiq oN59WxgyTTaKfgePD2NiLLGNap23YvGLTvvWrEQ= X-Google-Smtp-Source: ABdhPJyvppN0HqV1+BHwULTfyoXSItg8gFyR2fq1yAoQdDqKtezjnpO9V0Fz89//UuPF1LFSCD4BdU0ASk22wRTckPs= X-Received: by 2002:a05:6808:2219:: with SMTP id bd25mr24009916oib.33.1628683048176; Wed, 11 Aug 2021 04:57:28 -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:16 +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 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 chan= ges should be fine. There should be no behavior change. I added some detail= s below for your reference. > - if the platform doesn=E2=80=99t add PcdBootDiscoveryPolicy to platfor= m's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPolicy= Handler() would just get PcdBootDiscoveryPolicy default value and do nothin= g. > - if the platform doesn't include BootManagerPolicyDxe driver or produc= e EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just ret= urn with only printing a message to remind users/developers that Boot Disco= very 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 > Sent: Friday, August 6, 2021 9:45 PM > To: Grzegorz Bernacki > Cc: edk2-devel-groups-io ; Leif Lindholm ; Ard Biesheuvel ; Samer El-Haj-Mahmoud= ; Sunny Wang ; Marcin Wo= jtas ; upstream@semihalf.com > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package= . > > 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 | 96 = +++++++++++++++++++- > > 2 files changed, 100 insertions(+), 1 deletion(-) > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerL= ib.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/ArmPk= g/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 refresh= es > > + Boot order for newly discovered boot device. > > + > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > + not required. > > + @retval others Return values from GetVariable(), LocateProtoc= ol() > > + 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 (PcdBootDis= coveryPolicy)); > > + 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 Discovery= 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 gEfiBootManagerPolicyPr= otocolGuid." > > + "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", __F= UNCTION__, 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 EndOfDx= e > > -- > > 2.25.1 > > > IMPORTANT NOTICE: The contents of this email and any attachments are conf= idential and may also be privileged. If you are not the intended recipient,= please notify the sender immediately and do not disclose the contents to a= ny other person, use it for any purpose, or store or copy the information i= n any medium. Thank you.