Re: [PATCH v4 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Sunny Wang <Sunny.Wang@arm.com>
> Sent: Monday, August 2, 2021 11:16 AM
> To: Grzegorz Bernacki <gjb@semihalf.com>; devel@edk2.groups.io; Wang,
> Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; mw@semihalf.com;
> upstream@semihalf.com; pete@akeo.ie; Bi, Dandan <dandan.bi@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Sunny Wang <Sunny.Wang@arm.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [PATCH v4 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
> 
> Hi Hao, Jian, and MdeModulePkg maintainers,
> 
> Could you help merge this patch? This patch already got Reviewed-by from
> Zhichao and me. If we still need to get others' reviews, feel free to let us
> know.


Really sorry for the slow response.
Pull request created at: https://github.com/tianocore/edk2/pull/1855

By the way, I do not see an explicit 'Reviewed-by' tag from you for this V4 patch, so I will treat the above statement as your 'R-b' tag for this patch.
Thanks you and Zhichao for the reviewing effort.

Best Regards,
Hao Wu


> By the way, we do have an edk2-platform change below that is waiting for
> this patch to get merged first, and will have more patches based on this to be
> sent out to solve some deployment and testing problems caused by Fast
> boot on ARM platforms.
>     - https://edk2.groups.io/g/devel/message/78072
> 
> Best Regards,
> Sunny Wang
> 
> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Thursday, July 22, 2021 3:44 PM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; mw@semihalf.com; upstream@semihalf.com;
> pete@akeo.ie; jian.j.wang@intel.com; hao.a.wu@intel.com;
> dandan.bi@intel.com; eric.dong@intel.com; Grzegorz Bernacki
> <gjb@semihalf.com>; Zhichao Gao <zhichao.gao@intel.com>
> Subject: [PATCH v4 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@semihalf.com>
> Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec                                                     |   9 ++
> 
> MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> inf        |  52 +++++++
>  MdeModulePkg/Include/Guid/BootDiscoveryPolicy.h                                   |  22
> +++
> 
> MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> c          | 160 ++++++++++++++++++++
> 
> MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.
> uni        |  18 +++
> 
> MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> Strings.uni |  29 ++++
> 
> MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib
> Vfr.Vfr     |  44 ++++++
>  7 files changed, 334 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..133e04ee86
> 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,12 @@
>    # @Prompt Console Output Row of Text Setup
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x40
> 00000e
> 
> +  ## Specify the Boot Discovery Policy settings  # To support
> + configuring from setup page, this PCD should be overridden in DynamicHii
> type in its platform .dsc:
> +  #
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy|L"BootDiscove
> ryP
> + olicy"|gBootDiscoveryPolicyMgrFormsetGuid|0
> +  # @Prompt Boot Discovery Policy
> +
> +
> 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);
> +  }
> +
> +  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..eea3ca6c8d
> --- /dev/null
> +++
> b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyU
> +++ iLib.uni
> @@ -0,0 +1,18 @@
> +// /** @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
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> 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 information in any
> medium. Thank you.