MinPlatformPkg/PlatformInit: FV code

MinPlatformPkg/PlatformInit: FV code

[Marvin H?user]

Dear developers, dear Jiewen,

I have been investigating the devel-MinPlatform branch of edk2-platforms for educational purposes and got two questions regarding the Firmware Volume code in PlatformInitPreMem, if you do not mind. I assume the tree was tested, so most likely I misunderstood some things.


  1.  Why is a Firmware Volume HOB built to cover the entire flash range (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L379)? Am I correct that this implies a FV spanning through the entire flash MMIO range, which would then imply all other FVs are contained within it? This would make sense, however that's not what I saw in the KabylakeOpenBoardPkg Flash Map, which has the NV Storage first (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/KabylakeOpenBoardPkg/Include/Fdf/FlashMapInclude.fdf#L25).
  2.  Why are FV Info PPIs installed for the UefiBoot and the OsBoot FVs (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L344)? If I checked correctly, installing this PPI type will trigger PeiCore to dispatch PEIMs in the FVs, however there are only DXE drivers in these. Why are no FV HOBs installed, which are gotten by DxeCore?

Thanks in advance for your time!

Best regards,
Marvin.

Re: MinPlatformPkg/PlatformInit: FV code

[Yao, Jiewen]

Excellent question.

Comment inline.

From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
Sent: Wednesday, January 31, 2018 1:54 AM
To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
Subject: MinPlatformPkg/PlatformInit: FV code

Dear developers, dear Jiewen,

I have been investigating the devel-MinPlatform branch of edk2-platforms for educational purposes and got two questions regarding the Firmware Volume code in PlatformInitPreMem, if you do not mind. I assume the tree was tested, so most likely I misunderstood some things.


  1.  Why is a Firmware Volume HOB built to cover the entire flash range (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L379)? Am I correct that this implies a FV spanning through the entire flash MMIO range, which would then imply all other FVs are contained within it? This would make sense, however that's not what I saw in the KabylakeOpenBoardPkg Flash Map, which has the NV Storage first (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/KabylakeOpenBoardPkg/Include/Fdf/FlashMapInclude.fdf#L25).

[Jiewen] You are right. We should not use FD region for FV. Will fix it.



  1.  Why are FV Info PPIs installed for the UefiBoot and the OsBoot FVs (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L344)? If I checked correctly, installing this PPI type will trigger PeiCore to dispatch PEIMs in the FVs, however there are only DXE drivers in these. Why are no FV HOBs installed, which are gotten by DxeCore?

[Jiewen] In DxeIpl, PeiServicesFfsFindNextVolume() is used to search DxeCore.
In PeiCore, PeiFfsFindNextVolume() calls FindNextCoreFvHandle() for DxeCore one by one. If PcdFrameworkCompatibilitySupport is FALSE, it returns &Private->Fv[Instance] directly.

And Fv[Instance] is added in FirmwareVolmeInfoPpiNotifyCallback(), when gEfiPeiFirmwareVolumeInfo2PpiGuid is installed.

So if PcdFrameworkCompatibilitySupport is FALSE, install PPI is the only way to let PEI core discover DxeCore.
Only if PcdFrameworkCompatibilitySupport is TRUE, install PPI is not required, but the FindNextCoreFvHandle() will install the PPI for the HobFv. The result is same.


Thank you
Yao Jiewen




Thanks in advance for your time!

Best regards,
Marvin.

Re: MinPlatformPkg/PlatformInit: FV code

[Marvin H?user]

Good point with the DxeCore, I didn't consider that. Though OsBoot would be irrelevant to the PEI phase, wouldn't it be?

Thanks,
Marvin

From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Friday, February 2, 2018 1:40 PM
To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org
Subject: RE: MinPlatformPkg/PlatformInit: FV code

Excellent question.

Comment inline.

From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
Sent: Wednesday, January 31, 2018 1:54 AM
To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Subject: MinPlatformPkg/PlatformInit: FV code

Dear developers, dear Jiewen,

I have been investigating the devel-MinPlatform branch of edk2-platforms for educational purposes and got two questions regarding the Firmware Volume code in PlatformInitPreMem, if you do not mind. I assume the tree was tested, so most likely I misunderstood some things.


  1.  Why is a Firmware Volume HOB built to cover the entire flash range (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L379)? Am I correct that this implies a FV spanning through the entire flash MMIO range, which would then imply all other FVs are contained within it? This would make sense, however that's not what I saw in the KabylakeOpenBoardPkg Flash Map, which has the NV Storage first (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/KabylakeOpenBoardPkg/Include/Fdf/FlashMapInclude.fdf#L25).

[Jiewen] You are right. We should not use FD region for FV. Will fix it.



  1.  Why are FV Info PPIs installed for the UefiBoot and the OsBoot FVs (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L344)? If I checked correctly, installing this PPI type will trigger PeiCore to dispatch PEIMs in the FVs, however there are only DXE drivers in these. Why are no FV HOBs installed, which are gotten by DxeCore?

[Jiewen] In DxeIpl, PeiServicesFfsFindNextVolume() is used to search DxeCore.
In PeiCore, PeiFfsFindNextVolume() calls FindNextCoreFvHandle() for DxeCore one by one. If PcdFrameworkCompatibilitySupport is FALSE, it returns &Private->Fv[Instance] directly.

And Fv[Instance] is added in FirmwareVolmeInfoPpiNotifyCallback(), when gEfiPeiFirmwareVolumeInfo2PpiGuid is installed.

So if PcdFrameworkCompatibilitySupport is FALSE, install PPI is the only way to let PEI core discover DxeCore.
Only if PcdFrameworkCompatibilitySupport is TRUE, install PPI is not required, but the FindNextCoreFvHandle() will install the PPI for the HobFv. The result is same.


Thank you
Yao Jiewen




Thanks in advance for your time!

Best regards,
Marvin.

Re: MinPlatformPkg/PlatformInit: FV code

[Yao, Jiewen]

Ah, good catch.

That is correct - it is irrelevant to PEI. To put to FV Hob is enough, I believe.

Appreciate your careful review, which helps us clean up the code. :-)

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Marvin H?user
> Sent: Saturday, February 3, 2018 2:30 AM
> To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] MinPlatformPkg/PlatformInit: FV code
> 
> Good point with the DxeCore, I didn't consider that. Though OsBoot would be
> irrelevant to the PEI phase, wouldn't it be?
> 
> Thanks,
> Marvin
> 
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Friday, February 2, 2018 1:40 PM
> To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org
> Subject: RE: MinPlatformPkg/PlatformInit: FV code
> 
> Excellent question.
> 
> Comment inline.
> 
> From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
> Sent: Wednesday, January 31, 2018 1:54 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Yao, Jiewen
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: MinPlatformPkg/PlatformInit: FV code
> 
> Dear developers, dear Jiewen,
> 
> I have been investigating the devel-MinPlatform branch of edk2-platforms for
> educational purposes and got two questions regarding the Firmware Volume
> code in PlatformInitPreMem, if you do not mind. I assume the tree was tested, so
> most likely I misunderstood some things.
> 
> 
>   1.  Why is a Firmware Volume HOB built to cover the entire flash range
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> m/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L3
> 79)? Am I correct that this implies a FV spanning through the entire flash MMIO
> range, which would then imply all other FVs are contained within it? This would
> make sense, however that's not what I saw in the KabylakeOpenBoardPkg Flash
> Map, which has the NV Storage first
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> m/Intel/KabylakeOpenBoardPkg/Include/Fdf/FlashMapInclude.fdf#L25).
> 
> [Jiewen] You are right. We should not use FD region for FV. Will fix it.
> 
> 
> 
>   1.  Why are FV Info PPIs installed for the UefiBoot and the OsBoot FVs
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> m/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L3
> 44)? If I checked correctly, installing this PPI type will trigger PeiCore to dispatch
> PEIMs in the FVs, however there are only DXE drivers in these. Why are no FV
> HOBs installed, which are gotten by DxeCore?
> 
> [Jiewen] In DxeIpl, PeiServicesFfsFindNextVolume() is used to search DxeCore.
> In PeiCore, PeiFfsFindNextVolume() calls FindNextCoreFvHandle() for DxeCore
> one by one. If PcdFrameworkCompatibilitySupport is FALSE, it returns
> &Private->Fv[Instance] directly.
> 
> And Fv[Instance] is added in FirmwareVolmeInfoPpiNotifyCallback(), when
> gEfiPeiFirmwareVolumeInfo2PpiGuid is installed.
> 
> So if PcdFrameworkCompatibilitySupport is FALSE, install PPI is the only way to
> let PEI core discover DxeCore.
> Only if PcdFrameworkCompatibilitySupport is TRUE, install PPI is not required,
> but the FindNextCoreFvHandle() will install the PPI for the HobFv. The result is
> same.
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> 
> 
> Thanks in advance for your time!
> 
> Best regards,
> Marvin.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: MinPlatformPkg/PlatformInit: FV code

[Yao, Jiewen]

Marvin
I have filed 2 bugzilla to record this.

https://bugzilla.tianocore.org/show_bug.cgi?id=872
https://bugzilla.tianocore.org/show_bug.cgi?id=871

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Saturday, February 3, 2018 9:54 AM
> To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] MinPlatformPkg/PlatformInit: FV code
> 
> Ah, good catch.
> 
> That is correct - it is irrelevant to PEI. To put to FV Hob is enough, I believe.
> 
> Appreciate your careful review, which helps us clean up the code. :-)
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Marvin H?user
> > Sent: Saturday, February 3, 2018 2:30 AM
> > To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2] MinPlatformPkg/PlatformInit: FV code
> >
> > Good point with the DxeCore, I didn't consider that. Though OsBoot would be
> > irrelevant to the PEI phase, wouldn't it be?
> >
> > Thanks,
> > Marvin
> >
> > From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> > Sent: Friday, February 2, 2018 1:40 PM
> > To: Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org
> > Subject: RE: MinPlatformPkg/PlatformInit: FV code
> >
> > Excellent question.
> >
> > Comment inline.
> >
> > From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
> > Sent: Wednesday, January 31, 2018 1:54 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Yao, Jiewen
> > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> > Subject: MinPlatformPkg/PlatformInit: FV code
> >
> > Dear developers, dear Jiewen,
> >
> > I have been investigating the devel-MinPlatform branch of edk2-platforms for
> > educational purposes and got two questions regarding the Firmware Volume
> > code in PlatformInitPreMem, if you do not mind. I assume the tree was tested,
> so
> > most likely I misunderstood some things.
> >
> >
> >   1.  Why is a Firmware Volume HOB built to cover the entire flash range
> >
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> >
> m/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L3
> > 79)? Am I correct that this implies a FV spanning through the entire flash
> MMIO
> > range, which would then imply all other FVs are contained within it? This would
> > make sense, however that's not what I saw in the KabylakeOpenBoardPkg
> Flash
> > Map, which has the NV Storage first
> >
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> > m/Intel/KabylakeOpenBoardPkg/Include/Fdf/FlashMapInclude.fdf#L25).
> >
> > [Jiewen] You are right. We should not use FD region for FV. Will fix it.
> >
> >
> >
> >   1.  Why are FV Info PPIs installed for the UefiBoot and the OsBoot FVs
> >
> (https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platfor
> >
> m/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L3
> > 44)? If I checked correctly, installing this PPI type will trigger PeiCore to
> dispatch
> > PEIMs in the FVs, however there are only DXE drivers in these. Why are no FV
> > HOBs installed, which are gotten by DxeCore?
> >
> > [Jiewen] In DxeIpl, PeiServicesFfsFindNextVolume() is used to search DxeCore.
> > In PeiCore, PeiFfsFindNextVolume() calls FindNextCoreFvHandle() for DxeCore
> > one by one. If PcdFrameworkCompatibilitySupport is FALSE, it returns
> > &Private->Fv[Instance] directly.
> >
> > And Fv[Instance] is added in FirmwareVolmeInfoPpiNotifyCallback(), when
> > gEfiPeiFirmwareVolumeInfo2PpiGuid is installed.
> >
> > So if PcdFrameworkCompatibilitySupport is FALSE, install PPI is the only way to
> > let PEI core discover DxeCore.
> > Only if PcdFrameworkCompatibilitySupport is TRUE, install PPI is not required,
> > but the FindNextCoreFvHandle() will install the PPI for the HobFv. The result is
> > same.
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >
> >
> > Thanks in advance 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