Re: [PATCH] ArmVirtPkg: remove status code support

[Laszlo Ersek <lersek@redhat.com>]

On 07/05/17 15:04, Ard Biesheuvel wrote:
> Commit 7b1dc6c569a 'ArmVirtPkg: switch to generic ResetSystemRuntimeDxe'
> replaced all references in ArmVirtPkg to the deprecated ResetRuntimeDxe
> from EmbeddedPkg with the well maintained generic alternative that lives
> in MdeModulePkg.
> 
> However, as it turns out, the generic driver has a dependency on the
> library class ReportStatusCodeLib, whose default resolution is an
> implementation that is not safe for use at runtime, resulting in crashes
> when trying to invoke it from the OS.
> 
> Since we have no use for status codes in any of the ArmVirtPkg
> platforms, let's replace all resolutions with a common one to the NULL
> implementation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)

Alternative approach (if we wish to follow what OVMF does):

(1) Copy the library resolutions (as appropriate) from OvmfPkg:

- SEC, PEI_CORE, PEIM:
  MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf

- DXE_CORE, DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION:
  MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf

- DXE_RUNTIME_DRIVER:
  MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf

(2) Port commit a6d594c5fabd ("OvmfPkg: use StatusCode Router and Handler from MdeModulePkg", 2016-08-03) to ArmVirtPkg.

This should result in status code reporting and handling that is functional at runtime as well.

What do you prefer?

Thanks
Laszlo

> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 0b9b457b5619..bfc40286d7ab 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -154,6 +154,8 @@ [LibraryClasses.common]
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>    UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>  
> +  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> +
>  [LibraryClasses.common.SEC]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> @@ -174,7 +176,6 @@ [LibraryClasses.common.PEI_CORE]
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>    PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>    OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
>    UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> @@ -191,7 +192,6 @@ [LibraryClasses.common.PEIM]
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>    PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>    OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
>    PeiResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf
> @@ -205,13 +205,11 @@ [LibraryClasses.common.DXE_CORE]
>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>    MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
>    DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> -  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>    ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
>    UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>  
>  [LibraryClasses.common.DXE_DRIVER]
> -  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -223,10 +221,8 @@ [LibraryClasses.common.UEFI_APPLICATION]
>    HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
>    ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>    FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
> -  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>  
>  [LibraryClasses.common.UEFI_DRIVER]
> -  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>    UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf
>    ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> @@ -234,7 +230,6 @@ [LibraryClasses.common.UEFI_DRIVER]
>  
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> -  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
> @@ -334,8 +329,6 @@ [PcdsFixedAtBuild.common]
>    #  DEBUG_ERROR     0x80000000  // Error
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|$(DEBUG_PRINT_ERROR_LEVEL)
>  
> -  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> -
>    #
>    # Optional feature to help prevent EFI memory map fragments
>    # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
>