Re: [edk2-devel] [PATCH 2/2] BoardModulePkg: Adds PCD to load UEFI Shell image

["Isaac Oram" <isaac.w.oram@intel.com>]

edk2-platforms/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c 338:  Can you change to mUefiShellFileGuid and initialize it to zero?  It seems misleading to me to have the default value here when it is not used.  I think we should also move the declarations to the beginning of the file.

I am curious why this kind of global variable doesn't trigger PF and PCD does.  If you know off the top of your head.  You do not need to research if the code is functional with GCC.  Also, please add me to CC list so I don't filter the emails.

Thanks,
Isaac


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul Lateef Attar via groups.io
Sent: Tuesday, November 15, 2022 4:04 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [PATCH 2/2] BoardModulePkg: Adds PCD to load UEFI Shell image

defines two PCDs, PcdShellFile and PcdShellFileDesc, which holds the GUID and description of the UEFI shell file to be loaded.

A PCDs based solution gives flexibility to the user to load different images, by just overriding the DSC file.

The user can load a diagnostic image or test image during PCDBootToShellOnly or later stages.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
---
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec          | 5 +++++
 .../Library/BoardBdsHookLib/BoardBdsHookLib.inf           | 3 +++
 .../Library/BoardBdsHookLib/BoardBootOption.c             | 8 +++++---
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index 2953e9527224..73cbd62be030 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -7,6 +7,7 @@
 # for the build infrastructure.

 #

 # Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>

+# Copyright (C) 2022 Advanced Micro Devices, Inc. All rights 
+reserved.<BR>

 #

 # SPDX-License-Identifier: BSD-2-Clause-Patent

 #

@@ -266,6 +267,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUSize|0x00000000|UINT32|0x2000002B

   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUOffset|0x00000000|UINT32|0x2000002C

 

+  # GUID of Shell file to be loaded, default value is 
+ gUefiShellFileGuid define in ShellPkg.dec

+  gMinPlatformPkgTokenSpaceGuid.PcdShellFile|{GUID({0x7c04a583, 0x9e3e, 
+ 0x4f1c, {0xad, 0x65, 0xe0, 0x52, 0x68, 0xd0, 0xb4, 
+ 0xd1}})}|VOID*|0x20000230

+  gMinPlatformPkgTokenSpaceGuid.PcdShellFileDesc|L"Internal UEFI Shell 
+ 2.0"|VOID*|0x20000231

+

 [PcdsDynamic, PcdsDynamicEx]

   gMinPlatformPkgTokenSpaceGuid.PcdPcIoApicEnable|0x0|UINT32|0x90000019

   gMinPlatformPkgTokenSpaceGuid.PcdAcpiSleepControlRegisterAddressSpaceId|0x00|UINT8|0x0001004B

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.inf b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.inf
index 69f3fcb55222..e2ac73498b90 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.inf
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHook
+++ Lib.inf
@@ -2,6 +2,7 @@
 # Module Information file for the Bds Hook Library.

 #

 # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

+# Copyright (C) 2022 Advanced Micro Devices, Inc. All rights 
+reserved.<BR>

 #

 # SPDX-License-Identifier: BSD-2-Clause-Patent

 #

@@ -59,6 +60,8 @@ [Pcd]
   gMinPlatformPkgTokenSpaceGuid.PcdTrustedConsoleInputDevicePath    ## CONSUMES

   gMinPlatformPkgTokenSpaceGuid.PcdTrustedConsoleOutputDevicePath   ## CONSUMES

   gMinPlatformPkgTokenSpaceGuid.PcdTrustedStorageDevicePath         ## CONSUMES

+  gMinPlatformPkgTokenSpaceGuid.PcdShellFile                        ## CONSUMES

+  gMinPlatformPkgTokenSpaceGuid.PcdShellFileDesc                    ## CONSUMES

 

 [Sources]

   BoardBdsHook.h

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index dec3ce93ef71..de1676dad0c7 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOpt
+++ ion.c
@@ -2,6 +2,8 @@
   Driver for Platform Boot Options support.

 

 Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

+Copyright (C) 2022 Advanced Micro Devices, Inc. All rights 
+reserved.<BR>

+

 SPDX-License-Identifier: BSD-2-Clause-Patent

 

 **/

@@ -335,7 +337,6 @@ PlatformBootManagerWaitCallback (
 

 EFI_GUID gUefiShellFileGuid = { 0x7C04A583, 0x9E3E, 0x4f1c, { 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 } };

 

-#define INTERNAL_UEFI_SHELL_NAME      L"Internal UEFI Shell 2.0"

 #define UEFI_HARD_DRIVE_NAME          L"UEFI Hard Drive"

 

 /**

@@ -352,7 +353,8 @@ RegisterDefaultBootOption (
 

     ShellData = NULL;

     ShellDataSize = 0;

-    RegisterFvBootOption (&gUefiShellFileGuid,      INTERNAL_UEFI_SHELL_NAME, (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8 *)ShellData, ShellDataSize);

+    CopyMem (&gUefiShellFileGuid, PcdGetPtr (PcdShellFile), sizeof 
+ (GUID));

+    RegisterFvBootOption (&gUefiShellFileGuid, (CHAR16 *) PcdGetPtr 
+ (PcdShellFileDesc), (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8 
+ *)ShellData, ShellDataSize);

 

   //

   // Boot Menu

@@ -557,7 +559,7 @@ BootOptionPriority (
       return 6;

 

     }

-    if (StrCmp (BootOption->Description, INTERNAL_UEFI_SHELL_NAME) == 0) {

+    if (StrCmp (BootOption->Description, (CHAR16 *) PcdGetPtr 
+ (PcdShellFileDesc)) == 0) {

       if (PcdGetBool (PcdBootToShellOnly)) {

         return 0;

       }

--
2.25.1