1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH V3 0/9] UEFI Variable SMI Reduction
  4. View patch

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

Kubacki, Michael A posted 9 patches 6 years, 3 months ago
There is a newer version of this series
[edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime cache
Posted by Kubacki, Michael A 6 years, 3 months ago 
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@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@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|FALSE
 !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|FALSE
 !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|FALSE
 !endif
 
 [PcdsFixedAtBuild]
-- 
2.16.2.windows.1


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

View/Reply Online (#48926): https://edk2.groups.io/g/devel/message/48926
Mute This Topic: https://groups.io/mt/34536814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime cache
Posted by Laszlo Ersek 6 years, 3 months ago 
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@linaro.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@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|FALSE
>  !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|FALSE
>  !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|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
> 

For this patch:

Reviewed-by: Laszlo Ersek <lersek@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 (#48934): https://edk2.groups.io/g/devel/message/48934
Mute This Topic: https://groups.io/mt/34536814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime cache
Posted by Kubacki, Michael A 6 years, 3 months ago 
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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Monday, October 14, 2019 11:50 AM
> To: devel@edk2.groups.io; Kubacki, Michael A
> <michael.a.kubacki@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming
> <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Wu,
> Hao A <hao.a.wu@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@linaro.org>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Michael Kubacki <michael.a.kubacki@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@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.