Re: [edk2-devel] [PATCH v3 4/4] OvmfPkg: Refer to Shell app via its declared GUID

["Laszlo Ersek" <lersek@redhat.com>]

On 06/14/19 07:18, Wu, Hao A wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1843
> 
> Currently, the file GUID reference of the UEFI Shell app is indirected
> via the PCD gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile,
> which is set to a fixed value for OvmfPkg.
> 
> So instead, use the symbolic GUID in ShellPkg for this purpose, and drop
> the reference to this PCD, and to the IntelFrameworkModulePkg package
> entirely.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc                                           | 2 --
>  OvmfPkg/OvmfPkgIa32X64.dsc                                        | 2 --
>  OvmfPkg/OvmfPkgX64.dsc                                            | 2 --
>  OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c              | 4 ++--
>  5 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 6830e361bf..473eaba246 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -503,8 +503,6 @@ [PcdsFixedAtBuild]
>    #
>  !include NetworkPkg/NetworkPcds.dsc.inc
>  
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
> -
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
>  !endif
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 372161365c..73f33b7218 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -509,8 +509,6 @@ [PcdsFixedAtBuild.X64]
>    #
>  !include NetworkPkg/NetworkPcds.dsc.inc
>  
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
> -
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
>  !endif
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index e0c60bc5dc..39ac791565 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -508,8 +508,6 @@ [PcdsFixedAtBuild]
>    #
>  !include NetworkPkg/NetworkPcds.dsc.inc
>  
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
> -
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
>  !endif
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index d25e0a417f..060a3ab4c5 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Platform BDS customizations library.
>  #
> -#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -29,10 +29,10 @@ [Sources]
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> -  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>    SourceLevelDebugPkg/SourceLevelDebugPkg.dec
>    OvmfPkg/OvmfPkg.dec
>    SecurityPkg/SecurityPkg.dec
> +  ShellPkg/ShellPkg.dec
>  
>  [LibraryClasses]
>    BaseLib
> @@ -60,7 +60,6 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
>  
>  [Pcd.IA32, Pcd.X64]
>    gEfiMdePkgTokenSpaceGuid.PcdFSBClock
> @@ -77,3 +76,4 @@ [Guids]
>    gEfiXenInfoGuid
>    gEfiEndOfDxeEventGroupGuid
>    gRootBridgesConnectedEventGroupGuid
> +  gUefiShellFileGuid
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 5b15d3bb12..797731a41c 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Platform BDS customizations.
>  
> -  Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -1532,7 +1532,7 @@ PlatformBootManagerAfterConsole (
>    // Register UEFI Shell
>    //
>    PlatformRegisterFvBootOption (
> -    PcdGetPtr (PcdShellFile), L"EFI Internal Shell", LOAD_OPTION_ACTIVE
> +    &gUefiShellFileGuid, L"EFI Internal Shell", LOAD_OPTION_ACTIVE
>      );
>  
>    RemoveStaleFvFileOptions ();
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you!
Laszlo