From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by mx.groups.io with SMTP id smtpd.web12.20756.1629704650281104367 for ; Mon, 23 Aug 2021 00:44:10 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=o7gbJEA1; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.219.54, mailfrom: gjb@semihalf.com) Received: by mail-qv1-f54.google.com with SMTP id v1so9157583qva.7 for ; Mon, 23 Aug 2021 00:44:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=5NGrtn8qBpjt6iN8CigxB/rUfM4Q/cp/FGzcR2kMjwg=; b=o7gbJEA1RLb6YYyQmMpAyfZbEbSu7ZlTFlgMz8nYraLNA2ziUwOVM6rRLFB6Xn9KzJ dCXDgE2R7S4INLlFqOO1iOxTmVtlRovvLCZAix8RNEDxdBwuIyImS4kYJ+cVWbP2oTJp ECQmzVIvx2JOR09lt9omJCWv41c0NHVGrQYFZjf3p8VzwseX4oS+kQ3grb/f929Za1Fd CyC1PwAU2zaAlXSPXteLLOf0j6yhJ37dzHIEGHWR2iJTIp1oZCIadFMeMstxdEv4zY9Y xiPkmfaoKjy4SJZEZbims7JVYY0Op4CbzF6+FvoE0EGmKAixyJ405rkBwjvNP4QMBOVX X6/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=5NGrtn8qBpjt6iN8CigxB/rUfM4Q/cp/FGzcR2kMjwg=; b=ll8Pnt8ss0bgQDC7DaoW0iWBYqqSIx3MTJTCgk68fNp3UTjkaVw4I3qVsYzYKkagG2 YwiG8iEuOt/Dq9Ls17z+m3PHj3qO1JEgJqWwkCes/+w+xL+oN0VlK+f4d2wVcDmcvXTW jFlxRVpv5P3kvhnBtA+cSX9rI6Z3Ngc9KSyfK8Herndydr2efofkjroI1Kw+b/lhErol dRoCxlPICiIsoSop42N5uTiYU/Ws/86LRbSm8dt4e5gexNRzwlF/ZA27xUKnSn/cXc4u NCiq6XprOC0L/M06tCUvK33WHwDuc3Uv9vvN2cb4B9BuQ5wH/bOgBE3/n6XKdghagzx6 tOhg== X-Gm-Message-State: AOAM531sfttcySctD1ID8tHDiesKV5ojQFwU7fBGctgDVIdO5ZQSGwip 0ilB0kxnW7koWFxlRlWOz1iXeVb/Lwd5vToJ86YLGw== X-Google-Smtp-Source: ABdhPJyni82lEj5od/8DA8hoL4KPhkPOkDcLXJhJUpOERdVPCMPG0qUAmw8QU0nzaYpWWjYks1N8T7E2njcJsmtFwKI= X-Received: by 2002:a05:6214:142a:: with SMTP id o10mr9519459qvx.61.1629704649451; Mon, 23 Aug 2021 00:44:09 -0700 (PDT) MIME-Version: 1.0 References: <20210816100927.3548826-1-gjb@semihalf.com> <40c62f50-ee44-ad2d-85ef-e1286f43f69a@arm.com> In-Reply-To: <40c62f50-ee44-ad2d-85ef-e1286f43f69a@arm.com> From: "Grzegorz Bernacki" Date: Mon, 23 Aug 2021 09:43:58 +0200 Message-ID: Subject: Re: [edk2-platforms PATCH v2] ArmPkg: Enable boot discovery policy for ARM package. To: Sami Mujawar Cc: Samer El-Haj-Mahmoud , "devel@edk2.groups.io" , "leif@nuviainc.com" , "ardb+tianocore@kernel.org" , Sunny Wang , "mw@semihalf.com" , "upstream@semihalf.com" , nd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Sami, Thanks for the comments, I will send a new version of patch soon. thanks, greg pt., 20 sie 2021 o 14:04 Sami Mujawar napisa=C5=82(a= ): > > Hi Grzegorz, > > Thank you for this patch. I have a few minor suggestions, otherwise this = patch looks good to me. > > I have tested this patch on FVP and Juno and both platforms work fine. > > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > > On 18/08/2021 08:43 PM, Samer El-Haj-Mahmoud wrote: > > + Sami for ArmPkg review > > -----Original Message----- > From: Grzegorz Bernacki > Sent: Monday, August 16, 2021 6:09 AM > To: devel@edk2.groups.io > Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud > ; Sunny Wang > ; mw@semihalf.com; upstream@semihalf.com; > Grzegorz Bernacki > Subject: [edk2-platforms PATCH v2] ArmPkg: Enable boot discovery policy f= or > 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 > --- > 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..ef5c323743 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 refreshes > + Boot order for newly discovered boot device. > + > + @retval EFI_SUCCESS Devices connected successfully 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 =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 > (PcdBootDiscoveryPolicy)); > + if (Status =3D=3D EFI_NOT_FOUND) { > + return EFI_SUCCESS; > + } else if (EFI_ERROR (Status)) { > + return Status; > + } > + DiscoveryPolicy =3D PcdGet32 (PcdBootDiscoveryPolicy); > > [SAMI] Can 'DiscoveryPolicy' be initialised a few lines above and used in= the call to PcdSet32S ()? > > + } 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 > gEfiBootManagerPolicyProtocolGuid." > + "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(); > > [SAMI] Please add a space after the function name and the opening parenth= esis. > > + > + 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 > >