Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules

["Brian J. Johnson" <brian.johnson@hpe.com>]

[-- Attachment #1: Type: text/plain, Size: 3415 bytes --]

On 6/18/20 2:01 AM, Dandan Bi wrote:
>
> Hi All,
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816 
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2816>
>
> We plan to separate two kinds of NULL class libraries for Memory and 
> serial handlers from *MdeModulePkg/Universal/StatusCodeHandler/…/ 
> StatusCodeHandlerPei/RuntimeDxe/Smm* modules.
>
> The benefit we want to gain from this separation is to 1) make the 
> code clear and easy to maintain, 2) make platform flexible to choose 
> any handler library they need, and it also can reduce image size since 
> the unused handlers can be excluded.
>
> If you have any concern or comments for this separation, please let me 
> know.
>
> We plan to add new separated NULL class library 
> *MemoryStausCodeHandlerLib *and*SerialStatusCodeHandlerLib *with 
> different phase implementation into *MdeModulePkg\Library\* directory.
>
> The main tree structure may like below:
>
> MdeModulePkg\Library
>
> |------*MemoryStausCodeHandlerLib*
>
> |------|------ PeiMemoryStausCodeHandlerLib.inf
>
> |------|------ RuntimeDxeMemoryStatusCodeHandlerLib.inf
>
> |------|------ SmmMemoryStausCodeHandlerLib.inf
>
> |------*SerialStatusCodeHandlerLib*
>
> |------|------ PeiSerialStatusCodeHandlerLib.inf
>
> |------|------ RuntimeDxeSerialStatusCodeHandlerLib.inf
>
> |------|------ SmmSerialStatusCodeHandlerLib.inf
>
> **
>
> **
>
> We will update existing platform use cases in edk2 and edk2-platform 
> repo to cover the new NULL class library to make sure this change 
> doesn’t impact any platform.
>
> After this separation, StatusCodeHandler module usage will like below, 
> and it’s also very flexible for platform to cover more handler 
> libraries to meet their requirements.
>
> MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
>
>   <LibraryClasses>
>
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
>
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
>
>     …
>
> }
>
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf 
> {
>
>   <LibraryClasses>
>
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
>
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
>
>     …
>
> }
>
> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
>
>   <LibraryClasses>
>
>  NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf
>
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf
>
>     …
>
> }
>
> Thanks,
>
> Dandan
>

Dandan,

We'll have a lot of layers of indirection....  The 
ReportStatusCodeRouter modules will call one or more 
StatusCodeHandlerModules, and the standard StatusCodeHandler modules 
will call multiple StatusCodeHandlerLib libraries.

How about adding StatusCodeHandlerLib support directly to the 
ReportStatusCodeRouter modules?  Then platforms could omit the 
StatusCodeHandler modules if they're only using the open-source code.  
That sounds like less overhead since fewer modules would be needed.

Thanks,

-- 

*Brian J. Johnson
*Enterprise X86 Lab

Hewlett Packard Enterprise

*hpe.com* <3D"hpe.com">


[-- Attachment #2: Type: text/html, Size: 12839 bytes --]