Liming,
Using HII PCDs introduces too many software layers that may cause potential
bugs if any layer is not proper implemented (Imaging a case when a platform
forgets to include the mdeModule.dsc.inc).
Variable used here is very straightforward.
That was also the reason when I redesigned the BDS 10 years ago, I chose not to
include the PcdBootState into the MdeModulePkg but left that in platform.
I agree with you that consistency is better. And I suggest we submit a Bugzilla
to change all open platforms to use variable directly for first boot detection.
Thanks,
Ray
> -----Original Message-----
> From: gaoliming <gaolim...@byosoft.com.cn>
> Sent: Wednesday, March 17, 2021 11:01 AM
> To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang....@intel.com>; Ni, Ray
> <ray...@intel.com>
> Cc: Dong, Eric <eric.d...@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@intel.com>; Agyeman, Prince
> <prince.agye...@intel.com>; Gao, Zhichao <zhichao....@intel.com>
> Subject: 回复: [edk2-devel] 回复: [Patch edk2-platforms V2] Intel/BoardModulePkg:
> sort load option in the first boot
>
> Zhiguang:
> This is the common platform usage. I suggest to apply the same solution. My
> solution is to define this PCD PcdBootState in
> MdeModulePkg.dec, and add MdeModule.dsc.inc file that defines this PCD as
> DynamicHii PCD, platform DSC includes
> MdeModule.dsc.inc file, platform modules consume this PCD (set/get).
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Zhiguang Liu
> > 发送时间: 2021年3月16日 9:57
> > 收件人: devel@edk2.groups.io; gaolim...@byosoft.com.cn; Ni, Ray
> > <ray...@intel.com>
> > 抄送: Dong, Eric <eric.d...@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desim...@intel.com>; Agyeman, Prince
> > <prince.agye...@intel.com>; Gao, Zhichao <zhichao....@intel.com>
> > 主题: Re: [edk2-devel] 回复: [Patch edk2-platforms V2]
> > Intel/BoardModulePkg: sort load option in the first boot
> >
> > Hi Liming,
> >
> > Thanks for the comments. This patch is merged before this comment, but I can
> > still send another patch to modify if needed.
> >
> > However, I think the implement in this patch is more simple.
> > The implement in QuarkPlatformPkg need changes in inf, dec and dsc files,
> > and is not as intuitive as just getting and setting a variable.
> > It may be simpler if the implements can reuse a same DynamicHiiPcd, do you
> > think it is possible?
> > If I misunderstand anything, please correct me.
> >
> > Thanks
> > Zhiguang
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > gaoliming
> > > Sent: Monday, March 15, 2021 9:36 AM
> > > To: Ni, Ray <ray...@intel.com>; Liu, Zhiguang <zhiguang....@intel.com>;
> > > devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.d...@intel.com>; Desimone, Nathaniel L
> > > <nathaniel.l.desim...@intel.com>; Agyeman, Prince
> > > <prince.agye...@intel.com>; Gao, Zhichao <zhichao....@intel.com>
> > > Subject: [edk2-devel] 回复: [Patch edk2-platforms V2]
> > > Intel/BoardModulePkg: sort load option in the first boot
> > >
> > > Zhiguang:
> > > I see QuarkPlatformPkg uses PCD
> > > gQuarkPlatformTokenSpaceGuid.PcdBootState
> > > to decide whether current boot is the first boot or not.
> > > This PCD is configured as DynamicHiiPcd, and be set in
> > > Platform\Intel\QuarkPlatformPkg\Library\PlatformBootManagerLib\Platfor
> > > mBootM
> > > anager.c
> > >
> > > Can you use the same solution in Intel BoardModulePkg?
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: Ni, Ray <ray...@intel.com>
> > > > 发送时间: 2021年3月10日 17:56
> > > > 收件人: Liu, Zhiguang <zhiguang....@intel.com>; devel@edk2.groups.io
> > > > 抄送: Dong, Eric <eric.d...@intel.com>; Liming Gao
> > > > <gaolim...@byosoft.com.cn>; Desimone, Nathaniel L
> > > > <nathaniel.l.desim...@intel.com>; Agyeman, Prince
> > > > <prince.agye...@intel.com>; Gao, Zhichao <zhichao....@intel.com>
> > > > 主题: RE: [Patch edk2-platforms V2] Intel/BoardModulePkg: sort load
> > > > option in the first boot
> > > >
> > > > 1. DataSIze should be set to sizeof (BOOLEAN) before calling
> > > > GetVariable()
> > > >
> > > > > + Status = gRT->GetVariable (
> > > > > + L"IsFirstBoot",
> > > >
> > > > 2. Can you please define a macro in this C file for IsFirstBoot string?
> > > > e.g.: #define IS_FIRST_BOOT_VAR_NAME L"IsFirstBoot"
> > > >
> > > > > + if (IsFirstBoot == TRUE) {
> > > >
> > > > 3. Please remove "== TRUE". Just use "If (IsFirstBoot)".
> > > >
> > > > > + L"IsFirstBoot",
> > > > 4. Please use the macro defined as above.
> > > >
> > > > >
> > > > > + &gEfiCallerIdGuid,
> > > > >
> > > > > + EFI_VARIABLE_NON_VOLATILE |
> > > > > EFI_VARIABLE_RUNTIME_ACCESS |
> > > EFI_VARIABLE_BOOTSERVICE_ACCESS,
> > > >
> > > > 5. Please remove "EFI_VARIABLE_RUNTIME_ACCESS".
> > > >
> > > > > + 1,
> > > > 6. Please use sizeof (BOOLEAN) instead of "1".
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72950): https://edk2.groups.io/g/devel/message/72950
Mute This Topic: https://groups.io/mt/81395975/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
