Re: [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID

[Laszlo Ersek <lersek@redhat.com>]

On 03/22/17 15:04, Ard Biesheuvel wrote:
> 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 our platforms.
> 
> So instead, use the new symbolic GUID added for this purpose, and drop
> the reference to this PCD, and to the IntelFrameworkModulePkg package
> entirely.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc                                           | 1 -
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                                     | 1 -
>  ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 2 +-
>  ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
>  4 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 0bbbe4a7aa4a..4f686faa559c 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -139,7 +139,6 @@ [PcdsFixedAtBuild.common]
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>  
>    #
>    # The maximum physical I/O addressability of the processor, set with
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 71f16ed192de..6b6555c889a3 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -156,7 +156,6 @@ [PcdsFixedAtBuild.AARCH64]
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>  
>    #
>    # The maximum physical I/O addressability of the processor, set with
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 94da51ad49f1..f9591964d577 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -691,7 +691,7 @@ PlatformBootManagerAfterConsole (
>    // Register UEFI Shell
>    //
>    PlatformRegisterFvBootOption (
> -    PcdGetPtr (PcdShellFile), L"EFI Internal Shell", LOAD_OPTION_ACTIVE
> +    &gUefiShellFileGuid, L"EFI Internal Shell", LOAD_OPTION_ACTIVE
>      );
>  
>    RemoveStaleFvFileOptions ();
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 1f162c663fc1..bac6c781b3c8 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -35,11 +35,11 @@ [Sources]
>    QemuKernel.c
>  
>  [Packages]
> -  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
>    OvmfPkg/OvmfPkg.dec
> -  ArmVirtPkg/ArmVirtPkg.dec
> +  ShellPkg/ShellPkg.dec
>  
>  [LibraryClasses]
>    BaseLib
> @@ -58,7 +58,6 @@ [LibraryClasses]
>    UefiRuntimeServicesTableLib
>  
>  [FixedPcd]
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> @@ -74,6 +73,7 @@ [Guids]
>    gEfiFileSystemVolumeLabelInfoIdGuid
>    gEfiEndOfDxeEventGroupGuid
>    gRootBridgesConnectedEventGroupGuid
> +  gUefiShellFileGuid
>  
>  [Protocols]
>    gEfiDevicePathProtocolGuid
> 

Works for me if the shellpkg maintainers are OK with the centralization
of the GUID.

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