[PATCH 0/2] BoardModulePkg: BoardBdsHookLib GCC fix

[PATCH 0/2] BoardModulePkg: BoardBdsHookLib GCC fix

[Abdul Lateef Attar]

 Patch 1: Fix the GCC boot time page fault exception,
  by copying PcdGetPtr to local variables.
 Patch 2: Adds PCD to hold the UEFI shell GUID value
  and description, to provide flexibility to load
  any UEFI application as UEFI shell.

PR : https://github.com/tianocore/edk2-platforms/pull/50

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
CC: Isaac Oram <isaac.w.oram@intel.com>

Abdul Lateef Attar (2):
  BoardModulePkg: Copy device path before processing
  BoardModulePkg: Adds PCD to load UEFI Shell image

 .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  5 +++++
 .../BoardBdsHookLib/BoardBdsHookLib.inf       |  3 +++
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----
 .../Library/BoardBdsHookLib/BoardBootOption.c |  8 +++++---
 4 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/2] BoardModulePkg: Copy device path before processing

[Abdul Lateef Attar]

From: Abdul Lateef Attar <abdattar@amd.com>

GCC compiler puts the DevicePath PCDs to the read-only
section. During boot if try to process the device path
after PtrGetPtr it throws a page fault exception.

Hence making a local copy using DuplicateDevicePath()
to avoid the page fault exception.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index 0bcee7c9a4ba..8700118d255a 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
@@ -3,6 +3,7 @@
   implementation instance of 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
 
 **/
@@ -131,7 +132,7 @@ IsTrustedConsole (
 
   switch (ConsoleType) {
     case ConIn:
-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleInputDevicePath);
+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr (PcdTrustedConsoleInputDevicePath));
       break;
     case ConOut:
       //
@@ -147,7 +148,7 @@ IsTrustedConsole (
         TempDevicePath = NextDevicePathNode (TempDevicePath);
       }
 
-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleOutputDevicePath);
+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr (PcdTrustedConsoleOutputDevicePath));
       break;
     default:
       ASSERT (FALSE);
@@ -171,7 +172,9 @@ IsTrustedConsole (
   } while (TempDevicePath != NULL);
 
   FreePool (ConsoleDevice);
-
+  if (TrustedConsoleDevicepath != NULL) {
+    FreePool (TrustedConsoleDevicepath);
+  }
   return FALSE;
 }
 
@@ -624,7 +627,7 @@ ConnectTrustedStorage (
   EFI_STATUS                Status;
   EFI_HANDLE                DeviceHandle;
 
-  TrustedStorageDevicepath = PcdGetPtr (PcdTrustedStorageDevicePath);
+  TrustedStorageDevicepath = DuplicateDevicePath (PcdGetPtr (PcdTrustedStorageDevicePath));
   DumpDevicePath (L"TrustedStorage", TrustedStorageDevicepath);
 
   TempDevicePath = TrustedStorageDevicepath;
@@ -649,6 +652,9 @@ ConnectTrustedStorage (
 
     FreePool (Instance);
   } while (TempDevicePath != NULL);
+  if (TrustedStorageDevicepath != NULL) {
+    FreePool (TrustedStorageDevicepath);
+  }
 }
 
 
@@ -1031,7 +1037,7 @@ AddConsoleVariable (
   EFI_HANDLE                GraphicsControllerHandle;
   EFI_DEVICE_PATH           *GopDevicePath;
 
-  TempDevicePath = ConsoleDevicePath;
+  TempDevicePath = DuplicateDevicePath (ConsoleDevicePath);
   do {
     Instance = GetNextDevicePathInstance (&TempDevicePath, &Size);
     if (Instance == NULL) {
@@ -1074,6 +1080,9 @@ AddConsoleVariable (
 
     FreePool (Instance);
   } while (TempDevicePath != NULL);
+  if (TempDevicePath != NULL) {
+    FreePool (TempDevicePath);
+  }
 }
 
 
-- 
2.25.1

[PATCH 2/2] BoardModulePkg: Adds PCD to load UEFI Shell image

[Abdul Lateef Attar]

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/BoardBdsHookLib.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/BoardBootOption.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

Re: [edk2-devel] [PATCH 0/2] BoardModulePkg: BoardBdsHookLib GCC fix

[Attar, AbdulLateef (Abdul Lateef)]

[AMD Official Use Only - General]

Hi Maintainers,
        Could you please review and merge the below patch?
Thanks
AbduL

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul Lateef Attar via groups.io
Sent: 15 November 2022 17:34
To: devel@edk2.groups.io
Cc: Eric Dong <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Isaac Oram <isaac.w.oram@intel.com>
Subject: [edk2-devel] [PATCH 0/2] BoardModulePkg: BoardBdsHookLib GCC fix

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


 Patch 1: Fix the GCC boot time page fault exception,
  by copying PcdGetPtr to local variables.
 Patch 2: Adds PCD to hold the UEFI shell GUID value
  and description, to provide flexibility to load
  any UEFI application as UEFI shell.

PR : https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2-platforms%2Fpull%2F50&amp;data=05%7C01%7CAbdulLateef.Attar%40amd.com%7Cbe5dc1f4cbf941fa88de08dac723cf3d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638041253865897866%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=LwpBjZqUi%2BMCh60aERuXCusAPrKNYk0vbpLrg7dCYic%3D&amp;reserved=0

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
CC: Isaac Oram <isaac.w.oram@intel.com>

Abdul Lateef Attar (2):
  BoardModulePkg: Copy device path before processing
  BoardModulePkg: Adds PCD to load UEFI Shell image

 .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  5 +++++
 .../BoardBdsHookLib/BoardBdsHookLib.inf       |  3 +++
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----  .../Library/BoardBdsHookLib/BoardBootOption.c |  8 +++++---
 4 files changed, 27 insertions(+), 8 deletions(-)

--
2.25.1

Re: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

[Isaac Oram]

Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c 
Line 168: Don't we need to free buffer on this path?
Lines 655, 1083:  Please put a newline between while and the block to free resources.

Regards,
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 1/2] BoardModulePkg: Copy device path before processing

From: Abdul Lateef Attar <abdattar@amd.com>

GCC compiler puts the DevicePath PCDs to the read-only section. During boot if try to process the device path after PtrGetPtr it throws a page fault exception.

Hence making a local copy using DuplicateDevicePath() to avoid the page fault exception.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index 0bcee7c9a4ba..8700118d255a 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHook
+++ Lib.c
@@ -3,6 +3,7 @@
   implementation instance of 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

 

 **/

@@ -131,7 +132,7 @@ IsTrustedConsole (
 

   switch (ConsoleType) {

     case ConIn:

-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleInputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr 
+ (PcdTrustedConsoleInputDevicePath));

       break;

     case ConOut:

       //

@@ -147,7 +148,7 @@ IsTrustedConsole (
         TempDevicePath = NextDevicePathNode (TempDevicePath);

       }

 

-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleOutputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr 
+ (PcdTrustedConsoleOutputDevicePath));

       break;

     default:

       ASSERT (FALSE);

@@ -171,7 +172,9 @@ IsTrustedConsole (
   } while (TempDevicePath != NULL);

 

   FreePool (ConsoleDevice);

-

+  if (TrustedConsoleDevicepath != NULL) {

+    FreePool (TrustedConsoleDevicepath);

+  }

   return FALSE;

 }

 

@@ -624,7 +627,7 @@ ConnectTrustedStorage (
   EFI_STATUS                Status;

   EFI_HANDLE                DeviceHandle;

 

-  TrustedStorageDevicepath = PcdGetPtr (PcdTrustedStorageDevicePath);

+  TrustedStorageDevicepath = DuplicateDevicePath (PcdGetPtr 
+ (PcdTrustedStorageDevicePath));

   DumpDevicePath (L"TrustedStorage", TrustedStorageDevicepath);

 

   TempDevicePath = TrustedStorageDevicepath;

@@ -649,6 +652,9 @@ ConnectTrustedStorage (
 

     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TrustedStorageDevicepath != NULL) {

+    FreePool (TrustedStorageDevicepath);

+  }

 }

 

 

@@ -1031,7 +1037,7 @@ AddConsoleVariable (
   EFI_HANDLE                GraphicsControllerHandle;

   EFI_DEVICE_PATH           *GopDevicePath;

 

-  TempDevicePath = ConsoleDevicePath;

+  TempDevicePath = DuplicateDevicePath (ConsoleDevicePath);

   do {

     Instance = GetNextDevicePathInstance (&TempDevicePath, &Size);

     if (Instance == NULL) {

@@ -1074,6 +1080,9 @@ AddConsoleVariable (
 

     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TempDevicePath != NULL) {

+    FreePool (TempDevicePath);

+  }

 }

 

 

--
2.25.1

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

[Isaac Oram]

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

Re: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

[Attar, AbdulLateef (Abdul Lateef)]

[AMD Official Use Only - General]

Hi Isaac,

Please see inline for my reply. [Abdul]

Thanks
AbduL


-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@intel.com>
Sent: 18 January 2023 02:01
To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
Line 168: Don't we need to free buffer on this path?
[Abdul] we are freeing all allocated buffers here before returning.

Lines 655, 1083:  Please put a newline between while and the block to free resources.
[Abdul] will make the changes in V3 patch.

Regards,
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 1/2] BoardModulePkg: Copy device path before processing

From: Abdul Lateef Attar <abdattar@amd.com>

GCC compiler puts the DevicePath PCDs to the read-only section. During boot if try to process the device path after PtrGetPtr it throws a page fault exception.

Hence making a local copy using DuplicateDevicePath() to avoid the page fault exception.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index 0bcee7c9a4ba..8700118d255a 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHook
+++ Lib.c
@@ -3,6 +3,7 @@
   implementation instance of 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



 **/

@@ -131,7 +132,7 @@ IsTrustedConsole (


   switch (ConsoleType) {

     case ConIn:

-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleInputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedConsoleInputDevicePath));

       break;

     case ConOut:

       //

@@ -147,7 +148,7 @@ IsTrustedConsole (
         TempDevicePath = NextDevicePathNode (TempDevicePath);

       }



-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleOutputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedConsoleOutputDevicePath));

       break;

     default:

       ASSERT (FALSE);

@@ -171,7 +172,9 @@ IsTrustedConsole (
   } while (TempDevicePath != NULL);



   FreePool (ConsoleDevice);

-

+  if (TrustedConsoleDevicepath != NULL) {

+    FreePool (TrustedConsoleDevicepath);

+  }

   return FALSE;

 }



@@ -624,7 +627,7 @@ ConnectTrustedStorage (
   EFI_STATUS                Status;

   EFI_HANDLE                DeviceHandle;



-  TrustedStorageDevicepath = PcdGetPtr (PcdTrustedStorageDevicePath);

+  TrustedStorageDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedStorageDevicePath));

   DumpDevicePath (L"TrustedStorage", TrustedStorageDevicepath);



   TempDevicePath = TrustedStorageDevicepath;

@@ -649,6 +652,9 @@ ConnectTrustedStorage (


     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TrustedStorageDevicepath != NULL) {

+    FreePool (TrustedStorageDevicepath);

+  }

 }





@@ -1031,7 +1037,7 @@ AddConsoleVariable (
   EFI_HANDLE                GraphicsControllerHandle;

   EFI_DEVICE_PATH           *GopDevicePath;



-  TempDevicePath = ConsoleDevicePath;

+  TempDevicePath = DuplicateDevicePath (ConsoleDevicePath);

   do {

     Instance = GetNextDevicePathInstance (&TempDevicePath, &Size);

     if (Instance == NULL) {

@@ -1074,6 +1080,9 @@ AddConsoleVariable (


     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TempDevicePath != NULL) {

+    FreePool (TempDevicePath);

+  }

 }





--
2.25.1