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

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

2020-06-18 Thread Dandan Bi

Hi All,

REF: 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 {
  
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
...
}

MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
  {
  
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
...
}

MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
  

NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf
...
}


Thanks,
Dandan

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61467): https://edk2.groups.io/g/devel/message/61467
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-19 Thread Laszlo Ersek

On 06/18/20 09:01, Dandan Bi wrote:
> Hi All,
> 
> REF: 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 {
>   
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
> ...
> }
> 
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
>   {
>   
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
> ...
> }
> 
> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
>   
> 
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf
> ...
> }

So I assume you're going to remove PcdStatusCodeUseSerial and
PcdStatusCodeUseMemory, and when converting the existent platforms, the
new NULL class resolutions in the DSC files will reflect the specific
PCD values used in those DSC files until then. Is that right?

I'm OK with it.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61516): https://edk2.groups.io/g/devel/message/61516
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-19 Thread Brian J. Johnson


On 6/18/20 2:01 AM, Dandan Bi wrote:


Hi All,

REF: 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 {

  

NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf

NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf

    …

}

MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf 
{


  

NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf

NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf

    …

}

MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {

  

 
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">


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61533): https://edk2.groups.io/g/devel/message/61533
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-19 Thread Andrew Fish via groups.io



> On Jun 19, 2020, at 10:29 AM, Brian J. Johnson  wrote:
> 
> On 6/18/20 2:01 AM, Dandan Bi wrote:
>> Hi All,
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816 
>> 
>>
>> We plan to separate two kinds of NULL class libraries for Memory and serial 
>> handlers fromMdeModulePkg/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 {
>>   
>> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
>> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
>> …
>> }
>>
>> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
>>   {
>>   
>> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
>> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
>> …
>> }
>>
>> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
>>   
>> 
>> 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.
> 
> 

I think the need to execute from ROM makes this tricky. 

It looks to me that it is easy to move from PCD to libs for the 
StatusCodeHandler since registration is basically `RscHandlerPpi->Register 
(SerialStatusCodeReportWorker);`. The issue I see is the ReportStatusCodeRouter 
publishes RscHandlerPpi after the PEIMs constructor has been called and if the 
PEIM. Given globals don’t work when running from ROM you would have to do 
something like publish a HOB in the library constructor and then have the 
GenericStatusCodePeiEntry() walk the HOBs and install the handlers. So I guess 
it is a little easier than I 1st thought when I started writing this mail, but 
it would require a new public API. 

Thanks,

Andrew Fish
> Thanks,
> 
> -- 
> Brian J. Johnson
> Enterprise X86 Lab
> 
> Hewlett Packard Enterprise
> 
> hpe.com 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61536): https://edk2.groups.io/g/devel/message/61536
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-19 Thread Dong, Eric

From: devel@edk2.groups.io  On Behalf Of Brian J. Johnson
Sent: Saturday, June 20, 2020 1:29 AM
To: devel@edk2.groups.io; Bi, Dandan ; r...@edk2.groups.io
Cc: Dong, Eric ; Ni, Ray ; Wang, Jian J 
; Wu, Hao A ; Tan, Ming 

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

On 6/18/20 2:01 AM, Dandan Bi wrote:
Hi All,

REF: 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 {

NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
...
}

MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
  {

NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
...
}

MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {

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

Hi Brain,

You are right. Current design truly has a lot of layers. The 
ReportStatusCodeRouter module provides the register logic and maintain the 
registered status code handlers. Now the platform may have more than one of 
drivers used to register the status code handler.  This RFC used to resolve the 
platform has more than one status code handler drivers' issue. We expect the 
platform only need one wrapper driver in MdeModulePkg to let the status code 
handler library to register its handler on it.

Thanks,

Eric

Thanks,
--

Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise

hpe.com<3D%22hpe.com%22>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61543): https://edk2.groups.io/g/devel/message/61543
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-21 Thread Dandan Bi

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: Friday, June 19, 2020 8:48 PM
> To: r...@edk2.groups.io; Bi, Dandan ;
> devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Wang,
> Jian J ; Wu, Hao A ; Tan,
> Ming 
> Subject: Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler:
> Separate NULL class libraries for Memory and serial handlers from
> MdeModulePkg/Universal/StatusCodeHandler modules
> 
> On 06/18/20 09:01, Dandan Bi wrote:
> > Hi All,
> >
> > REF: 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.in
> f {
> >   
> >
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemorySt
> ausCode
> > NULL|HandlerLib.inf
> >
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusC
> o
> > NULL|deHandlerLib.inf
> > ...
> > }
> >
> >
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.inf  {
> >   
> >
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeM
> emorySt
> > NULL|ausCodeHandlerLib.inf
> >
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSeri
> alS
> > NULL|tatusCodeHandlerLib.inf
> > ...
> > }
> >
> >
> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSm
> m.inf {
> >   
> >
> >
> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemory
> StausCode
> > HandlerLib.inf
> >
> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatus
> Co
> > NULL|deHandlerLib.inf
> > ...
> > }
> 
> So I assume you're going to remove PcdStatusCodeUseSerial and
> PcdStatusCodeUseMemory, and when converting the existent platforms,
> the new NULL class resolutions in the DSC files will reflect the specific PCD
> values used in those DSC files until then. Is that right?
> 
Thanks for pointing out the PCD part which I miss in this RFC.
This commit 
https://github.com/tianocore/edk2/commit/45bc28172fbf38ac21e2592c07189b55f57695e3
 have updated PcdStatusCodeUseSerial and PcdStatusCodeUseMemory type.
We plan to keep PcdStatusCodeUseSerial and PcdStatusCodeUseMemory.  Through 
NULL class resolutions in the DSC can make the code handler code included or 
not, then we still can control handler enable/disable through the PCD 
dynamically if the handler is included.
What do you think of this?


Thanks,
Dandan
> I'm OK with it.
> 
> Thanks
> Laszlo
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61550): https://edk2.groups.io/g/devel/message/61550
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-22 Thread Dandan Bi

Hi Brian,

Personally, I prefer to add the NULL class Library to StatusCodeHandler modules.

  1.  I think we should make the functionality of each module clear and 
separated. It may also be why we added ReportStatusCodeRouter and 
StatusCodeHandler modules in edk2 repo before.

ReportStatusCodeRouter modules are responsible for producing Status Code 
Protocol/PPI and Report Status Code Handler Protocol/PPI, and StatusCodeHandler 
modules are responsible for producing handlers (Handlers can be provided by 
NULL class Libraries in this RFC).

 So, that’s why I don’t want to add the NULL class Library to 
ReportStatusCodeRouter modules directly, which change the functionality scope 
of existing modules.



  1.  I agree that we have a lot of layers of indirection now, but what we may 
gain is the good modularity. And you also mentioned that one or more 
StatusCodeHandler Modules may be used. We also want to achieve that only the 
StatusCodeHandler modules in MdeModulePkg can be used after this separation, 
platform can only add its own handler Libs to meet its requirement.



  1.  As Andrew mentioned below, if add the libraries to 
ReportStatusCodeRouter, there will be some issue we need to fix, which seems 
also make the code logic a little tricky to me.



Thanks,
Dandan
From: Andrew Fish 
Sent: Saturday, June 20, 2020 2:04 AM
To: edk2-devel-groups-io ; brian.john...@hpe.com
Cc: Bi, Dandan ; r...@edk2.groups.io; Dong, Eric 
; Ni, Ray ; Wang, Jian J 
; Wu, Hao A ; Tan, Ming 

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




On Jun 19, 2020, at 10:29 AM, Brian J. Johnson 
mailto:brian.john...@hpe.com>> wrote:

On 6/18/20 2:01 AM, Dandan Bi wrote:
Hi All,

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816

We plan to separate two kinds of NULL class libraries for Memory and serial 
handlers fromMdeModulePkg/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 {
  
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
…
}

MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
  {
  
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
…
}

MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
  

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.


I think the need to execute from ROM makes this tricky.

It looks to me that it is easy to move from PCD to libs for the 
StatusCodeHandler since registration is basically `RscHandlerPpi->Register 
(SerialStatusCodeReportWorker);`. The issue I see is the ReportStatusCodeRouter 
publishes RscHandlerPpi after th

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

2020-06-22 Thread Laszlo Ersek

On 06/22/20 06:57, Bi, Dandan wrote:
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Laszlo
>> Ersek
>> Sent: Friday, June 19, 2020 8:48 PM
>> To: r...@edk2.groups.io; Bi, Dandan ;
>> devel@edk2.groups.io
>> Cc: Dong, Eric ; Ni, Ray ; Wang,
>> Jian J ; Wu, Hao A ; Tan,
>> Ming 
>> Subject: Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler:
>> Separate NULL class libraries for Memory and serial handlers from
>> MdeModulePkg/Universal/StatusCodeHandler modules
>>
>> On 06/18/20 09:01, Dandan Bi wrote:
>>> Hi All,
>>>
>>> REF: 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.in
>> f {
>>>   
>>>
>> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemorySt
>> ausCode
>>> NULL|HandlerLib.inf
>>>
>> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusC
>> o
>>> NULL|deHandlerLib.inf
>>> ...
>>> }
>>>
>>>
>> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
>> dlerRuntimeDxe.inf  {
>>>   
>>>
>> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeM
>> emorySt
>>> NULL|ausCodeHandlerLib.inf
>>>
>> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSeri
>> alS
>>> NULL|tatusCodeHandlerLib.inf
>>> ...
>>> }
>>>
>>>
>> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSm
>> m.inf {
>>>   
>>>
>>>
>> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemory
>> StausCode
>>> HandlerLib.inf
>>>
>> NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatus
>> Co
>>> NULL|deHandlerLib.inf
>>> ...
>>> }
>>
>> So I assume you're going to remove PcdStatusCodeUseSerial and
>> PcdStatusCodeUseMemory, and when converting the existent platforms,
>> the new NULL class resolutions in the DSC files will reflect the specific PCD
>> values used in those DSC files until then. Is that right?
>>
> Thanks for pointing out the PCD part which I miss in this RFC.
> This commit 
> https://github.com/tianocore/edk2/commit/45bc28172fbf38ac21e2592c07189b55f57695e3
>  have updated PcdStatusCodeUseSerial and PcdStatusCodeUseMemory type.
> We plan to keep PcdStatusCodeUseSerial and PcdStatusCodeUseMemory.  Through 
> NULL class resolutions in the DSC can make the code handler code included or 
> not, then we still can control handler enable/disable through the PCD 
> dynamically if the handler is included.
> What do you think of this?

Hm... OK.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61561): https://edk2.groups.io/g/devel/message/61561
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-06-24 Thread Brian J. Johnson


Dandan,

The Status Code Protocol/PPI is the high-level interface which is being 
implemented.  The ReportStatusCodeRouter module implements this in terms 
of the ReportStatusCodeHandler Protocol/PPI.  That allows multiple 
ReportStatusCodeHandler modules to be used at once, so they can each 
react to different types of status codes, or report them through 
multiple channels.  That sort of multiplexing seems like a useful feature.


Now we're considering adding a mechanism which allows registering status 
code handlers via NULL libraries, rather than via the protocol/PPI.  
That sounds like exactly what ReportStatusCodeRouter is intended for.  
It wouldn't really change its scope, it would just make it more 
flexible.  Adding this feature via a StatusCodeHandler module wouldn't 
improve modularity, it would just add complexity.  As an OEM, adding a 
custom handler would look the same to me either way:  I would have to 
add the NULL class library to a MdeModulePkg driver's entry in my .dsc 
file.  It doesn't matter to me whether it's the ReportStatusCodeRouter 
or StatusCodeHandler module.  And if I can do it in 
ReportStatusCodeRouter, then I don't need to include any 
StatusCodeHandler modules in the build at all.  That saves code space 
and reduces the number of modules in the APRIORI list, which are both 
good things.


ReportStatusCodeRouterPei already has to track registered handlers in 
PEI when running from ROM (it uses a HOB.)  Tracking handlers from NULL 
libraries wouldn't be any harder.  In fact, it looks like it could just 
export the Register() function to the NULL libraries, and they could 
call it in their constructors.


I think using NULL libraries for status code handlers is a great idea.  
I'd just like to take that opportunity to reduce the complexity of the 
overall status code stack while we're at it.


Thanks,

*Brian J. Johnson
*Enterprise X86 Lab

Hewlett Packard Enterprise


*From:* Bi, Dandan [mailto:dandan...@intel.com]
*Sent:* Monday, June 22, 2020, 2:27 AM
*To:* Andrew Fish , edk2-devel-groups-io 
, brian.john...@hpe.com 
*Cc:* r...@edk2.groups.io , Dong, Eric 
, Ni, Ray , Wang, Jian J 
, Wu, Hao A , Tan, Ming 
, Laszlo Ersek , Bi, Dandan 

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



Hi Brian,

Personally, I prefer to add the NULL class Library to 
StatusCodeHandler modules.


 1. I think we should make the functionality of each module clear and
separated. It may also be why we added ReportStatusCodeRouter and
StatusCodeHandler modules in edk2 repo before.

ReportStatusCodeRouter modules are responsible for producing Status 
Code Protocol/PPI and Report Status Code Handler Protocol/PPI, and 
StatusCodeHandler modules are responsible for producing handlers 
(Handlers can be provided by NULL class Libraries in this RFC).


 So, that’s why I don’t want to add the NULL class Library to 
ReportStatusCodeRouter modules directly, which change the 
functionality scope of existing modules.


 2. I agree that we have a lot of layers of indirection now, but what
we may gain is the good modularity. And you also mentioned that
one or more StatusCodeHandler Modules may be used. We also want to
achieve that only the StatusCodeHandler modules in MdeModulePkg
can be used after this separation, platform can only add its own
handler Libs to meet its requirement.

 3. As Andrew mentioned below, if add the libraries to
ReportStatusCodeRouter, there will be some issue we need to fix,
which seems also make the code logic a little tricky to me.

Thanks,

Dandan

*From:* Andrew Fish 
*Sent:* Saturday, June 20, 2020 2:04 AM
*To:* edk2-devel-groups-io ; brian.john...@hpe.com
*Cc:* Bi, Dandan ; r...@edk2.groups.io; Dong, Eric 
; Ni, Ray ; Wang, Jian J 
; Wu, Hao A ; Tan, Ming 

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




On Jun 19, 2020, at 10:29 AM, Brian J. Johnson
mailto:brian.john...@hpe.com>wrote:

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 ha

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

2020-06-25 Thread Dong, Eric

Hi Brian,

In this new design, we still use register status code handler Protocol/Ppi to 
register the handler logic. We just want to change the StatusCodeHandler 
driver. We try to split the register logic to NULL library to make the code 
more modularity. We already created sample library in Edk2-Platforms repo  
https://github.com/tianocore/edk2-platforms/tree/master/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib.
  You can check this code to understand more about what we want to do.

Thanks,
Eric
From: Brian J. Johnson 
Sent: Thursday, June 25, 2020 4:25 AM
To: Bi, Dandan ; Andrew Fish ; 
edk2-devel-groups-io 
Cc: r...@edk2.groups.io; Dong, Eric ; Ni, Ray 
; Wang, Jian J ; Wu, Hao A 
; Tan, Ming ; Laszlo Ersek 

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

Dandan,

The Status Code Protocol/PPI is the high-level interface which is being 
implemented.  The ReportStatusCodeRouter module implements this in terms of the 
ReportStatusCodeHandler Protocol/PPI.  That allows multiple 
ReportStatusCodeHandler modules to be used at once, so they can each react to 
different types of status codes, or report them through multiple channels.  
That sort of multiplexing seems like a useful feature.

Now we're considering adding a mechanism which allows registering status code 
handlers via NULL libraries, rather than via the protocol/PPI.  That sounds 
like exactly what ReportStatusCodeRouter is intended for.  It wouldn't really 
change its scope, it would just make it more flexible.  Adding this feature via 
a StatusCodeHandler module wouldn't improve modularity, it would just add 
complexity.  As an OEM, adding a custom handler would look the same to me 
either way:  I would have to add the NULL class library to a MdeModulePkg 
driver's entry in my .dsc file.  It doesn't matter to me whether it's the 
ReportStatusCodeRouter or StatusCodeHandler module.  And if I can do it in 
ReportStatusCodeRouter, then I don't need to include any StatusCodeHandler 
modules in the build at all.  That saves code space and reduces the number of 
modules in the APRIORI list, which are both good things.

ReportStatusCodeRouterPei already has to track registered handlers in PEI when 
running from ROM (it uses a HOB.)  Tracking handlers from NULL libraries 
wouldn't be any harder.  In fact, it looks like it could just export the 
Register() function to the NULL libraries, and they could call it in their 
constructors.

I think using NULL libraries for status code handlers is a great idea.  I'd 
just like to take that opportunity to reduce the complexity of the overall 
status code stack while we're at it.

Thanks,

Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise


From: Bi, Dandan [mailto:dandan...@intel.com]
Sent: Monday, June 22, 2020, 2:27 AM
To: Andrew Fish <mailto:af...@apple.com>, edk2-devel-groups-io 
<mailto:devel@edk2.groups.io>, 
brian.john...@hpe.com<mailto:brian.john...@hpe.com> 
<mailto:brian.john...@hpe.com>
Cc: r...@edk2.groups.io<mailto:r...@edk2.groups.io> 
<mailto:r...@edk2.groups.io>, Dong, Eric 
<mailto:eric.d...@intel.com>, Ni, Ray 
<mailto:ray...@intel.com>, Wang, Jian J 
<mailto:jian.j.w...@intel.com>, Wu, Hao A 
<mailto:hao.a...@intel.com>, Tan, Ming 
<mailto:ming....@intel.com>, Laszlo Ersek 
<mailto:ler...@redhat.com>, Bi, Dandan 
<mailto:dandan...@intel.com>
Subject: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL 
class libraries for Memory and serial handlers from 
MdeModulePkg/Universal/StatusCodeHandler modules

Hi Brian,

Personally, I prefer to add the NULL class Library to StatusCodeHandler modules.

  1.  I think we should make the functionality of each module clear and 
separated. It may also be why we added ReportStatusCodeRouter and 
StatusCodeHandler modules in edk2 repo before.

ReportStatusCodeRouter modules are responsible for producing Status Code 
Protocol/PPI and Report Status Code Handler Protocol/PPI, and StatusCodeHandler 
modules are responsible for producing handlers (Handlers can be provided by 
NULL class Libraries in this RFC).

 So, that’s why I don’t want to add the NULL class Library to 
ReportStatusCodeRouter modules directly, which change the functionality scope 
of existing modules.



  1.  I agree that we have a lot of layers of indirection now, but what we may 
gain is the good modularity. And you also mentioned that one or more 
StatusCodeHandler Modules may be used. We also want to achieve that only the 
StatusCodeHandler modules in MdeModulePkg can be used after this separation, 
platform can only add its own handler Libs to meet its requirement.



  1.  As Andrew mentioned below, if add the librari

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

2020-06-25 Thread Brian J. Johnson

Thanks for the link.  I agree that this change will make the 
StatusCodeHandler driver more modular, and is a step in the right direction.


But I think it could go further, with almost no additional work, and 
simplify the overall Status Code mechanism, not just the 
StatusCodeHandler driver.  Currently, the StatusCodeHandler driver entry 
routines run some initialization code, register callbacks (eg. for 
ExitBootServices and SetVirtualAddressMap), and call the RscHandler 
PPI/Protocol to register the worker routines.  If I'm understanding the 
proposal correctly, all that code will be moved to the individual NULL 
libraries, since any particular library may or may not need any of it.  
Then the StatusCodeHandler modules will be left with no code of their 
own at all:  they will only be wrappers for the NULL libraries. Their 
entry routines will do nothing except return EFI_SUCCESS! (1)


It seems strange and wasteful to keep around empty modules like this.  
So I'm suggesting adding the NULL libraries to the StatusCodeRouter 
modules instead.  They would need to export the protocol/PPI routines to 
the NULL libraries via a header file, so they could call them directly 
instead of looking up the protocol/PPI.  But that's a minor change.  
Then you could remove the empty StatusCodeHandler modules entirely.  The 
advantage would be that there would be fewer modules in the build, 
simplifying the .dsc and .fdf files slightly.  It would also reduce code 
size a bit by sharing common library routines, such as BaseLib, with the 
StatusCodeRouter modules.


If those don't seem like worthwhile advantages, that's OK with me.  I 
don't want to belabor the point, or impede progress.  If others are OK 
with the proposal as it stands, then I am too.


Thanks,

*Brian J. Johnson
*Enterprise X86 Lab

Hewlett Packard Enterprise


(1) The StatusCodeHandlerRuntimeDxe driver also handles 
PcdStatusCodeReplayIn as part of its entry code.  That code would 
probably have to stay in a separate module rather than being linked to 
StatusCodeRouter as a NULL library.  That way it could be dispatched 
after the ReportStatusCode protocol is available.



*From:* Dong, Eric [mailto:eric.d...@intel.com]
*Sent:* Thursday, June 25, 2020, 10:41 AM
*To:* Brian J. Johnson , Bi, Dandan 
, Andrew Fish , 
edk2-devel-groups-io 
*Cc:* r...@edk2.groups.io , Ni, Ray 
, Wang, Jian J , Wu, Hao A 
, Tan, Ming , Laszlo Ersek 

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



Hi Brian,

In this new design, we still use register status code handler 
Protocol/Ppi to register the handler logic. We just want to change the 
StatusCodeHandler driver. We try to split the register logic to NULL 
library to make the code more modularity. We already created sample 
library in Edk2-Platforms repo 
https://github.com/tianocore/edk2-platforms/tree/master/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib. 
You can check this code to understand more about what we want to do.


Thanks,

Eric

*From:* Brian J. Johnson 
*Sent:* Thursday, June 25, 2020 4:25 AM
*To:* Bi, Dandan ; Andrew Fish ; 
edk2-devel-groups-io 
*Cc:* r...@edk2.groups.io; Dong, Eric ; Ni, Ray 
; Wang, Jian J ; Wu, Hao A 
; Tan, Ming ; Laszlo Ersek 

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


Dandan,

The Status Code Protocol/PPI is the high-level interface which is 
being implemented.  The ReportStatusCodeRouter module implements this 
in terms of the ReportStatusCodeHandler Protocol/PPI.  That allows 
multiple ReportStatusCodeHandler modules to be used at once, so they 
can each react to different types of status codes, or report them 
through multiple channels.  That sort of multiplexing seems like a 
useful feature.


Now we're considering adding a mechanism which allows registering 
status code handlers via NULL libraries, rather than via the 
protocol/PPI. That sounds like exactly what ReportStatusCodeRouter is 
intended for.  It wouldn't really change its scope, it would just make 
it more flexible.  Adding this feature via a StatusCodeHandler module 
wouldn't improve modularity, it would just add complexity.  As an OEM, 
adding a custom handler would look the same to me either way:  I would 
have to add the NULL class library to a MdeModulePkg driver's entry in 
my .dsc file.  It doesn't matter to me whether it's the 
ReportStatusCodeRouter or StatusCodeHandler module.  And if I can do 
it in ReportStatusCodeRouter, then I don't need to include any 
StatusCodeHandler modules in the build at all.  That saves code space 
and reduces the n

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

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

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

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

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

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

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

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

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

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

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

11 matches

Site Navigation

Mail list logo

Footer information