From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f50.google.com (mail-qv1-f50.google.com [209.85.219.50]) by mx.groups.io with SMTP id smtpd.web12.8790.1625824031873928866 for ; Fri, 09 Jul 2021 02:47:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=z7zk/GRh; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.219.50, mailfrom: gjb@semihalf.com) Received: by mail-qv1-f50.google.com with SMTP id i4so3776553qvq.10 for ; Fri, 09 Jul 2021 02:47:11 -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=G8YvtYILvvWqe9KmBXG4+aCyDW+TnGdIxjfVd1lBgnE=; b=z7zk/GRh8+w2tA79OPh5G9vlJvzns7gdMk0aAH4DDiJ3oE5VsnBTYMpqDjJqDJkXj6 ksEfjtj5wTWaev3G39fpzPwI6pBFHdx/1rzyhq5JsEJUVeuj+CsmhhLz15FSCt1lm0V6 zKnaeXyYsnLYWt+7UwqMkbqJI208bdzpqTGzWjpL8uSTc/1R07h81WrmpmAsdiS8xqrJ Ryhs2u7Plaa/C2PkReieqk0N0Y3M2lsMiaSZSxyRDYq1c1+ikIsFL+1tXmCAgCF9r4r7 xq+Z7/nXDxGR2cMsNuwi8MZXhQ86GFPiZarz2BDigtR508Hl9zHlTTs7787gVkNEIFyn 8p9A== 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=G8YvtYILvvWqe9KmBXG4+aCyDW+TnGdIxjfVd1lBgnE=; b=aQWXYV9vxqyt134JlF1+115CuBWAi+kxxY8tRlLDb5ANG1LLcgDgYaxhpHr3g1VZh1 3yeWR6Cr/A6fc4xffYv33Q9miYKVFUuNSKcVosx5bANHRFPEi0fM8UgnuBQnsSel1RVy ll5LnA4nwxebqUdHd5+rut6CAxw+n7y08NZPzwS1KcfD/taHa39Bzdb5NB8AiJsNnPnN z7xYlRSsE1tioikiruWzUoejFRtHiF9b+jK+G4ixMz0HCrrGAO/WwMk6eyyzuUEbvW94 gB3snqF4pEGR05LjZAlwIouTijKA7nd5TB+RpeDKBExtmIoaRd2UkGFF26bRoSnnTlsU /ejA== X-Gm-Message-State: AOAM5312eVSdONmqXxmmPzA7QkTriV8SqnYtcK/waEiS8K29Oae0pNib GpZBD1n2DheJBUiGNKyJnTV/Wxq0MoBKHZCRW4suekrPaMI= X-Google-Smtp-Source: ABdhPJzek+jvjKlpxhtlU891yT8Se364wntIL8jJLZWuSDfUQ1eYU9UMW2ST2yLEC+lCV3pGDo6++CeO+IAnuOttFa0= X-Received: by 2002:a0c:d644:: with SMTP id e4mr34938097qvj.45.1625824030859; Fri, 09 Jul 2021 02:47:10 -0700 (PDT) MIME-Version: 1.0 References: <20210706104432.1239133-1-gjb@semihalf.com> <20210706104432.1239133-3-gjb@semihalf.com> In-Reply-To: From: "Grzegorz Bernacki" Date: Fri, 9 Jul 2021 11:47:00 +0200 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy. To: devel@edk2.groups.io, zhichao.gao@intel.com Cc: "leif@nuviainc.com" , "ardb+tianocore@kernel.org" , "Samer.El-Haj-Mahmoud@arm.com" , "sunny.Wang@arm.com" , "mw@semihalf.com" , "upstream@semihalf.com" , "pete@akeo.ie" , "Wang, Jian J" , "Wu, Hao A" , "Bi, Dandan" , "Dong, Eric" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Yes, you're right, I will send a corrected version of the patch. Thanks a lot, greg czw., 8 lip 2021 o 10:15 Gao, Zhichao napisa=C5=82= (a): > > See below comments. > > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of > > Grzegorz Bernacki > > Sent: Tuesday, July 6, 2021 6:45 PM > > To: devel@edk2.groups.io > > Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj- > > Mahmoud@arm.com; sunny.Wang@arm.com; mw@semihalf.com; > > upstream@semihalf.com; pete@akeo.ie; Wang, Jian J > > ; Wu, Hao A ; Bi, Dandan > > ; Dong, Eric ; Grzegorz > > Bernacki ; Sunny Wang > > Subject: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/Raspberry= Pi: > > Enable Boot Discovery Policy. > > > > This commit modify platform boot to check the value of BootDiscoveryPo= licy > > variable and use BootPolicyManager Protocol to connect devices specifi= ed by > > the variable. > > > > Signed-off-by: Grzegorz Bernacki > > Reviewed-by: Sunny Wang > > --- > > Platform/RaspberryPi/RPi4/RPi4.dsc = | 3 + > > Platform/RaspberryPi/RPi4/RPi4.fdf = | 1 + > > > > Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManag > > erLib.inf | 5 ++ > > Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > > | 90 ++++++++++++++++++++ > > 4 files changed, 99 insertions(+) > > > > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc > > b/Platform/RaspberryPi/RPi4/RPi4.dsc > > index fd73c4d14b..8b9beac64a 100644 > > --- a/Platform/RaspberryPi/RPi4/RPi4.dsc > > +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc > > @@ -555,6 +555,7 @@ > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|L"Columns"|gRasp > > berryPiTokenSpaceGuid|0x0|80 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|L"Rows"|gRaspb > > erryPiTokenSpaceGuid|0x0|25 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|L"Rows"|gRaspberryPi > > TokenSpaceGuid|0x0|25 > > + > > + > > gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy|L"BootDiscove > > ryP > > + olicy"|gBootDiscoveryPolicyMgrFormsetGuid|0 > > > > [PcdsDynamicDefault.common] > > # > > @@ -682,6 +683,7 @@ > > # > > # Bds > > # > > + > > MdeModulePkg/Universal/BootManagerPolicyDxe/BootManagerPolicyDxe.i > > nf > > MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > > MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf > > MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf > > @@ -690,6 +692,7 @@ > > Platform/RaspberryPi/Drivers/LogoDxe/LogoDxe.inf > > MdeModulePkg/Application/UiApp/UiApp.inf { > > > > + > > + > > NULL|MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolic > > y > > + UiLib.inf > > > > NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > > NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf > > > > NULL|Platform/RaspberryPi/Library/PlatformUiAppLib/PlatformUiAppLib.in= f > > diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf > > b/Platform/RaspberryPi/RPi4/RPi4.fdf > > index 1e13909a57..371197a93e 100644 > > --- a/Platform/RaspberryPi/RPi4/RPi4.fdf > > +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf > > @@ -253,6 +253,7 @@ READ_LOCK_STATUS =3D TRUE > > # > > # Bds > > # > > + INF > > + > > MdeModulePkg/Universal/BootManagerPolicyDxe/BootManagerPolicyDxe.i > > nf > > INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > > INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf > > INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf > > diff --git > > a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan > > agerLib.inf > > b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan > > agerLib.inf > > index fbf510ab96..4ef2f791ae 100644 > > --- > > a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan > > agerLib.inf > > +++ > > b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa > > +++ nagerLib.inf > > @@ -61,11 +61,13 @@ > > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType > > > > [Pcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy > > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > > gRaspberryPiTokenSpaceGuid.PcdSdIsArasan > > gRaspberryPiTokenSpaceGuid.PcdBootPolicy > > > > [Guids] > > + gBootDiscoveryPolicyMgrFormsetGuid > > gEfiFileInfoGuid > > gEfiFileSystemInfoGuid > > gEfiFileSystemVolumeLabelInfoIdGuid > > @@ -73,8 +75,11 @@ > > gEfiTtyTermGuid > > gUefiShellFileGuid > > gEfiEventExitBootServicesGuid > > + gEfiBootManagerPolicyNetworkGuid > > + gEfiBootManagerPolicyConnectAllGuid > > > > [Protocols] > > + gEfiBootManagerPolicyProtocolGuid > > gEfiDevicePathProtocolGuid > > gEfiGraphicsOutputProtocolGuid > > gEfiLoadedImageProtocolGuid > > diff --git > > a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > > b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > > index d081fdae63..4bfa906921 100644 > > --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c > > @@ -6,6 +6,7 @@ > > * Copyright (c) 2015-2016, Red Hat, Inc. > > * Copyright (c) 2014-2021, ARM Ltd. All rights reserved. > > * Copyright (c) 2004-2016, Intel Corporation. All rights reserved. > > + * Copyright (c) 2021, Semihalf All rights reserved. > > * > > * SPDX-License-Identifier: BSD-2-Clause-Patent > > * > > @@ -19,10 +20,12 @@ > > #include #include > > #include > > +#include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -598,6 +601,88 @@ PlatformBootManagerBeforeConsole ( > > FilterAndProcess (&gEfiUsb2HcProtocolGuid, NULL, Connect); } > > > > +/** > > + Connect device specified by BootDiscoverPolicy variable and refresh > > + Boot order for newly discovered boot device. > > + > > + @retval EFI_SUCCESS Devices connected succesfully or connection > > + not required. > > + @retval others Return values from GetVariable(), LocateProto= col() > > + 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)); > > Why need this Pcd statement? > > > + if (Status =3D=3D EFI_NOT_FOUND) { > > + return EFI_SUCCESS; > > + } else if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + } else if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + if (DiscoveryPolicy =3D=3D BDP_CONNECT_MINIMAL) { > > If the Status is EFI_NOT_FOUND at GetVariable, then the above DiscoveryP= olicy is uninitialized. > Maybe you need to use the Pcd value if GetVariable failed. > > Thanks, > Zhichao > > > + 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 Discover= y > > Policy\n", > > + __FUNCTION__, > > + DiscoveryPolicy > > + )); > > + return EFI_SUCCESS; > > + } > > + > > + Status =3D gBS->LocateProtocol ( > > + &gEfiBootManagerPolicyProtocolGuid, > > + NULL, > > + (VOID **)&BMPolicy > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a - Failed to locate > > gEfiBootManagerPolicyProtocolGuid - %r\n", __FUNCTION__, Status)); > > + 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 PlatformBootManagerAfterConsole= : > > @@ -644,6 +729,11 @@ PlatformBootManagerAfterConsole ( > > DEBUG ((DEBUG_INFO, "Boot Policy is Fast Boot. Skip connecting al= l > > devices\n")); > > } > > > > + Status =3D BootDiscoveryPolicyHandler (); if (EFI_ERROR(Status)) { > > + DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n", > > + Status)); } > > + > > Status =3D gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, > > (VOID**)&EsrtManagement); > > if (!EFI_ERROR (Status)) { > > EsrtManagement->SyncEsrtFmp (); > > -- > > 2.25.1 > > > > > > > > > > > > > >=20 > >