Dear developers,

Dear KabylakeSiliconPkg devs: If you are not interested in the suggestion of 
generic, shared modules, please read my comment on 2) nevertheless, as it 
contains a potential bug report. Sorry, but I could not verify this yet.

I have been exploring the code of most of the Intel 'open platforms' lately and 
noticed they shared a bunch of slightly different modules, which are diverse to 
fit the platform's requirements.
One of them is PchSmmDispatcher/QNCSmmDispatcher. I'm not sure about non-Intel 
platforms, though in my opinion, there should be a generic module in 
MdeModulePkg, if it can be shared across brands, or one in In IntelSiliconPkg, 
if it's useful for Intel platforms only. To see why I think a generic, shared 
module that consumes a platform library would be hugely beneficial, please take 
a look at these lines:


  1.  
https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Silicon/Intel/KabylakeSiliconPkg/Pch/PchSmiDispatcher/Smm/PchSmmCore.c#L361
  2.  
https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Silicon/Intel/KabylakeSiliconPkg/Pch/PchSmiDispatcher/Smm/PchSmmCore.c#L890
  3.  
https://github.com/tianocore/edk2/blob/master/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c#L753

1) shows a hook to SmmReadyToLock, which I first and only saw in 
KabylakeSiliconPkg. This addition has not been brought to the Quark platform, 
for example. With a shared module, all platforms that embed it would have 
profited.

2) on the other hand shows KabylakeSiliconPkg's PchSmmDispatcher accessing 
RecordInDb after the callback function has been executed, which QuarkSocPkg's 
QNCSmmDispatcher explicitely forbids in 3), as it might have been freed by the 
call. I am not sure whether it was a bug in the Quark firmware that has been 
fixed by the time the Kabylake code was written, though I don't believe so, 
because the changes to the loop to process multiple SMI sources in one go is 
not present in the Kabylake code either. If there was a shared module, the fix 
to Quark would have profited all other platforms.

It's obvious to me that KabylakeSiliconPkg has a bunch of things that were 
previously only distributed privately as part of the private Reference Code. I 
think that keeping the RC as small as possible (the very best case would be 
only platform library classes consumed by generic, open source modules) is a 
goal worth persuing, as more code can be shared and community-verified as part 
of EDK2.

Thanks for your time!

Regards,
Marvin.
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to