public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Inquiry regarding early DxeIplPeim loading.
@ 2018-07-12 23:19 Marvin H?user
  2018-07-13  9:24 ` Zeng, Star
  0 siblings, 1 reply; 7+ messages in thread
From: Marvin H?user @ 2018-07-12 23:19 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: star.zeng@intel.com, eric.dong@intel.com

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/DxeIplPeim/DxeLoad.c#L92
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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/DxeIplPeim/DxeLoad.c#L125
... and install the DxeIpl PPI before returning
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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/DxeIplPeim/DxeLoad.c#L118
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-12 23:19 Inquiry regarding early DxeIplPeim loading Marvin H?user
@ 2018-07-13  9:24 ` Zeng, Star
  2018-07-13 13:25   ` Marvin H?user
  0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2018-07-13  9:24 UTC (permalink / raw)
  To: Marvin H?user, edk2-devel@lists.01.org
  Cc: Dong, Eric, Cohen, Eugene, Gao, Liming, Zeng, Star

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:edk2-devel-bounces@lists.01.org] On Behalf Of Marvin H?user
Sent: Friday, July 13, 2018 7:19 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
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/DxeIplPeim/DxeLoad.c#L92
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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/DxeIplPeim/DxeLoad.c#L125
... and install the DxeIpl PPI before returning
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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/DxeIplPeim/DxeLoad.c#L118
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-13  9:24 ` Zeng, Star
@ 2018-07-13 13:25   ` Marvin H?user
  2018-07-18  9:11     ` Zeng, Star
  2018-07-18 21:36     ` Cohen, Eugene
  0 siblings, 2 replies; 7+ messages in thread
From: Marvin H?user @ 2018-07-13 13:25 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: Zeng, Star, eric.dong@intel.com

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 <star.zeng@intel.com>
> Sent: Friday, July 13, 2018 11:24 AM
> To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Cohen, Eugene <eugene@hp.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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:edk2-devel-bounces@lists.01.org] On Behalf Of
> Marvin H?user
> Sent: Friday, July 13, 2018 7:19 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-13 13:25   ` Marvin H?user
@ 2018-07-18  9:11     ` Zeng, Star
  2018-07-18 21:36     ` Cohen, Eugene
  1 sibling, 0 replies; 7+ messages in thread
From: Zeng, Star @ 2018-07-18  9:11 UTC (permalink / raw)
  To: Marvin H?user, edk2-devel@lists.01.org; +Cc: Dong, Eric, Zeng, Star

Hi Marvin,

Thanks for the idea.
Many configuration could be known at compile time for one specific platform, but we could not invent FeatureFlag PCD for all of them.
I do not think it costs much and I do not prefer a new FeatureFlag PCD for this case.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Marvin H?user
Sent: Friday, July 13, 2018 9:26 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] Inquiry regarding early DxeIplPeim loading.

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 <star.zeng@intel.com>
> Sent: Friday, July 13, 2018 11:24 AM
> To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2- 
> devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Cohen, Eugene <eugene@hp.com>; 
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Marvin H?user
> Sent: Friday, July 13, 2018 7:19 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-13 13:25   ` Marvin H?user
  2018-07-18  9:11     ` Zeng, Star
@ 2018-07-18 21:36     ` Cohen, Eugene
  2018-07-19  0:10       ` Marvin H?user
  1 sibling, 1 reply; 7+ messages in thread
From: Cohen, Eugene @ 2018-07-18 21:36 UTC (permalink / raw)
  To: Marvin H?user, edk2-devel@lists.01.org
  Cc: eric.dong@intel.com, Zeng, Star, Cohen, Eugene

The depex change did not pertain to S3 resume without memory initialization (obviously that wouldn't work).  The change was whether "permanent memory" was installed from a PI architecture perspective.  We had a situation that required that the S3 Resume path remain in PEI Cache-as-RAM (actual code in CAR, not just data) and this change was necessary to prevent CAR from being turned off prematurely on S3 resume.

By the way it was never our goal for DXE IPL to host the S3 resume path - my understanding is that this was a matter of convenience back in ancient history of S3 development (according to an old thread I dug up from a few years ago).  If S3 resume were parked somewhere else such that DXE IPL only did the IPL of DXE then this issue might go away.  So you're seeing a secondary effect of hosting S3 resume in DXE IPL and DXE IPL depex changes to support an unusual use case.


> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> Marvin H?user
> Sent: Friday, July 13, 2018 7:26 AM
> To: edk2-devel@lists.01.org
> Cc: eric.dong@intel.com; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] Inquiry regarding early DxeIplPeim loading.
> 
> 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 <star.zeng@intel.com>
> > Sent: Friday, July 13, 2018 11:24 AM
> > To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-
> > devel@lists.01.org
> > Cc: Dong, Eric <eric.dong@intel.com>; Cohen, Eugene
> <eugene@hp.com>;
> > Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > 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:edk2-devel-bounces@lists.01.org] On Behalf
> Of
> > Marvin H?user
> > Sent: Friday, July 13, 2018 7:19 AM
> > To: edk2-devel@lists.01.org
> > Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > 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/D
> xe
> > IplPeim/DxeLoad.c#L92
> >
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> xe
> > 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/D
> xe
> > IplPeim/DxeLoad.c#L125
> > ... and install the DxeIpl PPI before returning
> >
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> xe
> > 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/P
> ei/
> > 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/D
> xe
> > IplPeim/DxeLoad.c#L118
> >
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> xe
> > 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
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-18 21:36     ` Cohen, Eugene
@ 2018-07-19  0:10       ` Marvin H?user
  2018-07-19 15:40         ` Cohen, Eugene
  0 siblings, 1 reply; 7+ messages in thread
From: Marvin H?user @ 2018-07-19  0:10 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: Cohen, Eugene, star.zeng@intel.com

Hey Eugene,

I do not think this is a hack, I don't think it uses DxeIplPeim for "actual booting", but more for the compression PPIs it exposes.
FVs might be compressed for performance reason and be loaded into memory (DRAM or cache).
Though to be honest, I am not sure why these PPIs are exposed by DxeIplPeim instead of a dedicated module or maybe even PeiCore itself, if it's the de facto standard.

Also, if the change was done to avoid disabling CAR prematurely, why wasn't SEC code adapted?
UefiCpuPkg SecCore calls the PlatformSecLib function to disable CAR when permanent memory has been installed.
>From what I can see, the described change only makes sense if InstallMemory is not called during the entire PEI phase. Am I missing something?

Thanks,
Marvin

> -----Original Message-----
> From: Cohen, Eugene <eugene@hp.com>
> Sent: Wednesday, July 18, 2018 11:37 PM
> To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: eric.dong@intel.com; Zeng, Star <star.zeng@intel.com>; Cohen, Eugene
> <eugene@hp.com>
> Subject: RE: Inquiry regarding early DxeIplPeim loading.
> 
> The depex change did not pertain to S3 resume without memory initialization
> (obviously that wouldn't work).  The change was whether "permanent
> memory" was installed from a PI architecture perspective.  We had a
> situation that required that the S3 Resume path remain in PEI Cache-as-RAM
> (actual code in CAR, not just data) and this change was necessary to prevent
> CAR from being turned off prematurely on S3 resume.
> 
> By the way it was never our goal for DXE IPL to host the S3 resume path - my
> understanding is that this was a matter of convenience back in ancient
> history of S3 development (according to an old thread I dug up from a few
> years ago).  If S3 resume were parked somewhere else such that DXE IPL
> only did the IPL of DXE then this issue might go away.  So you're seeing a
> secondary effect of hosting S3 resume in DXE IPL and DXE IPL depex changes
> to support an unusual use case.
> 
> 
> > -----Original Message-----
> > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
> > H?user
> > Sent: Friday, July 13, 2018 7:26 AM
> > To: edk2-devel@lists.01.org
> > Cc: eric.dong@intel.com; Zeng, Star <star.zeng@intel.com>
> > Subject: Re: [edk2] Inquiry regarding early DxeIplPeim loading.
> >
> > 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 <star.zeng@intel.com>
> > > Sent: Friday, July 13, 2018 11:24 AM
> > > To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-
> > > devel@lists.01.org
> > > Cc: Dong, Eric <eric.dong@intel.com>; Cohen, Eugene
> > <eugene@hp.com>;
> > > Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > 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:edk2-devel-bounces@lists.01.org] On Behalf
> > Of
> > > Marvin H?user
> > > Sent: Friday, July 13, 2018 7:19 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star
> > > <star.zeng@intel.com>
> > > 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/D
> > xe
> > > IplPeim/DxeLoad.c#L92
> > >
> > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> > xe
> > > 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/D
> > xe
> > > IplPeim/DxeLoad.c#L125
> > > ... and install the DxeIpl PPI before returning
> > >
> > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> > xe
> > > 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/P
> > ei/
> > > 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/D
> > xe
> > > IplPeim/DxeLoad.c#L118
> > >
> > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/D
> > xe
> > > 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
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Inquiry regarding early DxeIplPeim loading.
  2018-07-19  0:10       ` Marvin H?user
@ 2018-07-19 15:40         ` Cohen, Eugene
  0 siblings, 0 replies; 7+ messages in thread
From: Cohen, Eugene @ 2018-07-19 15:40 UTC (permalink / raw)
  To: Marvin H?user, edk2-devel@lists.01.org; +Cc: star.zeng@intel.com

Marvin,

> Though to be honest, I am not sure why these PPIs are exposed by
> DxeIplPeim instead of a dedicated module or maybe even PeiCore itself, if
> it's the de facto standard.

When we were originally facing the issue one of the ideas was in fact to move this to PeiCore.  I don't recall why that path wasn't pursued.

> Also, if the change was done to avoid disabling CAR prematurely, why
> wasn't SEC code adapted?

This was an ARM SoC not X86 so the modules involved were different than what you're seeing.  I'm trying to remember the specifics but it was long enough ago that it escapes me and my time machine is broken.

Eugene


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-19 15:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-12 23:19 Inquiry regarding early DxeIplPeim loading Marvin H?user
2018-07-13  9:24 ` Zeng, Star
2018-07-13 13:25   ` Marvin H?user
2018-07-18  9:11     ` Zeng, Star
2018-07-18 21:36     ` Cohen, Eugene
2018-07-19  0:10       ` Marvin H?user
2018-07-19 15:40         ` Cohen, Eugene

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox