Re: [PATCH] ArmVirtPkg: remove status code support

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 5 July 2017 at 14:43, Laszlo Ersek <lersek@redhat.com> wrote:
> 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
>>
>