[edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
Grzegorz Bernacki
gjb at semihalf.com
Fri Jul 9 09:55:22 UTC 2021
Hi Zhichao,
Setting HII-type PCD causes variable initialization, so if
GetVariable() fails due to variable not being found, it will be
initialized by PcdSet32S() function.
thanks,
greg
czw., 8 lip 2021 o 10:08 Gao, Zhichao <zhichao.gao at intel.com> napisał(a):
>
> See below comments.
>
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > Grzegorz Bernacki
> > Sent: Tuesday, July 6, 2021 6:45 PM
> > To: devel at edk2.groups.io
> > Cc: leif at nuviainc.com; ardb+tianocore at kernel.org; Samer.El-Haj-
> > Mahmoud at arm.com; sunny.Wang at arm.com; mw at semihalf.com;
> > upstream at semihalf.com; pete at akeo.ie; Wang, Jian J
> > <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Bi, Dandan
> > <dandan.bi at intel.com>; Dong, Eric <eric.dong at intel.com>; Grzegorz
> > Bernacki <gjb at semihalf.com>; Sunny Wang <sunny.wang at arm.com>
> > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add
> > BootDiscoveryPolicyUiLib.
> >
> > This library extends Boot Maintenance Menu and allows to select Boot
> > Discovery Policy. When choice is made BootDiscoveryPolicy variable is set.
> > Platform code can use this variable to decide which class of device shall be
> > connected.
> >
> > Signed-off-by: Grzegorz Bernacki <gjb at semihalf.com>
> > Reviewed-by: Sunny Wang <sunny.wang at arm.com>
> > ---
> > MdeModulePkg/MdeModulePkg.dec | 6 +
> >
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > inf | 52 +++++++
> > MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h | 22
> > +++
> >
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > c | 160 ++++++++++++++++++++
> >
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > uni | 16 ++
> >
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> > Strings.uni | 29 ++++
> >
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> > Vfr.Vfr | 44 ++++++
> > 7 files changed, 329 insertions(+)
> > create mode 100644
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > inf
> > create mode 100644 MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h
> > create mode 100644
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > c
> > create mode 100644
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> > uni
> > create mode 100644
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> > Strings.uni
> > create mode 100644
> > MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> > Vfr.Vfr
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec index ad84421cf3..4e1c291768
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -425,6 +425,9 @@
> > ## Include/UniversalPayload/SerialPortInfo.h
> > gUniversalPayloadSerialPortInfoGuid = { 0xaa7e190d, 0xbe21, 0x4409,
> > { 0x8e, 0x67, 0xa2, 0xcd, 0xf, 0x61, 0xe1, 0x70 } }
> >
> > + ## GUID used for Boot Discovery Policy FormSet guid and related variables.
> > + gBootDiscoveryPolicyMgrFormsetGuid = { 0x5b6f7107, 0xbb3c, 0x4660, {
> > + 0x92, 0xcd, 0x54, 0x26, 0x90, 0x28, 0x0b, 0xbd } }
> > +
> > [Ppis]
> > ## Include/Ppi/AtaController.h
> > gPeiAtaControllerPpiGuid = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a,
> > 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
> > @@ -1600,6 +1603,9 @@
> > # @Prompt Console Output Row of Text Setup
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x40
> > 00000e
> >
> > + ## Specify the Boot Discovery Policy settings
> > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy|2|UINT32|0x4
> > 0000
> > + 00f
> > +
> > [PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20|UI
> > NT32|0x0001004c
> >
> > diff --git
> > a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.inf
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.inf
> > new file mode 100644
> > index 0000000000..1fb4d43caa
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> > +++ iLib.inf
> > @@ -0,0 +1,52 @@
> > +## @file
> > +# Library for BDS phase to use Boot Discovery Policy # # Copyright
> > +(c) 2021, ARM Ltd. All rights reserved.<BR> # Copyright (c) 2021,
> > +Semihalf All rights reserved.<BR> # SPDX-License-Identifier:
> > +BSD-2-Clause-Patent # ##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = BootDiscoveryPolicyUiLib
> > + MODULE_UNI_FILE = BootDiscoveryPolicyUiLib.uni
> > + FILE_GUID = BE73105A-B13D-4B57-A41A-463DBD15FE10
> > + MODULE_TYPE = DXE_DRIVER
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = NULL|DXE_DRIVER UEFI_APPLICATION
> > + CONSTRUCTOR = BootDiscoveryPolicyUiLibConstructor
> > + DESTRUCTOR = BootDiscoveryPolicyUiLibDestructor
> > +#
> > +# The following information is for reference only and not required by the
> > build tools.
> > +#
> > +# VALID_ARCHITECTURES = IA32 X64 AARCH64
> > +#
> > +
> > +[Sources]
> > + BootDiscoveryPolicyUiLib.c
> > + BootDiscoveryPolicyUiLibStrings.uni
> > + BootDiscoveryPolicyUiLibVfr.Vfr
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > +
> > +[LibraryClasses]
> > + DevicePathLib
> > + BaseLib
> > + UefiRuntimeServicesTableLib
> > + UefiBootServicesTableLib
> > + DebugLib
> > + HiiLib
> > + UefiLib
> > + BaseMemoryLib
> > +
> > +[Guids]
> > + gBootDiscoveryPolicyMgrFormsetGuid
> > +
> > +[Pcd]
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy ##
> > PRODUCES
> > +
> > +[Depex]
> > + gEfiHiiDatabaseProtocolGuid AND gPcdProtocolGuid
> > diff --git a/MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h
> > b/MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h
> > new file mode 100644
> > index 0000000000..8eb0968a16
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h
> > @@ -0,0 +1,22 @@
> > +/** @file
> > + Definition for structure & defines exported by Boot Discovery Policy
> > +UI
> > +
> > + Copyright (c) 2021, ARM Ltd. All rights reserved.<BR> Copyright (c)
> > + 2021, Semihalf All rights reserved.<BR>
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef _BOOT_DISCOVERY_POLICY_UI_LIB_H_ #define
> > +_BOOT_DISCOVERY_POLICY_UI_LIB_H_
> > +
> > +#define BDP_CONNECT_MINIMAL 0 /* Do not connect any additional
> > devices */
> > +#define BDP_CONNECT_NET 1
> > +#define BDP_CONNECT_ALL 2
> > +
> > +#define BOOT_DISCOVERY_POLICY_MGR_FORMSET_GUID { 0x5b6f7107,
> > 0xbb3c,
> > +0x4660, { 0x92, 0xcd, 0x54, 0x26, 0x90, 0x28, 0x0b, 0xbd } }
> > +
> > +#define BOOT_DISCOVERY_POLICY_VAR L"BootDiscoveryPolicy"
> > +
> > +#endif
> > diff --git
> > a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.c
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.c
> > new file mode 100644
> > index 0000000000..6814d0bb8f
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> > +++ iLib.c
> > @@ -0,0 +1,160 @@
> > +/** @file
> > + Boot Discovery Policy UI for Boot Maintenance menu.
> > +
> > + Copyright (c) 2021, ARM Ltd. All rights reserved.<BR> Copyright (c)
> > + 2021, Semihalf All rights reserved.<BR>
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Guid/BootDiscoveryPolicy.h>
> > +#include <Library/UefiDriverEntryPoint.h> #include
> > +<Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DevicePathLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/HiiLib.h>
> > +#include <Library/UefiLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Include/Library/PcdLib.h>
> > +
> > +///
> > +/// HII specific Vendor Device Path definition.
> > +///
> > +typedef struct {
> > + VENDOR_DEVICE_PATH VendorDevicePath;
> > + EFI_DEVICE_PATH_PROTOCOL End;
> > +} HII_VENDOR_DEVICE_PATH;
> > +
> > +extern unsigned char BootDiscoveryPolicyUiLibVfrBin[];
> > +
> > +EFI_HII_HANDLE mBPHiiHandle = NULL;
> > +EFI_HANDLE mBPDriverHandle = NULL;
> > +
> > +STATIC HII_VENDOR_DEVICE_PATH mVendorDevicePath = {
> > + {
> > + {
> > + HARDWARE_DEVICE_PATH,
> > + HW_VENDOR_DP,
> > + {
> > + (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
> > + (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> > + }
> > + },
> > + BOOT_DISCOVERY_POLICY_MGR_FORMSET_GUID
> > + },
> > + {
> > + END_DEVICE_PATH_TYPE,
> > + END_ENTIRE_DEVICE_PATH_SUBTYPE,
> > + {
> > + (UINT8)(END_DEVICE_PATH_LENGTH),
> > + (UINT8)((END_DEVICE_PATH_LENGTH) >> 8)
> > + }
> > + }
> > +};
> > +
> > +/**
> > +
> > + Initialize Boot Maintenance Menu library.
> > +
> > + @param ImageHandle The image handle.
> > + @param SystemTable The system table.
> > +
> > + @retval EFI_SUCCESS Install Boot manager menu success.
> > + @retval Other Return error status.gBPDisplayLibGuid
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +BootDiscoveryPolicyUiLibConstructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINTN Size;
> > + UINT32 BootDiscoveryPolicy;
> > +
> > + Size = sizeof (UINT32);
> > + Status = gRT->GetVariable (
> > + BOOT_DISCOVERY_POLICY_VAR,
> > + &gBootDiscoveryPolicyMgrFormsetGuid,
> > + NULL,
> > + &Size,
> > + &BootDiscoveryPolicy
> > + );
> > + if (EFI_ERROR (Status)) {
> > + Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32
> > (PcdBootDiscoveryPolicy));
> > + ASSERT_EFI_ERROR (Status);
> > + }
>
> I don't understand the above check. Seems the value of the variable is not used and the Pcd value is not changed.
>
> Thanks,
> Zhichao
>
> > +
> > + Status = gBS->InstallMultipleProtocolInterfaces (
> > + &mBPDriverHandle,
> > + &gEfiDevicePathProtocolGuid,
> > + &mVendorDevicePath,
> > + NULL
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + //
> > + // Publish our HII data
> > + //
> > + mBPHiiHandle = HiiAddPackages (
> > + &gBootDiscoveryPolicyMgrFormsetGuid,
> > + mBPDriverHandle,
> > + BootDiscoveryPolicyUiLibVfrBin,
> > + BootDiscoveryPolicyUiLibStrings,
> > + NULL
> > + );
> > + if (mBPHiiHandle == NULL) {
> > + gBS->UninstallMultipleProtocolInterfaces (
> > + mBPDriverHandle,
> > + &gEfiDevicePathProtocolGuid,
> > + &mVendorDevicePath,
> > + NULL
> > + );
> > +
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + Destructor of Boot Maintenance menu library.
> > +
> > + @param ImageHandle The firmware allocated handle for the EFI image.
> > + @param SystemTable A pointer to the EFI System Table.
> > +
> > + @retval EFI_SUCCESS The destructor completed successfully.
> > + @retval Other value The destructor did not complete successfully.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +BootDiscoveryPolicyUiLibDestructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > +
> > + if (mBPDriverHandle != NULL) {
> > + gBS->UninstallProtocolInterface (
> > + mBPDriverHandle,
> > + &gEfiDevicePathProtocolGuid,
> > + &mVendorDevicePath
> > + );
> > + mBPDriverHandle = NULL;
> > + }
> > +
> > + if (mBPHiiHandle != NULL) {
> > + HiiRemovePackages (mBPHiiHandle);
> > + mBPHiiHandle = NULL;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > diff --git
> > a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.uni
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > b.uni
> > new file mode 100644
> > index 0000000000..89231bc2d7
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> > +++ iLib.uni
> > @@ -0,0 +1,16 @@
> > +// /** @file
> > +// Boot Discovery Policy UI module.
> > +//
> > +// Copyright (c) 2021, ARM Ltd. All rights reserved.<BR> // Copyright
> > +(c) 2021, Semihalf All rights reserved.<BR> // //
> > +SPDX-License-Identifier: BSD-2-Clause-Patent // // **/
> > +
> > +
> > +#string STR_MODULE_ABSTRACT
> > +#language en-US "Boot Discovery Policy UI module."
> > +
> > +#string STR_MODULE_DESCRIPTION
> > +#language en-US "Boot Discovery Policy UI module."
> > diff --git
> > a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > bStrings.uni
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > bStrings.uni
> > new file mode 100644
> > index 0000000000..736011c9bb
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> > +++ iLibStrings.uni
> > @@ -0,0 +1,29 @@
> > +// *++
> > +//
> > +// Copyright (c) 2021, ARM Ltd. All rights reserved.<BR> // Copyright
> > +(c) 2021, Semihalf All rights reserved.<BR> //
> > +SPDX-License-Identifier: BSD-2-Clause-Patent // // Module Name:
> > +//
> > +// BootDiscoveryPolicyUiLibStrings.uni
> > +//
> > +// Abstract:
> > +//
> > +// String definitions for Boot Discovery Policy UI.
> > +//
> > +// --*/
> > +
> > +/=#
> > +
> > +
> > +#langdef en-US "English"
> > +
> > +#string STR_FORM_BDP_MAIN_TITLE #language en-US "Boot Discovery
> > Policy"
> > +
> > +#string STR_FORM_BDP_CONN_MIN #language en-US "Minimal"
> > +
> > +#string STR_FORM_BDP_CONN_NET #language en-US "Connect
> > Network Devices"
> > +
> > +#string STR_FORM_BDP_CONN_ALL #language en-US "Connect All
> > Devices"
> > +
> > diff --git
> > a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > bVfr.Vfr
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLi
> > bVfr.Vfr
> > new file mode 100644
> > index 0000000000..0de87ec34f
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> > +++ iLibVfr.Vfr
> > @@ -0,0 +1,44 @@
> > +///** @file
> > +//
> > +// Formset for Boot Discovery Policy UI // // Copyright (c) 2021, ARM
> > +Ltd. All rights reserved.<BR> // Copyright (c) 2021, Semihalf All
> > +rights reserved.<BR> // // SPDX-License-Identifier:
> > +BSD-2-Clause-Patent // //**/
> > +
> > +#include <Uefi/UefiMultiPhase.h>
> > +#include "Guid/BootDiscoveryPolicy.h"
> > +#include <Guid/HiiBootMaintenanceFormset.h>
> > +
> > +typedef struct {
> > + UINT32 BootDiscoveryPolicy;
> > +} BOOT_DISCOVERY_POLICY_VARSTORE_DATA;
> > +
> > +formset
> > + guid = BOOT_DISCOVERY_POLICY_MGR_FORMSET_GUID,
> > + title = STRING_TOKEN(STR_FORM_BDP_MAIN_TITLE),
> > + help = STRING_TOKEN(STR_FORM_BDP_MAIN_TITLE),
> > + classguid = EFI_IFR_BOOT_MAINTENANCE_GUID,
> > +
> > + efivarstore BOOT_DISCOVERY_POLICY_VARSTORE_DATA,
> > + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> > + name = BootDiscoveryPolicy,
> > + guid = BOOT_DISCOVERY_POLICY_MGR_FORMSET_GUID;
> > +
> > + form formid = 0x0001,
> > + title = STRING_TOKEN(STR_FORM_BDP_MAIN_TITLE);
> > +
> > + oneof varid = BootDiscoveryPolicy.BootDiscoveryPolicy,
> > + prompt = STRING_TOKEN(STR_FORM_BDP_MAIN_TITLE),
> > + help = STRING_TOKEN(STR_FORM_BDP_MAIN_TITLE),
> > + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> > + option text = STRING_TOKEN(STR_FORM_BDP_CONN_MIN), value =
> > BDP_CONNECT_MINIMAL, flags = DEFAULT;
> > + option text = STRING_TOKEN(STR_FORM_BDP_CONN_NET), value =
> > BDP_CONNECT_NET, flags = 0;
> > + option text = STRING_TOKEN(STR_FORM_BDP_CONN_ALL), value =
> > + BDP_CONNECT_ALL, flags = 0; endoneof;
> > +
> > + endform;
> > +endformset;
> > --
> > 2.25.1
> >
> >
> >
> >
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77651): https://edk2.groups.io/g/devel/message/77651
Mute This Topic: https://groups.io/mt/84017522/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list