From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f179.google.com (mail-qt1-f179.google.com [209.85.160.179]) by mx.groups.io with SMTP id smtpd.web12.6562.1628683689104665070 for ; Wed, 11 Aug 2021 05:08:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=sgDdPMyQ; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.160.179, mailfrom: mw@semihalf.com) Received: by mail-qt1-f179.google.com with SMTP id y9so1833393qtv.7 for ; Wed, 11 Aug 2021 05:08:08 -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=WXLe1Fxmse9oGGXB3ZmhI1b5rmk8OGeu+wdO+8cCiAs=; b=sgDdPMyQr06+KFhkL4JOP2mBmFYERGoyXYbJhC5khYE2ijDJyVz0ePr1OL5zDyJ+rL jyETcE53AKd4J5Am/zRQW574dhEuAIHfW5QLDTelg4coFB9ZI2YXXs9ceGM+O/kbJkiD 9xlQVfN5U+8OQi4sJhfS71pAGJKt0cNxqjy1orpwmLqrYf5yT4WKPBG0BBMDKNrTJd8P O7awVMW7X/3BbRJgVIFJRqlHkvLC5xxzwaQ1W2VbRUGztPP/p7EXOiJhRPBGegvrzZLl wUapJH7v9QBdE5HTTZ3+Sk+0qEQ8LmBRwrKmmyYIuUrt8LbFMd41zlsD3QH+DooHJx9b IaZQ== 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=WXLe1Fxmse9oGGXB3ZmhI1b5rmk8OGeu+wdO+8cCiAs=; b=iAq2QVuBF/OETpk+WQoJ9E+q/fKL6uC3C1Q5hI4LA4muy5r5epgEJRGAHQIvJGlzEO vChq5I7/AEM04rctZ94kzbnyYn63ZdYPm85cq5zNJOnrNGS2GQeOQtmOTj8LrQbWlJrN chuz6BoyKWpVpCdOHGP59JxoU6w+p0tHogEbXNzce15Eu4W/c1zzLxuWasLazKMGsJc6 7YWNfqmnuLPvaCOUJabxdO5TTuuyQLA3a0fJ3BPlMPzA8AM1AH58xi4MLD8+MTyKIpeX W0pBoPaLKcJDc4YX4BTKU78C8UY5yRgbFrUtNSfoty/73RX9ShN4FAfg8xopOFhD51ES DbbA== X-Gm-Message-State: AOAM531SnQSuNcFsn+FQnz47sKRudfdBhtujcfEaOIcrtXEA7VdS7zEm Rh71IVMbuvQ3SWgSmdJ03Vsr6JYaZFbzP63NetplXw== X-Google-Smtp-Source: ABdhPJxtkcQtS4hkQ/KwB2dB4l9tfsPcjTJ4t6GSDGuSJdZ3P1nEcFtpKAjRfvdb7uS8jTaw2YqbEQ7dTQr9jKCHyo4= X-Received: by 2002:ac8:58d3:: with SMTP id u19mr30243181qta.306.1628683688064; Wed, 11 Aug 2021 05:08:08 -0700 (PDT) MIME-Version: 1.0 References: <20210806083026.3295056-1-gjb@semihalf.com> In-Reply-To: From: "Marcin Wojtas" Date: Wed, 11 Aug 2021 14:07:56 +0200 Message-ID: Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package. To: Ard Biesheuvel Cc: Sunny Wang , Grzegorz Bernacki , edk2-devel-groups-io , Leif Lindholm , Ard Biesheuvel , Samer El-Haj-Mahmoud , "upstream@semihalf.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ard, =C5=9Br., 11 sie 2021 o 13:57 Ard Biesheuvel napisa=C5=82= (a): > > 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 = changes should be fine. There should be no behavior change. I added some de= tails below for your reference. > > > - if the platform doesn=E2=80=99t add PcdBootDiscoveryPolicy to pla= tform's dsc file and add BootDiscoveryPolicyUiLib to UiApp. BootDiscoveryPo= licyHandler() would just get PcdBootDiscoveryPolicy default value and do no= thing. > > > - if the platform doesn't include BootManagerPolicyDxe driver or pr= oduce EfiBootManagerPolicyProtocol, BootDiscoveryPolicyHandler() would just= return with only printing a message to remind users/developers that Boot D= iscovery 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 > > > Sent: Friday, August 6, 2021 9:45 PM > > > To: Grzegorz Bernacki > > > Cc: edk2-devel-groups-io ; Leif Lindholm ; Ard Biesheuvel ; Samer El-Haj-Mah= moud ; Sunny Wang ; Marci= n Wojtas ; upstream@semihalf.com > > > Subject: Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM pac= kage. > > > > > > On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki wro= te: > > > > > > > > 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? I= f > > > 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/PlatformBootMana= gerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf > > > > 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/A= rmPkg/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 interfac= es. > > > > > > > > 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 reserve= d.
> > > > 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 a= nd > > > > + connect devices of class specified by that variable. Then it ref= reshes > > > > + Boot order for newly discovered boot device. > > > > + > > > > + @retval EFI_SUCCESS Devices connected succesfully or connectio= n > > > > + not required. > > > > + @retval others Return values from GetVariable(), LocatePr= otocol() > > > > + 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 (PcdBoo= tDiscoveryPolicy)); > > > > + 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 Disco= very 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 gEfiBootManagerPoli= cyProtocolGuid." > > > > + "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 PlatformBootManagerAfterCons= ole: > > > > @@ -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 capsul= e > > > > // update approach where some capsules are dispatched before End= OfDxe > > > > -- > > > > 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 recipi= ent, 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 informati= on in any medium. Thank you.