Hey Star,

Thank you very much for your reply.
Interesting, that is basically the case I described as "insane" because I did 
not consider any platform to allow S3 resume without memory initialization. So, 
this code definitely makes sense.
You are right, according to the specification, moving it to the PostMem FV 
should be fine. However that will cost a shadow call and a re-entry for non-S3 
and an event registration for the S3 boot path.
As the information whether S3 resume without meminit is intended is known at 
compile-time, what's your opinion on a FeatureFlag PCD which chooses between 
direct calls and the shadow/event system?
I would prepare a patch as soon as I can properly test its working, if you are 
interested. The changes would be most minimal, I imagine.

Thanks,
Marvin.

> -----Original Message-----
> From: Zeng, Star <[email protected]>
> Sent: Friday, July 13, 2018 11:24 AM
> To: Marvin H?user <[email protected]>; edk2-
> [email protected]
> Cc: Dong, Eric <[email protected]>; Cohen, Eugene <[email protected]>;
> Gao, Liming <[email protected]>; Zeng, Star <[email protected]>
> Subject: RE: Inquiry regarding early DxeIplPeim loading.
> 
> Marvin,
> 
> You can check SHA-1: ebaafbe62c70309d0ceb44a0c4199093d0a823c4.
> It is for the case "Allow S3 Resume without having installed permanent
> memory (via InstallPeiMemory)" (PI Mantis 1532, you can search the
> sentence in PI spec) requested by HP.
> Yes before ebaafbe62c70309d0ceb44a0c4199093d0a823c4, DxeIpl.inf had
> gEfiPeiMemoryDiscoveredPpiGuid DEPEX.
> For the case you mentioned about MinPlatformPkg, I think you can put the
> DxeIpl.inf into a Post Memory FV if the platform will publish
> gEfiPeiMemoryDiscoveredPpiGuid indeed.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:[email protected]] On Behalf Of
> Marvin H?user
> Sent: Friday, July 13, 2018 7:19 AM
> To: [email protected]
> Cc: Dong, Eric <[email protected]>; Zeng, Star <[email protected]>
> Subject: [edk2] Inquiry regarding early DxeIplPeim loading.
> 
> Good day developers,
> 
> While checking out which edk2 modules request being shadowed, I came
> across DxeIplPeim being one of them, however I am not sure why it was
> designed this way.
> 
> If the Boot Mode is != S3, the module will register for shadowing and
> immediately return during the pre-memory phase
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L92
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L111
> 
> If the Boot Mode is S3, the module will register a Memory Discovered event
> to install crucial PPIs...
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L125
> ... and install the DxeIpl PPI before returning
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L132
> 
> However, by design, the DxeIpl PPI is not located until the very end of
> PeiCore, meaning the dispatcher ran out of modules to dispatch
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> PeiMain/PeiMain.c#L467
> Hence installing the DxeIpl PPI early in the S3 boot path does not seem to
> have any effect to me, as both paths are left awaiting memory availability
> (Shadow / event). The only functional change would be PeiCore failing to
> locate the DxeIpl PPI in case memory initialization silently fails and code
> execution continues, which is an insane state in the first place.
> 
> Am I missing any scenario where this design is helpful? Is there any
> disadvantage for adding a Depex on MemoryDiscovered PPI? Running only
> after memory initialization would shrink the initialization function by
> removing the shadowing request in non-S3 path and the event registration in
> the S3 path, as well as merging the PPI installation code as both 
> registrations
> end up executing the exact same code
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L118
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe
> IplPeim/DxeLoad.c#L57
> 
> The initialization function would collapse to PPI installations, a shadow or
> event registration call would be saved and platforms could safely embed
> DxeIplPeim into a Post Memory FV, such as MinPlatformPkg is using, to have
> the PEIM loaded directly into memory to gain yet more performance. The
> only restriction would be to prohibit compression.
> 
> Thanks for your time.
> 
> Best regards,
> Marvin.
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to