Re: [PATCH] ArmVirtPkg: remove status code support

[Laszlo Ersek <lersek@redhat.com>]

On 07/05/17 15:46, Ard Biesheuvel wrote:
> On 5 July 2017 at 14:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> 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?
>>>
> 
> 
> Depends on what status codes actually buy us. I'm clueless
> 

I was similarly clueless :) which is why I asked Cinnamon back then [*]
to write a detailed commit message for what would become commit
a6d594c5fabd.

[*] https://lists.01.org/pipermail/edk2-devel/2016-August/000199.html

So, if you read that message, you'll see that roughly, it buys us the
following in practice: when a module (boot time or runtime) calls one of
the REPORT_STATUS_CODE*() macros, a status code message will be printed
to the serial port.

That's it :)

I don't think it's extremely useful in practice for in-tree modules,
given that we -- ArmVirtPkg and OvmfPkg users -- prefer building all
modules with high DEBUG levels, add DEBUGs liberally to our own platform
code, and always capture the DEBUG output (serial port or QEMU debug
port) during boot.

For working with out-of-tree modules that rely heavily on status code
reporting, or else just to showcase/test the related PPIs and protocols
(which is arguably one of the goals of the in-tree virt platforms),
enabling the status code libs and drivers can be considered useful.

Given that OvmfPkg is already serving this showcase/test purpose, I'm
fine with disabling status code stuff in ArmVirtPkg for good; I just
wanted you to consider the alternative. So, what say you? :)

Thanks
Laszlo