[edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime cache

[Kubacki, Michael A]

You have a good point with the proposed order for enabling the PCD. I will make that
update in a V4 later today.

Thanks,
Michael

> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Monday, October 14, 2019 11:50 AM
> To: devel at edk2.groups.io; Kubacki, Michael A
> <michael.a.kubacki at intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Gao, Liming
> <liming.gao at intel.com>; Justen, Jordan L <jordan.l.justen at intel.com>; Wu,
> Hao A <hao.a.wu at intel.com>
> Subject: Re: [edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime
> cache
> 
> On 10/14/19 20:08, Kubacki, Michael A wrote:
> > Sets gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> > to FALSE in OvmfPkgIa32.dsc, OvmfPkgIa32X64.dsc, and OvmfPkgX64.dsc
> so
> > that when SMM_REQUIRE is TRUE, the SMM variable driver will not use
> > the runtime variable cache.
> >
> > This is done for OvmfPkg because it currently depends upon a SMM
> > variable GetVariable ()implementation as a simple method to exercise
> > the SMM driver stack. This allows the following commands to be used
> > for variables such as Boot####, BootOrder, and BootNext to test SMM
> > timing and stability differences on the BSP (e.g. CPU#0) vs an AP
> > (e.g. CPU#1).
> >  # taskset -c 0 efibootmgr
> >  # taskset -c 1 efibootmgr
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: Liming Gao <liming.gao at intel.com>
> > Cc: Jordan Justen <jordan.l.justen at intel.com>
> > Cc: Hao A Wu <hao.a.wu at intel.com>
> > Signed-off-by: Michael Kubacki <michael.a.kubacki at intel.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 1 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> >  OvmfPkg/OvmfPkgX64.dsc     | 1 +
> >  3 files changed, 3 insertions(+)
> >
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index
> > 66e944436a..4861e2342b 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -428,6 +428,7 @@
> >  !if $(SMM_REQUIRE) == TRUE
> >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALS
> E
> >  !endif
> >
> >  [PcdsFixedAtBuild]
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc
> b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 51c2bfb44f..f316d97d6a 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -433,6 +433,7 @@
> >  !if $(SMM_REQUIRE) == TRUE
> >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALS
> E
> >  !endif
> >
> >  [PcdsFixedAtBuild]
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index
> > ba7a758844..546649f0a2 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -433,6 +433,7 @@
> >  !if $(SMM_REQUIRE) == TRUE
> >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALS
> E
> >  !endif
> >
> >  [PcdsFixedAtBuild]
> >
> 
> For this patch:
> 
> Reviewed-by: Laszlo Ersek <lersek at redhat.com>
> 
> And many thanks for accommodating this use case!
> 
> However, I'd prefer the following order of changes:
> 
> - patch 7: introduce the PCD with default FALSE value
> - patch 9: set the PCD explicitly to FALSE in OvmfPkg
> - patch 10 (new patch): flip the PCD default value to TRUE
> 
> Because with v3, if someone were to bisect an issue, using OVMF, and the
> bisection landed them at patch 7 or at patch 8, they'd see a
> (transitory) change in variable caching behavior.
> 
> Thanks,
> Laszlo
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48936): https://edk2.groups.io/g/devel/message/48936
Mute This Topic: https://groups.io/mt/34536814/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-