public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add BootDiscoveryPolicyUiLib
@ 2021-07-06 10:44 Grzegorz Bernacki
  2021-07-06 10:44 ` [PATCH v2 1/1] MdeModulePkg: " Grzegorz Bernacki
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-06 10:44 UTC (permalink / raw)
  To: devel
  Cc: leif, ardb+tianocore, Samer.El-Haj-Mahmoud, sunny.Wang, mw,
	upstream, pete, jian.j.wang, hao.a.wu, dandan.bi, eric.dong,
	Grzegorz Bernacki

This patchset extends Boot Maintenance Menu and allows to select
Boot Discovery Policy. Raspberry Pi platforms uses the variable to
connect specified class of devices on boot. This patchset also
removes efdc159e which has similar functionality.

Discussion on design can be found at:
https://edk2.groups.io/g/rfc/topic/rfc_boot_discovery_policy/82450628

Changes since v1:
- make 'Connect All' (0x2) default value for PcdBootDiscoveryPolicy
- initialize BootDiscoveryPolicy variable in platform code, if not found

Grzegorz Bernacki (1):
edk2:
  MdeModulePkg: Add BootDiscoveryPolicyUiLib.
edk2-platforms:
  Platform/RaspberryPi: Enable Boot Discovery Policy.
  Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot"

 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/BootDiscoveryPolicyUiLibStrings.uni |  29 ++++
 MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.Vfr     |  44 ++++++
 Platform/RaspberryPi/RaspberryPi.dec                                           |   2 -
 Platform/RaspberryPi/RPi3/RPi3.dsc                                             |   9 +-
 Platform/RaspberryPi/RPi4/RPi4.dsc                                             |  12 +--
 Platform/RaspberryPi/RPi4/RPi4.fdf                                             |   1 +
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf                           |   3 +-
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |   6 +-
 Platform/RaspberryPi/Include/ConfigVars.h                                      |  12 +--
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr                        |  16 +---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                             |  11 +--
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 101 +++++++++++++++++---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni                        |  10 +-
 18 files changed, 435 insertions(+), 77 deletions(-)
 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/BootDiscoveryPolicyUiLibStrings.uni
 create mode 100644 MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.Vfr

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
  2021-07-06 10:44 [PATCH v2 0/2] Add BootDiscoveryPolicyUiLib Grzegorz Bernacki
@ 2021-07-06 10:44 ` Grzegorz Bernacki
  2021-07-08  8:08   ` [edk2-devel] " Gao, Zhichao
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy Grzegorz Bernacki
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot" Grzegorz Bernacki
  2 siblings, 1 reply; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-06 10:44 UTC (permalink / raw)
  To: devel
  Cc: leif, ardb+tianocore, Samer.El-Haj-Mahmoud, sunny.Wang, mw,
	upstream, pete, jian.j.wang, hao.a.wu, dandan.bi, eric.dong,
	Grzegorz Bernacki, Sunny Wang

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: Sunny Wang <sunny.wang@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/BootDiscoveryPolicyUiLibStrings.uni |  29 ++++
 MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.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/BootDiscoveryPolicyUiLibStrings.uni
 create mode 100644 MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.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|0x4000000e
 
+  ## Specify the Boot Discovery Policy settings
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy|2|UINT32|0x4000000f
+
 [PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20|UINT32|0x0001004c
 
diff --git a/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.inf b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.inf
new file mode 100644
index 0000000000..1fb4d43caa
--- /dev/null
+++ b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.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/BootDiscoveryPolicyUiLib.c b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.c
new file mode 100644
index 0000000000..6814d0bb8f
--- /dev/null
+++ b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.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/BootDiscoveryPolicyUiLib.uni b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.uni
new file mode 100644
index 0000000000..89231bc2d7
--- /dev/null
+++ b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.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/BootDiscoveryPolicyUiLibStrings.uni b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibStrings.uni
new file mode 100644
index 0000000000..736011c9bb
--- /dev/null
+++ b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibStrings.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/BootDiscoveryPolicyUiLibVfr.Vfr b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.Vfr
new file mode 100644
index 0000000000..0de87ec34f
--- /dev/null
+++ b/MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLibVfr.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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy.
  2021-07-06 10:44 [PATCH v2 0/2] Add BootDiscoveryPolicyUiLib Grzegorz Bernacki
  2021-07-06 10:44 ` [PATCH v2 1/1] MdeModulePkg: " Grzegorz Bernacki
@ 2021-07-06 10:44 ` Grzegorz Bernacki
  2021-07-08  8:15   ` [edk2-devel] " Gao, Zhichao
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot" Grzegorz Bernacki
  2 siblings, 1 reply; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-06 10:44 UTC (permalink / raw)
  To: devel
  Cc: leif, ardb+tianocore, Samer.El-Haj-Mahmoud, sunny.Wang, mw,
	upstream, pete, jian.j.wang, hao.a.wu, dandan.bi, eric.dong,
	Grzegorz Bernacki, Sunny Wang

This commit modify platform boot to check the value of
BootDiscoveryPolicy variable and use BootPolicyManager
Protocol to connect devices specified by the variable.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 Platform/RaspberryPi/RPi4/RPi4.dsc                                             |  3 +
 Platform/RaspberryPi/RPi4/RPi4.fdf                                             |  1 +
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.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"|gRaspberryPiTokenSpaceGuid|0x0|80
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|L"Rows"|gRaspberryPiTokenSpaceGuid|0x0|25
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|L"Rows"|gRaspberryPiTokenSpaceGuid|0x0|25
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy|L"BootDiscoveryPolicy"|gBootDiscoveryPolicyMgrFormsetGuid|0
 
 [PcdsDynamicDefault.common]
   #
@@ -682,6 +683,7 @@
   #
   # Bds
   #
+  MdeModulePkg/Universal/BootManagerPolicyDxe/BootManagerPolicyDxe.inf
   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 {
     <LibraryClasses>
+      NULL|MdeModulePkg/Library/BootDiscoveryPolicyUiLib/BootDiscoveryPolicyUiLib.inf
       NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
       NULL|Platform/RaspberryPi/Library/PlatformUiAppLib/PlatformUiAppLib.inf
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   = TRUE
   #
   # Bds
   #
+  INF MdeModulePkg/Universal/BootManagerPolicyDxe/BootManagerPolicyDxe.inf
   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/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index fbf510ab96..4ef2f791ae 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.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 <Library/UefiBootManagerLib.h>
 #include <Library/UefiLib.h>
 #include <Library/PrintLib.h>
+#include <Protocol/BootManagerPolicy.h>
 #include <Protocol/DevicePath.h>
 #include <Protocol/EsrtManagement.h>
 #include <Protocol/GraphicsOutput.h>
 #include <Protocol/LoadedImage.h>
+#include <Guid/BootDiscoveryPolicy.h>
 #include <Guid/EventGroup.h>
 #include <Guid/TtyTerm.h>
 #include <ConfigVars.h>
@@ -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(), 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 = sizeof (DiscoveryPolicy);
+  Status = gRT->GetVariable (
+                  BOOT_DISCOVERY_POLICY_VAR,
+                  &gBootDiscoveryPolicyMgrFormsetGuid,
+                  NULL,
+                  &Size,
+                  &DiscoveryPolicy
+                  );
+  if (Status == EFI_NOT_FOUND) {
+    Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy));
+    if (Status == EFI_NOT_FOUND) {
+      return EFI_SUCCESS;
+    } else if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  } else if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {
+    return EFI_SUCCESS;
+  }
+
+  switch (DiscoveryPolicy) {
+    case BDP_CONNECT_NET:
+      Class = &gEfiBootManagerPolicyNetworkGuid;
+      break;
+    case BDP_CONNECT_ALL:
+      Class = &gEfiBootManagerPolicyConnectAllGuid;
+      break;
+    default:
+      DEBUG ((
+        DEBUG_INFO,
+        "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n",
+        __FUNCTION__,
+        DiscoveryPolicy
+        ));
+      return EFI_SUCCESS;
+  }
+
+  Status = 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 = 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 all devices\n"));
   }
 
+  Status = BootDiscoveryPolicyHandler ();
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n", Status));
+  }
+
   Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, (VOID**)&EsrtManagement);
   if (!EFI_ERROR (Status)) {
     EsrtManagement->SyncEsrtFmp ();
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot"
  2021-07-06 10:44 [PATCH v2 0/2] Add BootDiscoveryPolicyUiLib Grzegorz Bernacki
  2021-07-06 10:44 ` [PATCH v2 1/1] MdeModulePkg: " Grzegorz Bernacki
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy Grzegorz Bernacki
@ 2021-07-06 10:44 ` Grzegorz Bernacki
  2021-07-08 18:36   ` Samer El-Haj-Mahmoud
  2 siblings, 1 reply; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-06 10:44 UTC (permalink / raw)
  To: devel
  Cc: leif, ardb+tianocore, Samer.El-Haj-Mahmoud, sunny.Wang, mw,
	upstream, pete, jian.j.wang, hao.a.wu, dandan.bi, eric.dong,
	Grzegorz Bernacki, Sunny Wang

This reverts commit efdc159ef7c9f15581a0f63d755a1530ff475156.

This commit is not longer required as Boot Discovery Policy has
been implemented for RPi.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 Platform/RaspberryPi/RaspberryPi.dec                                           |  2 --
 Platform/RaspberryPi/RPi3/RPi3.dsc                                             |  9 +--------
 Platform/RaspberryPi/RPi4/RPi4.dsc                                             |  9 +--------
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf                           |  3 +--
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 -
 Platform/RaspberryPi/Include/ConfigVars.h                                      | 12 +-----------
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr                        | 16 +---------------
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                             | 11 +----------
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 15 ++-------------
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni                        | 10 +---------
 10 files changed, 9 insertions(+), 79 deletions(-)

diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index f1dd8ac0ed..2ca25ff9e6 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -2,7 +2,6 @@
 #
 #  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
 #  Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
-#  Copyright (c) 2021, ARM Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -71,5 +70,4 @@
   gRaspberryPiTokenSpaceGuid.PcdFanTemp|0|UINT32|0x0000001D
   gRaspberryPiTokenSpaceGuid.PcdPlatformResetDelay|0|UINT32|0x0000001E
   gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F
-  gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020
   gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 53825bcf62..b6e3372c61 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -1,6 +1,6 @@
 # @file
 #
-#  Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
+#  Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
 #  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
 #  Copyright (c) 2017 - 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
@@ -512,13 +512,6 @@
   gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
 
-  #
-  # Boot Policy
-  # 0  - Fast Boot
-  # 1  - Full Discovery (Connect All)
-  #
-  gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
-
   #
   # Reset-related.
   #
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 8b9beac64a..07f36e7f1b 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -1,6 +1,6 @@
 # @file
 #
-#  Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
+#  Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
 #  Copyright (c) 2017 - 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
 #  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
@@ -528,13 +528,6 @@
   gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|60
 
-  #
-  # Boot Policy
-  # 0  - Fast Boot
-  # 1  - Full Discovery (Connect All)
-  #
-  gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
-
   #
   # Reset-related.
   #
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 597e1b4205..4bb2d08550 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -2,7 +2,7 @@
 #
 #  Component description file for the RasbperryPi DXE platform config driver.
 #
-#  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
+#  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
 #  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -93,7 +93,6 @@
   gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
   gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
   gRaspberryPiTokenSpaceGuid.PcdFanTemp
-  gRaspberryPiTokenSpaceGuid.PcdBootPolicy
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
 
 [Depex]
diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 4ef2f791ae..c047364b28 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -64,7 +64,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy 
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
   gRaspberryPiTokenSpaceGuid.PcdSdIsArasan
-  gRaspberryPiTokenSpaceGuid.PcdBootPolicy
 
 [Guids]
   gBootDiscoveryPolicyMgrFormsetGuid
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index 9ef62b7a6e..142317985a 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -1,7 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
- *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -143,14 +143,4 @@ typedef struct {
   UINT32 EnableDma;
 } MMC_EMMC_DMA_VARSTORE_DATA;
 
-#define FAST_BOOT      0
-#define FULL_DISCOVERY 1
-typedef struct {
-  /*
-   * 0 - Fast Boot
-   * 1 - Full Discovery (Connect All)
-   */
-  UINT32 BootPolicy;
-} BOOT_POLICY_VARSTORE_DATA;
-
 #endif /* CONFIG_VARS_H */
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 759db6212f..fa34eab809 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -1,7 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2018 Andrei Warkentin <andrey.warkentin@gmail.com>
- *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -116,11 +116,6 @@ formset
       name  = DisplayEnableSShot,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
-    efivarstore BOOT_POLICY_VARSTORE_DATA,
-      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-      name  = BootPolicy,
-      guid  = CONFIGDXE_FORM_SET_GUID;
-
     form formid = 1,
         title  = STRING_TOKEN(STR_FORM_SET_TITLE);
         subtitle text = STRING_TOKEN(STR_NULL_STRING);
@@ -195,14 +190,6 @@ formset
             option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
         endoneof;
 
-        oneof varid = BootPolicy.BootPolicy,
-            prompt      = STRING_TOKEN(STR_BOOT_POLICY_PROMPT),
-            help        = STRING_TOKEN(STR_BOOT_POLICY_HELP),
-            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
-            option text = STRING_TOKEN(STR_FAST_BOOT), value = FAST_BOOT , flags = 0;
-            option text = STRING_TOKEN(STR_FULL_DISCOVERY), value = FULL_DISCOVERY, flags = DEFAULT;
-        endoneof;
-
 #if (RPI_MODEL == 4)
         grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
           oneof varid = FanOnGpio.Enabled,
@@ -233,7 +220,6 @@ formset
             minsize = 0,
             maxsize = ASSET_TAG_STR_MAX_LEN,
         endstring;
-
     endform;
 
     form formid = 0x1003,
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index cf9880bd20..9e78cb47ad 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -1,6 +1,6 @@
 /** @file
  *
- *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
+ *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
  *  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -286,15 +286,6 @@ SetupVariables (
                     );
   }
 
-  Size = sizeof (UINT32);
-  Status = gRT->GetVariable (L"BootPolicy",
-                  &gConfigDxeFormSetGuid,
-                  NULL, &Size, &Var32);
-  if (EFI_ERROR (Status)) {
-    Status = PcdSet32S (PcdBootPolicy, PcdGet32 (PcdBootPolicy));
-    ASSERT_EFI_ERROR (Status);
-  }
-
   Size = sizeof (UINT32);
   Status = gRT->GetVariable (L"SdIsArasan",
                   &gConfigDxeFormSetGuid,
diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
index 4bfa906921..b5b485f3e8 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
@@ -4,7 +4,7 @@
  *  Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) 2016, Linaro Ltd. All rights reserved.
  *  Copyright (c) 2015-2016, Red Hat, Inc.
- *  Copyright (c) 2014-2021, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2014-2020, ARM Ltd. All rights reserved.
  *  Copyright (c) 2004-2016, Intel Corporation. All rights reserved.
  *  Copyright (c) 2021, Semihalf All rights reserved.
  *
@@ -28,11 +28,10 @@
 #include <Guid/BootDiscoveryPolicy.h>
 #include <Guid/EventGroup.h>
 #include <Guid/TtyTerm.h>
-#include <ConfigVars.h>
 
 #include "PlatformBm.h"
 
-#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)\n"
+#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)"
 
 #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
 
@@ -719,16 +718,6 @@ PlatformBootManagerAfterConsole (
     Print (BOOT_PROMPT);
   }
 
-  //
-  // Connect the rest of the devices if the boot polcy is set to Full discovery
-  //
-  if (PcdGet32 (PcdBootPolicy) == FULL_DISCOVERY) {
-    DEBUG ((DEBUG_INFO, "Boot Policy is Full Discovery. Connect all devices\n"));
-    EfiBootManagerConnectAll ();
-  } else if (PcdGet32 (PcdBootPolicy) == FAST_BOOT) {
-    DEBUG ((DEBUG_INFO, "Boot Policy is Fast Boot. Skip connecting all devices\n"));
-  }
-
   Status = BootDiscoveryPolicyHandler ();
   if (EFI_ERROR(Status)) {
     DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n", Status));
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 81761d64bb..466fa852cb 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -1,7 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
- *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -60,14 +60,6 @@
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset Tag"
 
-#string STR_BOOT_POLICY_PROMPT        #language en-US "Boot Policy"
-#string STR_BOOT_POLICY_HELP          #language en-US "When Fast Boot is selected, only required devices will be discovered for reducing "
-                                                      "the boot time. "
-                                                      "When Full Discovery is selected, all the devices will be discovered for some "
-                                                      "scenarios such as system deployment and diagnostic tests."
-#string STR_FAST_BOOT                 #language en-US "Fast Boot"
-#string STR_FULL_DISCOVERY            #language en-US "Full Discovery"
-
 /*
  * MMC/SD configuration.
  */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
  2021-07-06 10:44 ` [PATCH v2 1/1] MdeModulePkg: " Grzegorz Bernacki
@ 2021-07-08  8:08   ` Gao, Zhichao
  2021-07-09  9:55     ` Grzegorz Bernacki
  0 siblings, 1 reply; 11+ messages in thread
From: Gao, Zhichao @ 2021-07-08  8:08 UTC (permalink / raw)
  To: devel@edk2.groups.io, gjb@semihalf.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

See below comments.

> -----Original Message-----
> From: devel@edk2.groups.io <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
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@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@semihalf.com>
> Reviewed-by: Sunny Wang <sunny.wang@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
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy.
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy Grzegorz Bernacki
@ 2021-07-08  8:15   ` Gao, Zhichao
  2021-07-09  9:47     ` Grzegorz Bernacki
  0 siblings, 1 reply; 11+ messages in thread
From: Gao, Zhichao @ 2021-07-08  8:15 UTC (permalink / raw)
  To: devel@edk2.groups.io, gjb@semihalf.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

See below comments.

> -----Original Message-----
> From: devel@edk2.groups.io <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
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@arm.com>
> Subject: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi:
> Enable Boot Discovery Policy.
> 
> This commit modify platform boot to check the value of BootDiscoveryPolicy
> variable and use BootPolicyManager Protocol to connect devices specified by
> the variable.
> 
> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  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 {
>      <LibraryClasses>
> +
> +
> 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.inf
> 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   = 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 <Library/UefiBootManagerLib.h>  #include <Library/UefiLib.h>
> #include <Library/PrintLib.h>
> +#include <Protocol/BootManagerPolicy.h>
>  #include <Protocol/DevicePath.h>
>  #include <Protocol/EsrtManagement.h>
>  #include <Protocol/GraphicsOutput.h>
>  #include <Protocol/LoadedImage.h>
> +#include <Guid/BootDiscoveryPolicy.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/TtyTerm.h>
>  #include <ConfigVars.h>
> @@ -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(), 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 = sizeof (DiscoveryPolicy);
> +  Status = gRT->GetVariable (
> +                  BOOT_DISCOVERY_POLICY_VAR,
> +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> +                  NULL,
> +                  &Size,
> +                  &DiscoveryPolicy
> +                  );
> +  if (Status == EFI_NOT_FOUND) {
> +    Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32
> (PcdBootDiscoveryPolicy));

Why need this Pcd statement?

> +    if (Status == EFI_NOT_FOUND) {
> +      return EFI_SUCCESS;
> +    } else if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +  } else if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {

If the Status is EFI_NOT_FOUND at GetVariable, then the above DiscoveryPolicy 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 = &gEfiBootManagerPolicyNetworkGuid;
> +      break;
> +    case BDP_CONNECT_ALL:
> +      Class = &gEfiBootManagerPolicyConnectAllGuid;
> +      break;
> +    default:
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery
> Policy\n",
> +        __FUNCTION__,
> +        DiscoveryPolicy
> +        ));
> +      return EFI_SUCCESS;
> +  }
> +
> +  Status = 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 = 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 all
> devices\n"));
>    }
> 
> +  Status = BootDiscoveryPolicyHandler ();  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n",
> + Status));  }
> +
>    Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
> (VOID**)&EsrtManagement);
>    if (!EFI_ERROR (Status)) {
>      EsrtManagement->SyncEsrtFmp ();
> --
> 2.25.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot"
  2021-07-06 10:44 ` [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot" Grzegorz Bernacki
@ 2021-07-08 18:36   ` Samer El-Haj-Mahmoud
  0 siblings, 0 replies; 11+ messages in thread
From: Samer El-Haj-Mahmoud @ 2021-07-08 18:36 UTC (permalink / raw)
  To: Grzegorz Bernacki, devel@edk2.groups.io
  Cc: leif@nuviainc.com, ardb+tianocore@kernel.org, Sunny Wang,
	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, Sunny Wang, Samer El-Haj-Mahmoud

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Tuesday, July 6, 2021 6:45 AM
> 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>; Sunny Wang <Sunny.Wang@arm.com>
> Subject: [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup
> option for disabling Fast Boot"
>
> This reverts commit efdc159ef7c9f15581a0f63d755a1530ff475156.
>
> This commit is not longer required as Boot Discovery Policy has
> been implemented for RPi.
>
> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  Platform/RaspberryPi/RaspberryPi.dec                                           |  2 --
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                             |  9 +--------
>  Platform/RaspberryPi/RPi4/RPi4.dsc                                             |  9 +--------
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf                           |  3 +-
> -
>
> Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManag
> erLib.inf |  1 -
>  Platform/RaspberryPi/Include/ConfigVars.h                                      | 12 +--------
> ---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr                        | 16
> +---------------
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                             | 11 +-
> ---------
>  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> | 15 ++-------------
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni                        | 10
> +---------
>  10 files changed, 9 insertions(+), 79 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec
> b/Platform/RaspberryPi/RaspberryPi.dec
> index f1dd8ac0ed..2ca25ff9e6 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -2,7 +2,6 @@
>  #
>  #  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
>  #  Copyright (c) 2017-2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>
> -#  Copyright (c) 2021, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -71,5 +70,4 @@
>    gRaspberryPiTokenSpaceGuid.PcdFanTemp|0|UINT32|0x0000001D
>
> gRaspberryPiTokenSpaceGuid.PcdPlatformResetDelay|0|UINT32|0x0000001
> E
>    gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F
> -  gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020
>    gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc
> b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 53825bcf62..b6e3372c61 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -1,6 +1,6 @@
>  # @file
>  #
> -#  Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
>  #  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
>  #  Copyright (c) 2017 - 2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>
> @@ -512,13 +512,6 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFor
> mSetGuid|0x0|0
>
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSet
> Guid|0x0|0
>
> -  #
> -  # Boot Policy
> -  # 0  - Fast Boot
> -  # 1  - Full Discovery (Connect All)
> -  #
> -
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeForm
> SetGuid|0x0|1
> -
>    #
>    # Reset-related.
>    #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
> b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 8b9beac64a..07f36e7f1b 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -1,6 +1,6 @@
>  # @file
>  #
> -#  Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
>  #  Copyright (c) 2017 - 2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>
>  #  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
> @@ -528,13 +528,6 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFor
> mSetGuid|0x0|0
>
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSet
> Guid|0x0|60
>
> -  #
> -  # Boot Policy
> -  # 0  - Fast Boot
> -  # 1  - Full Discovery (Connect All)
> -  #
> -
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeForm
> SetGuid|0x0|1
> -
>    #
>    # Reset-related.
>    #
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 597e1b4205..4bb2d08550 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -2,7 +2,7 @@
>  #
>  #  Component description file for the RasbperryPi DXE platform config
> driver.
>  #
> -#  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> +#  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>  #  Copyright (c) 2018 - 2020, Andrei Warkentin
> <andrey.warkentin@gmail.com>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -93,7 +93,6 @@
>    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>    gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
>    gRaspberryPiTokenSpaceGuid.PcdFanTemp
> -  gRaspberryPiTokenSpaceGuid.PcdBootPolicy
>    gRaspberryPiTokenSpaceGuid.PcdUartInUse
>
>  [Depex]
> diff --git
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> index 4ef2f791ae..c047364b28 100644
> ---
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> +++
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> @@ -64,7 +64,6 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
>    gRaspberryPiTokenSpaceGuid.PcdSdIsArasan
> -  gRaspberryPiTokenSpaceGuid.PcdBootPolicy
>
>  [Guids]
>    gBootDiscoveryPolicyMgrFormsetGuid
> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> b/Platform/RaspberryPi/Include/ConfigVars.h
> index 9ef62b7a6e..142317985a 100644
> --- a/Platform/RaspberryPi/Include/ConfigVars.h
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -1,7 +1,7 @@
>  /** @file
>   *
>   *  Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> - *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + *  Copyright (c) 2020, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -143,14 +143,4 @@ typedef struct {
>    UINT32 EnableDma;
>  } MMC_EMMC_DMA_VARSTORE_DATA;
>
> -#define FAST_BOOT      0
> -#define FULL_DISCOVERY 1
> -typedef struct {
> -  /*
> -   * 0 - Fast Boot
> -   * 1 - Full Discovery (Connect All)
> -   */
> -  UINT32 BootPolicy;
> -} BOOT_POLICY_VARSTORE_DATA;
> -
>  #endif /* CONFIG_VARS_H */
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index 759db6212f..fa34eab809 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -1,7 +1,7 @@
>  /** @file
>   *
>   *  Copyright (c) 2018 Andrei Warkentin <andrey.warkentin@gmail.com>
> - *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + *  Copyright (c) 2020, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -116,11 +116,6 @@ formset
>        name  = DisplayEnableSShot,
>        guid  = CONFIGDXE_FORM_SET_GUID;
>
> -    efivarstore BOOT_POLICY_VARSTORE_DATA,
> -      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> -      name  = BootPolicy,
> -      guid  = CONFIGDXE_FORM_SET_GUID;
> -
>      form formid = 1,
>          title  = STRING_TOKEN(STR_FORM_SET_TITLE);
>          subtitle text = STRING_TOKEN(STR_NULL_STRING);
> @@ -195,14 +190,6 @@ formset
>              option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value =
> SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
>          endoneof;
>
> -        oneof varid = BootPolicy.BootPolicy,
> -            prompt      = STRING_TOKEN(STR_BOOT_POLICY_PROMPT),
> -            help        = STRING_TOKEN(STR_BOOT_POLICY_HELP),
> -            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> -            option text = STRING_TOKEN(STR_FAST_BOOT), value = FAST_BOOT ,
> flags = 0;
> -            option text = STRING_TOKEN(STR_FULL_DISCOVERY), value =
> FULL_DISCOVERY, flags = DEFAULT;
> -        endoneof;
> -
>  #if (RPI_MODEL == 4)
>          grayoutif NOT ideqval SystemTableMode.Mode ==
> SYSTEM_TABLE_MODE_ACPI;
>            oneof varid = FanOnGpio.Enabled,
> @@ -233,7 +220,6 @@ formset
>              minsize = 0,
>              maxsize = ASSET_TAG_STR_MAX_LEN,
>          endstring;
> -
>      endform;
>
>      form formid = 0x1003,
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index cf9880bd20..9e78cb47ad 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -1,6 +1,6 @@
>  /** @file
>   *
> - *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> + *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>   *  Copyright (c) 2018 - 2020, Andrei Warkentin
> <andrey.warkentin@gmail.com>
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -286,15 +286,6 @@ SetupVariables (
>                      );
>    }
>
> -  Size = sizeof (UINT32);
> -  Status = gRT->GetVariable (L"BootPolicy",
> -                  &gConfigDxeFormSetGuid,
> -                  NULL, &Size, &Var32);
> -  if (EFI_ERROR (Status)) {
> -    Status = PcdSet32S (PcdBootPolicy, PcdGet32 (PcdBootPolicy));
> -    ASSERT_EFI_ERROR (Status);
> -  }
> -
>    Size = sizeof (UINT32);
>    Status = gRT->GetVariable (L"SdIsArasan",
>                    &gConfigDxeFormSetGuid,
> diff --git
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 4bfa906921..b5b485f3e8 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -4,7 +4,7 @@
>   *  Copyright (c) 2017-2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>
>   *  Copyright (c) 2016, Linaro Ltd. All rights reserved.
>   *  Copyright (c) 2015-2016, Red Hat, Inc.
> - *  Copyright (c) 2014-2021, ARM Ltd. All rights reserved.
> + *  Copyright (c) 2014-2020, ARM Ltd. All rights reserved.
>   *  Copyright (c) 2004-2016, Intel Corporation. All rights reserved.
>   *  Copyright (c) 2021, Semihalf All rights reserved.
>   *
> @@ -28,11 +28,10 @@
>  #include <Guid/BootDiscoveryPolicy.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/TtyTerm.h>
> -#include <ConfigVars.h>
>
>  #include "PlatformBm.h"
>
> -#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)\n"
> +#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)"
>
>  #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type)
> >> 8) }
>
> @@ -719,16 +718,6 @@ PlatformBootManagerAfterConsole (
>      Print (BOOT_PROMPT);
>    }
>
> -  //
> -  // Connect the rest of the devices if the boot polcy is set to Full discovery
> -  //
> -  if (PcdGet32 (PcdBootPolicy) == FULL_DISCOVERY) {
> -    DEBUG ((DEBUG_INFO, "Boot Policy is Full Discovery. Connect all
> devices\n"));
> -    EfiBootManagerConnectAll ();
> -  } else if (PcdGet32 (PcdBootPolicy) == FAST_BOOT) {
> -    DEBUG ((DEBUG_INFO, "Boot Policy is Fast Boot. Skip connecting all
> devices\n"));
> -  }
> -
>    Status = BootDiscoveryPolicyHandler ();
>    if (EFI_ERROR(Status)) {
>      DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n",
> Status));
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 81761d64bb..466fa852cb 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -1,7 +1,7 @@
>  /** @file
>   *
>   *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> - *  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + *  Copyright (c) 2020, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -60,14 +60,6 @@
>  #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>  #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system
> Asset Tag"
>
> -#string STR_BOOT_POLICY_PROMPT        #language en-US "Boot Policy"
> -#string STR_BOOT_POLICY_HELP          #language en-US "When Fast Boot is
> selected, only required devices will be discovered for reducing "
> -                                                      "the boot time. "
> -                                                      "When Full Discovery is selected, all the
> devices will be discovered for some "
> -                                                      "scenarios such as system deployment and
> diagnostic tests."
> -#string STR_FAST_BOOT                 #language en-US "Fast Boot"
> -#string STR_FULL_DISCOVERY            #language en-US "Full Discovery"
> -
>  /*
>   * MMC/SD configuration.
>   */
> --
> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy.
  2021-07-08  8:15   ` [edk2-devel] " Gao, Zhichao
@ 2021-07-09  9:47     ` Grzegorz Bernacki
  0 siblings, 0 replies; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-09  9:47 UTC (permalink / raw)
  To: devel, zhichao.gao
  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

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 <zhichao.gao@intel.com> napisał(a):
>
> See below comments.
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <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
> > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> > <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> > Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@arm.com>
> > Subject: [edk2-devel] [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi:
> > Enable Boot Discovery Policy.
> >
> > This commit modify platform boot to check the value of BootDiscoveryPolicy
> > variable and use BootPolicyManager Protocol to connect devices specified by
> > the variable.
> >
> > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> > Reviewed-by: Sunny Wang <sunny.wang@arm.com>
> > ---
> >  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 {
> >      <LibraryClasses>
> > +
> > +
> > 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.inf
> > 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   = 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 <Library/UefiBootManagerLib.h>  #include <Library/UefiLib.h>
> > #include <Library/PrintLib.h>
> > +#include <Protocol/BootManagerPolicy.h>
> >  #include <Protocol/DevicePath.h>
> >  #include <Protocol/EsrtManagement.h>
> >  #include <Protocol/GraphicsOutput.h>
> >  #include <Protocol/LoadedImage.h>
> > +#include <Guid/BootDiscoveryPolicy.h>
> >  #include <Guid/EventGroup.h>
> >  #include <Guid/TtyTerm.h>
> >  #include <ConfigVars.h>
> > @@ -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(), 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 = sizeof (DiscoveryPolicy);
> > +  Status = gRT->GetVariable (
> > +                  BOOT_DISCOVERY_POLICY_VAR,
> > +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> > +                  NULL,
> > +                  &Size,
> > +                  &DiscoveryPolicy
> > +                  );
> > +  if (Status == EFI_NOT_FOUND) {
> > +    Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32
> > (PcdBootDiscoveryPolicy));
>
> Why need this Pcd statement?
>
> > +    if (Status == EFI_NOT_FOUND) {
> > +      return EFI_SUCCESS;
> > +    } else if (EFI_ERROR (Status)) {
> > +      return Status;
> > +    }
> > +  } else if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {
>
> If the Status is EFI_NOT_FOUND at GetVariable, then the above DiscoveryPolicy 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 = &gEfiBootManagerPolicyNetworkGuid;
> > +      break;
> > +    case BDP_CONNECT_ALL:
> > +      Class = &gEfiBootManagerPolicyConnectAllGuid;
> > +      break;
> > +    default:
> > +      DEBUG ((
> > +        DEBUG_INFO,
> > +        "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery
> > Policy\n",
> > +        __FUNCTION__,
> > +        DiscoveryPolicy
> > +        ));
> > +      return EFI_SUCCESS;
> > +  }
> > +
> > +  Status = 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 = 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 all
> > devices\n"));
> >    }
> >
> > +  Status = BootDiscoveryPolicyHandler ();  if (EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_INFO, "Error applying Boot Discovery Policy:%r\n",
> > + Status));  }
> > +
> >    Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
> > (VOID**)&EsrtManagement);
> >    if (!EFI_ERROR (Status)) {
> >      EsrtManagement->SyncEsrtFmp ();
> > --
> > 2.25.1
> >
> >
> >
> >
> >
>
>
>
> 
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
  2021-07-08  8:08   ` [edk2-devel] " Gao, Zhichao
@ 2021-07-09  9:55     ` Grzegorz Bernacki
  2021-07-21  5:14       ` Gao, Zhichao
  0 siblings, 1 reply; 11+ messages in thread
From: Grzegorz Bernacki @ 2021-07-09  9:55 UTC (permalink / raw)
  To: Gao, Zhichao
  Cc: devel@edk2.groups.io, 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

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@intel.com> napisał(a):
>
> See below comments.
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <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
> > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> > <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> > Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@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@semihalf.com>
> > Reviewed-by: Sunny Wang <sunny.wang@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
> >
> >
> >
> > 
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
  2021-07-09  9:55     ` Grzegorz Bernacki
@ 2021-07-21  5:14       ` Gao, Zhichao
  2021-07-21  7:27         ` Sunny Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Gao, Zhichao @ 2021-07-21  5:14 UTC (permalink / raw)
  To: Grzegorz Bernacki
  Cc: devel@edk2.groups.io, 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

OK. I am not familiar with PCD, it is new usage for me. And now I got to know the reason. But seems the behavior would be different base on the initialization on DSC file.
Whatever, this patch is OK to me. Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Friday, July 9, 2021 5:55 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>
> Cc: devel@edk2.groups.io; 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
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add
> BootDiscoveryPolicyUiLib.
> 
> 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@intel.com> napisał(a):
> >
> > See below comments.
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <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
> > > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> > > <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> > > Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@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@semihalf.com>
> > > Reviewed-by: Sunny Wang <sunny.wang@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
> > >
> > >
> > >
> > > 
> > >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.
  2021-07-21  5:14       ` Gao, Zhichao
@ 2021-07-21  7:27         ` Sunny Wang
  0 siblings, 0 replies; 11+ messages in thread
From: Sunny Wang @ 2021-07-21  7:27 UTC (permalink / raw)
  To: Gao, Zhichao, Grzegorz Bernacki
  Cc: devel@edk2.groups.io, leif@nuviainc.com,
	ardb+tianocore@kernel.org, Samer El-Haj-Mahmoud, mw@semihalf.com,
	upstream@semihalf.com, pete@akeo.ie, Wang, Jian J, Wu, Hao A,
	Bi, Dandan, Dong, Eric, Sunny Wang

Hi Greg,

I just had an offline discussion with Zhichao. Zhichao made a good point. It looks like this feature requires platform code to add PCD override in DynamicHii type in its platform .dsc file. Otherwise, the  PcdSet32S() call won't initiate the "BootDiscoveryPolicy" variable.
Therefore, could you add a comment to the dec file to say that the PCD override in DynamicHii type is required for enabling this feature?  You can take the comment for PcdTcgPhysicalPresenceInterfaceVer in \SecurityPkg\SecurityPkg.dec below as a reference for adding your comment for PcdBootDiscoveryPolicy in MdeModulePkg/MdeModulePkg.dec.

  # To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>
  # For example, map to TCG2_VERSION.PpiVersion to be configured by Tcg2ConfigDxe driver.<BR>
  # gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS<BR>

Best Regards,
Sunny Wang

-----Original Message-----
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Wednesday, July 21, 2021 1:14 PM
To: Grzegorz Bernacki <gjb@semihalf.com>
Cc: devel@edk2.groups.io; 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; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add BootDiscoveryPolicyUiLib.

OK. I am not familiar with PCD, it is new usage for me. And now I got to know the reason. But seems the behavior would be different base on the initialization on DSC file.
Whatever, this patch is OK to me. Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Friday, July 9, 2021 5:55 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>
> Cc: devel@edk2.groups.io; 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
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add
> BootDiscoveryPolicyUiLib.
>
> 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@intel.com> napisał(a):
> >
> > See below comments.
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <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
> > > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> > > <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Grzegorz
> > > Bernacki <gjb@semihalf.com>; Sunny Wang <sunny.wang@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@semihalf.com>
> > > Reviewed-by: Sunny Wang <sunny.wang@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
> > >
> > >
> > >
> > > 
> > >
> >
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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-07-21  7:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-06 10:44 [PATCH v2 0/2] Add BootDiscoveryPolicyUiLib Grzegorz Bernacki
2021-07-06 10:44 ` [PATCH v2 1/1] MdeModulePkg: " Grzegorz Bernacki
2021-07-08  8:08   ` [edk2-devel] " Gao, Zhichao
2021-07-09  9:55     ` Grzegorz Bernacki
2021-07-21  5:14       ` Gao, Zhichao
2021-07-21  7:27         ` Sunny Wang
2021-07-06 10:44 ` [edk2-platforms PATCH v2 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy Grzegorz Bernacki
2021-07-08  8:15   ` [edk2-devel] " Gao, Zhichao
2021-07-09  9:47     ` Grzegorz Bernacki
2021-07-06 10:44 ` [edk2-platforms PATCH v2 2/2] Revert "Platform/RaspberryPi: Setup option for disabling Fast Boot" Grzegorz Bernacki
2021-07-08 18:36   ` Samer El-Haj-Mahmoud

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox