public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for universal UEFI payload
       [not found] <175E86BB586C3E96.27409@groups.io>
@ 2023-05-12 23:40 ` Guo Dong
  2023-05-15  1:08   ` 回复: " gaoliming
  0 siblings, 1 reply; 2+ messages in thread
From: Guo Dong @ 2023-05-12 23:40 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming
  Cc: Ni, Ray, Rhodes, Sean, Lu, James, Guo, Gua


Hi Liming,
I know EDK2 is code freeze now. This patch fixes the UEFI payload boot to shell bug and it got reviewed and should have no impact to other packages.
Do you think we could merge it for this stable release?
https://github.com/tianocore/edk2/pull/4377

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
Sent: Friday, May 12, 2023 3:44 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>
Subject: [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for universal UEFI payload

From: Guo Dong <guo.dong@intel.com>

After moving BDS driver to a new FV for universal UEFI payload, the shell boot option path is not correct since it used the BDS FV instead of DXE FV in its device path.
This patch would find the correct FV by reading shell file.
It also removed PcdShellFile by using gUefiShellFileGuid.

Signed-off-by: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Reviewed-by: James Lu <james.lu@intel.com>
Reviewed-by: Gua Guo <gua.guo@intel.com>
---
 UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c      | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  5 +++--
 UefiPayloadPkg/UefiPayloadPkg.dec                                        |  3 ---
 3 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 62637ae6aa..1660d6c330 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.
+++ c
@@ -2,7 +2,7 @@
   This file include all platform action which can be customized   by IBV/OEM. -Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -11,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PlatformConsole.h" #include <Guid/BootManagerMenu.h> #include <Library/HobLib.h>+#include <Protocol/FirmwareVolume2.h>  /**   Signal EndOfDxe event and install SMM Ready to lock protocol.@@ -89,6 +90,77 @@ PlatformFindLoadOption (
   return -1; } +/**+  Get the FV device path for the shell file.++  @return   A pointer to device path structure.+**/+EFI_DEVICE_PATH_PROTOCOL *+BdsGetShellFvDevicePath (+  VOID+  )+{+  UINTN                          FvHandleCount;+  EFI_HANDLE                     *FvHandleBuffer;+  UINTN                          Index;+  EFI_STATUS                     Status;+  EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;+  UINTN                          Size;+  UINT32                         AuthenticationStatus;+  EFI_DEVICE_PATH_PROTOCOL       *DevicePath;+  EFI_FV_FILETYPE                FoundType;+  EFI_FV_FILE_ATTRIBUTES         FileAttributes;++  Status = EFI_SUCCESS;+  gBS->LocateHandleBuffer (+         ByProtocol,+         &gEfiFirmwareVolume2ProtocolGuid,+         NULL,+         &FvHandleCount,+         &FvHandleBuffer+         );++  for (Index = 0; Index < FvHandleCount; Index++) {+    Size   = 0;+    gBS->HandleProtocol (+           FvHandleBuffer[Index],+           &gEfiFirmwareVolume2ProtocolGuid,+           (VOID **)&Fv+           );+    Status = Fv->ReadFile (+                   Fv,+                   &gUefiShellFileGuid,+                   NULL,+                   &Size,+                   &FoundType,+                   &FileAttributes,+                   &AuthenticationStatus+                   );+    if (!EFI_ERROR (Status)) {+      //+      // Found the shell file+      //+      break;+    }+  }++  if (EFI_ERROR (Status)) {+    if (FvHandleCount) {+      FreePool (FvHandleBuffer);+    }++    return NULL;+  }++  DevicePath = DevicePathFromHandle (FvHandleBuffer[Index]);++  if (FvHandleCount) {+    FreePool (FvHandleBuffer);+  }++  return DevicePath;+}+ /**   Register a boot option using a file GUID in the FV. @@ -109,15 +181,11 @@ PlatformRegisterFvBootOption (
   EFI_BOOT_MANAGER_LOAD_OPTION       *BootOptions;   UINTN                              BootOptionCount;   MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;-  EFI_LOADED_IMAGE_PROTOCOL          *LoadedImage;   EFI_DEVICE_PATH_PROTOCOL           *DevicePath; -  Status = gBS->HandleProtocol (gImageHandle, &gEfiLoadedImageProtocolGuid, (VOID **)&LoadedImage);-  ASSERT_EFI_ERROR (Status);-   EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);   DevicePath = AppendDevicePathNode (-                 DevicePathFromHandle (LoadedImage->DeviceHandle),+                 BdsGetShellFvDevicePath (),                  (EFI_DEVICE_PATH_PROTOCOL *)&FileNode                  ); @@ -248,7 +316,7 @@ PlatformBootManagerAfterConsole (
   //   // Register UEFI Shell   //-  PlatformRegisterFvBootOption (PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE);+  PlatformRegisterFvBootOption (&gUefiShellFileGuid, L"UEFI Shell", LOAD_OPTION_ACTIVE);    if (FixedPcdGetBool (PcdBootManagerEscape)) {     Print (diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index f9626175e2..a3951b7a7e 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerL
+++ ib.inf
@@ -1,7 +1,7 @@
 ## @file #  Include all platform action which can be customized by IBV/OEM. #-#  Copyright (c) 2012 - 2021, Intel Corporation. All rights reserved.<BR>+#  Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR> #  SPDX-License-Identifier: BSD-2-Clause-Patent # ##@@ -32,6 +32,7 @@
   MdePkg/MdePkg.dec   MdeModulePkg/MdeModulePkg.dec   UefiPayloadPkg/UefiPayloadPkg.dec+  ShellPkg/ShellPkg.dec  [LibraryClasses]   BaseLib@@ -52,6 +53,7 @@
 [Guids]   gEfiEndOfDxeEventGroupGuid   gEdkiiBootManagerMenuFileGuid+  gUefiShellFileGuid  [Protocols]   gEfiGenericMemTestProtocolGuid  ## CONSUMES@@ -69,7 +71,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn   gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand-  gUefiPayloadPkgTokenSpaceGuid.PcdShellFile   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParitydiff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
index a23a7b5a78..8d111f3a90 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -67,9 +67,6 @@ gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemSize|0|UINT32|0x10000002
 ## Save bootloader parameter gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameter|0|UINT64|0x10000004 -## FFS filename to find the shell application.-gUefiPayloadPkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1c, 0x4f, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }|VOID*|0x10000005- ## Used to help reduce fragmentation in the EFI memory map gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0x19|UINT32|0x10000012 gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0x04|UINT32|0x10000013--
2.39.1.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104812): https://edk2.groups.io/g/devel/message/104812
Mute This Topic: https://groups.io/mt/98859456/1781375
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [guo.dong@intel.com]
-=-=-=-=-=-=



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

* 回复: [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for universal UEFI payload
  2023-05-12 23:40 ` [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for universal UEFI payload Guo Dong
@ 2023-05-15  1:08   ` gaoliming
  0 siblings, 0 replies; 2+ messages in thread
From: gaoliming @ 2023-05-15  1:08 UTC (permalink / raw)
  To: devel, guo.dong
  Cc: 'Ni, Ray', 'Rhodes, Sean', 'Lu, James',
	'Guo, Gua'

Guo:
  This is a bug fix. I think it is OK to merge for this stable tag. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Guo Dong
> 发送时间: 2023年5月13日 7:40
> 收件人: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>
> 抄送: Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>;
> Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>
> 主题: Re: [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for
> universal UEFI payload
> 
> 
> Hi Liming,
> I know EDK2 is code freeze now. This patch fixes the UEFI payload boot to
> shell bug and it got reviewed and should have no impact to other packages.
> Do you think we could merge it for this stable release?
> https://github.com/tianocore/edk2/pull/4377
> 
> Thanks,
> Guo
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
> Sent: Friday, May 12, 2023 3:44 PM
> To: devel@edk2.groups.io
> Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes,
> Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua
> <gua.guo@intel.com>
> Subject: [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for
> universal UEFI payload
> 
> From: Guo Dong <guo.dong@intel.com>
> 
> After moving BDS driver to a new FV for universal UEFI payload, the shell
boot
> option path is not correct since it used the BDS FV instead of DXE FV in
its
> device path.
> This patch would find the correct FV by reading shell file.
> It also removed PcdShellFile by using gUefiShellFileGuid.
> 
> Signed-off-by: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: James Lu <james.lu@intel.com>
> Cc: Gua Guo <gua.guo@intel.com>
> Reviewed-by: James Lu <james.lu@intel.com>
> Reviewed-by: Gua Guo <gua.guo@intel.com>
> ---
>  UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> | 82
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++-------
> 
> UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i
> nf |  5 +++--
>  UefiPayloadPkg/UefiPayloadPkg.dec
> |  3 ---
>  3 files changed, 78 insertions(+), 12 deletions(-)
> 
> diff --git
> a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> index 62637ae6aa..1660d6c330 100644
> ---
> a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> +++
> b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.
> +++ c
> @@ -2,7 +2,7 @@
>    This file include all platform action which can be customized   by
> IBV/OEM. -Copyright (c) 2015 - 2021, Intel Corporation. All rights
> reserved.<BR>+Copyright (c) 2015 - 2023, Intel Corporation. All rights
> reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -11,6
> +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "PlatformConsole.h" #include <Guid/BootManagerMenu.h>
> #include <Library/HobLib.h>+#include <Protocol/FirmwareVolume2.h>  /**
> Signal EndOfDxe event and install SMM Ready to lock protocol.@@ -89,6
> +90,77 @@ PlatformFindLoadOption (
>    return -1; } +/**+  Get the FV device path for the shell file.++
@return
> A pointer to device path structure.+**/+EFI_DEVICE_PATH_PROTOCOL
> *+BdsGetShellFvDevicePath (+  VOID+  )+{+  UINTN
> FvHandleCount;+  EFI_HANDLE                     *FvHandleBuffer;+
> UINTN                          Index;+  EFI_STATUS
> Status;+  EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;+  UINTN
> Size;+  UINT32                         AuthenticationStatus;+
> EFI_DEVICE_PATH_PROTOCOL       *DevicePath;+  EFI_FV_FILETYPE
> FoundType;+  EFI_FV_FILE_ATTRIBUTES         FileAttributes;++  Status
> = EFI_SUCCESS;+  gBS->LocateHandleBuffer (+         ByProtocol,+
> &gEfiFirmwareVolume2ProtocolGuid,+         NULL,+
> &FvHandleCount,+         &FvHandleBuffer+         );++  for (Index =
> 0; Index < FvHandleCount; Index++) {+    Size   = 0;+
> gBS->HandleProtocol (+           FvHandleBuffer[Index],+
> &gEfiFirmwareVolume2ProtocolGuid,+           (VOID
> **)&Fv+           );+    Status = Fv->ReadFile (+
> Fv,+                   &gUefiShellFileGuid,+
> NULL,+                   &Size,+                   &FoundType,+
> &FileAttributes,+
> &AuthenticationStatus+                   );+    if (!EFI_ERROR (Status))
> {+      //+      // Found the shell file+      //+
> break;+    }+  }++  if (EFI_ERROR (Status)) {+    if (FvHandleCount) {+
> FreePool (FvHandleBuffer);+    }++    return NULL;+  }++  DevicePath =
> DevicePathFromHandle (FvHandleBuffer[Index]);++  if (FvHandleCount) {+
> FreePool (FvHandleBuffer);+  }++  return DevicePath;+}+ /**   Register a
> boot option using a file GUID in the FV. @@ -109,15 +181,11 @@
> PlatformRegisterFvBootOption (
>    EFI_BOOT_MANAGER_LOAD_OPTION       *BootOptions;   UINTN
> BootOptionCount;   MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;-
> EFI_LOADED_IMAGE_PROTOCOL          *LoadedImage;
> EFI_DEVICE_PATH_PROTOCOL           *DevicePath; -  Status =
> gBS->HandleProtocol (gImageHandle, &gEfiLoadedImageProtocolGuid, (VOID
> **)&LoadedImage);-  ASSERT_EFI_ERROR (Status);-
> EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);   DevicePath =
> AppendDevicePathNode (-                 DevicePathFromHandle
> (LoadedImage->DeviceHandle),+
> BdsGetShellFvDevicePath (),
> (EFI_DEVICE_PATH_PROTOCOL *)&FileNode                  ); @@
> -248,7 +316,7 @@ PlatformBootManagerAfterConsole (
>    //   // Register UEFI Shell   //-  PlatformRegisterFvBootOption
> (PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE);+
> PlatformRegisterFvBootOption (&gUefiShellFileGuid, L"UEFI Shell",
> LOAD_OPTION_ACTIVE);    if (FixedPcdGetBool (PcdBootManagerEscape))
> {     Print (diff --git
> a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib
> .inf
> b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib
> .inf
> index f9626175e2..a3951b7a7e 100644
> ---
> a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib
> .inf
> +++
> b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerL
> +++ ib.inf
> @@ -1,7 +1,7 @@
>  ## @file #  Include all platform action which can be customized by
> IBV/OEM. #-#  Copyright (c) 2012 - 2021, Intel Corporation. All rights
> reserved.<BR>+#  Copyright (c) 2012 - 2023, Intel Corporation. All rights
> reserved.<BR> #  SPDX-License-Identifier: BSD-2-Clause-Patent # ##@@
> -32,6 +32,7 @@
>    MdePkg/MdePkg.dec   MdeModulePkg/MdeModulePkg.dec
> UefiPayloadPkg/UefiPayloadPkg.dec+  ShellPkg/ShellPkg.dec
> [LibraryClasses]   BaseLib@@ -52,6 +53,7 @@
>  [Guids]   gEfiEndOfDxeEventGroupGuid
> gEdkiiBootManagerMenuFileGuid+  gUefiShellFileGuid  [Protocols]
> gEfiGenericMemTestProtocolGuid  ## CONSUMES@@ -69,7 +71,6 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn
> gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand-
> gUefiPayloadPkgTokenSpaceGuid.PcdShellFile
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParitydiff --git
> a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
> index a23a7b5a78..8d111f3a90 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dec
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dec
> @@ -67,9 +67,6 @@
> gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemSize|0|UINT32|0x10000
> 002
>  ## Save bootloader parameter
> gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameter|0|UINT64|0x100
> 00004 -## FFS filename to find the shell
> application.-gUefiPayloadPkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5,
0x04,
> 0x7C, 0x3E, 0x9E, 0x1c, 0x4f, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4,
> 0xD1 }|VOID*|0x10000005- ## Used to help reduce fragmentation in the EFI
> memory map
> gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0x
> 19|UINT32|0x10000012
> gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0x04|
> UINT32|0x10000013--
> 2.39.1.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#104812):
> https://edk2.groups.io/g/devel/message/104812
> Mute This Topic: https://groups.io/mt/98859456/1781375
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [guo.dong@intel.com]
> -=-=-=-=-=-=
> 
> 
> 
> 
> 
> 




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

end of thread, other threads:[~2023-05-15  1:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <175E86BB586C3E96.27409@groups.io>
2023-05-12 23:40 ` [edk2-devel][PATCH V2] UefiPayloadPkg: Fix boot shell issue for universal UEFI payload Guo Dong
2023-05-15  1:08   ` 回复: " gaoliming

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