Re: [edk2] MinPlatformPkg/PlatformInit: FV code

2018-02-02 Thread 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-boun...@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Saturday, February 3, 2018 9:54 AM
> To: Marvin H?user <marvin.haeu...@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-boun...@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@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@intel.com]
> > Sent: Friday, February 2, 2018 1:40 PM
> > To: Marvin H?user <marvin.haeu...@outlook.com>; edk2-devel@lists.01.org
> > Subject: RE: MinPlatformPkg/PlatformInit: FV code
> >
> > Excellent question.
> >
> > Comment inline.
> >
> > From: Marvin H?user [mailto:marvin.haeu...@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@intel.com<mailto:jiewen@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
> > >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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] MinPlatformPkg/PlatformInit: FV code

2018-02-02 Thread 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-boun...@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@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@intel.com]
> Sent: Friday, February 2, 2018 1:40 PM
> To: Marvin H?user <marvin.haeu...@outlook.com>; edk2-devel@lists.01.org
> Subject: RE: MinPlatformPkg/PlatformInit: FV code
> 
> Excellent question.
> 
> Comment inline.
> 
> From: Marvin H?user [mailto:marvin.haeu...@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@intel.com<mailto:jiewen@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
> >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


Re: [edk2] MinPlatformPkg/PlatformInit: FV code

2018-02-02 Thread 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@intel.com]
Sent: Friday, February 2, 2018 1:40 PM
To: Marvin H?user ; edk2-devel@lists.01.org
Subject: RE: MinPlatformPkg/PlatformInit: FV code

Excellent question.

Comment inline.

From: Marvin H?user [mailto:marvin.haeu...@outlook.com]
Sent: Wednesday, January 31, 2018 1:54 AM
To: edk2-devel@lists.01.org; Yao, Jiewen 
>
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 
>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: [edk2] MinPlatformPkg/PlatformInit: FV code

2018-02-02 Thread Yao, Jiewen
Excellent question.

Comment inline.

From: Marvin H?user [mailto:marvin.haeu...@outlook.com]
Sent: Wednesday, January 31, 2018 1:54 AM
To: edk2-devel@lists.01.org; Yao, Jiewen 
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 
>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