1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
  4. View patch

[edk2] [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE

Laszlo Ersek posted 7 patches 7 years, 7 months ago
[edk2] [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE
Posted by Laszlo Ersek 7 years, 7 months ago 
For the emulated variable store, PlatformPei allocates reserved memory (as
early as possible, so that the address remains the same during reboot),
and PcdEmuVariableNvStoreReserved carries the address to
EmuVariableFvbRuntimeDxe.

However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build,
and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste
reserved memory whenever that's the case.

(Even a dynamic default for PcdEmuVariableNvStoreReserved would be
unnecessary; but that way the PcdSet64S() call in the
ReserveEmuVariableNvStore() function doesn't compile.)

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc        | 3 +++
 OvmfPkg/OvmfPkgIa32X64.dsc     | 3 +++
 OvmfPkg/OvmfPkgX64.dsc         | 3 +++
 OvmfPkg/PlatformPei/Platform.c | 4 +++-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 64427716c53c..b46eef6cabc3 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -495,7 +495,10 @@ [PcdsFixedAtBuild]
 ################################################################################
 
 [PcdsDynamicDefault]
+  # only set when
+  #   ($(SMM_REQUIRE) == FALSE)
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
+
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 887964cd27c2..08f471fbc542 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -501,7 +501,10 @@ [PcdsFixedAtBuild.X64]
 ################################################################################
 
 [PcdsDynamicDefault]
+  # only set when
+  #   ($(SMM_REQUIRE) == FALSE)
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
+
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index dc5fea3577d4..24053e5ff82d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -500,7 +500,10 @@ [PcdsFixedAtBuild]
 ################################################################################
 
 [PcdsDynamicDefault]
+  # only set when
+  #   ($(SMM_REQUIRE) == FALSE)
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
+
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 5e983a8dcea9..1b4dc00b0180 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -672,7 +672,9 @@ InitializePlatform (
   mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
 
   if (mBootMode != BOOT_ON_S3_RESUME) {
-    ReserveEmuVariableNvStore ();
+    if (!FeaturePcdGet (PcdSmmSmramRequire)) {
+      ReserveEmuVariableNvStore ();
+    }
     PeiFvInitialization ();
     MemMapInitialization ();
     NoexecDxeInitialization ();
-- 
2.9.3


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE
Posted by Jordan Justen 7 years, 7 months ago 
On 2017-05-05 14:02:56, Laszlo Ersek wrote:
> For the emulated variable store, PlatformPei allocates reserved memory (as
> early as possible, so that the address remains the same during reboot),
> and PcdEmuVariableNvStoreReserved carries the address to
> EmuVariableFvbRuntimeDxe.
> 
> However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build,
> and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste
> reserved memory whenever that's the case.
> 
> (Even a dynamic default for PcdEmuVariableNvStoreReserved would be
> unnecessary; but that way the PcdSet64S() call in the
> ReserveEmuVariableNvStore() function doesn't compile.)
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc        | 3 +++
>  OvmfPkg/OvmfPkgIa32X64.dsc     | 3 +++
>  OvmfPkg/OvmfPkgX64.dsc         | 3 +++
>  OvmfPkg/PlatformPei/Platform.c | 4 +++-
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 64427716c53c..b46eef6cabc3 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -495,7 +495,10 @@ [PcdsFixedAtBuild]
>  ################################################################################
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 887964cd27c2..08f471fbc542 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -501,7 +501,10 @@ [PcdsFixedAtBuild.X64]
>  ################################################################################
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)

I don't think we should bother adding these comments into the .dsc.

Ultimately, I would prefer to always allocate this, even when SMM is
set to be required. It'd be nice if we could always fallback to
EmuFvb, but I understand that this might not be possible given how
difficult it is to determine if QEMU actually has SMM enabled. Anyway,
I think this patch makes sense until we can potentially fix that.
(Which may or may not be worth fixing.)

Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index dc5fea3577d4..24053e5ff82d 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -500,7 +500,10 @@ [PcdsFixedAtBuild]
>  ################################################################################
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 5e983a8dcea9..1b4dc00b0180 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -672,7 +672,9 @@ InitializePlatform (
>    mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
>  
>    if (mBootMode != BOOT_ON_S3_RESUME) {
> -    ReserveEmuVariableNvStore ();
> +    if (!FeaturePcdGet (PcdSmmSmramRequire)) {
> +      ReserveEmuVariableNvStore ();
> +    }
>      PeiFvInitialization ();
>      MemMapInitialization ();
>      NoexecDxeInitialization ();
> -- 
> 2.9.3
> 
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE
Posted by Laszlo Ersek 7 years, 7 months ago 
On 05/15/17 20:09, Jordan Justen wrote:
> On 2017-05-05 14:02:56, Laszlo Ersek wrote:
>> For the emulated variable store, PlatformPei allocates reserved memory (as
>> early as possible, so that the address remains the same during reboot),
>> and PcdEmuVariableNvStoreReserved carries the address to
>> EmuVariableFvbRuntimeDxe.
>>
>> However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build,
>> and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste
>> reserved memory whenever that's the case.
>>
>> (Even a dynamic default for PcdEmuVariableNvStoreReserved would be
>> unnecessary; but that way the PcdSet64S() call in the
>> ReserveEmuVariableNvStore() function doesn't compile.)
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  OvmfPkg/OvmfPkgIa32.dsc        | 3 +++
>>  OvmfPkg/OvmfPkgIa32X64.dsc     | 3 +++
>>  OvmfPkg/OvmfPkgX64.dsc         | 3 +++
>>  OvmfPkg/PlatformPei/Platform.c | 4 +++-
>>  4 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 64427716c53c..b46eef6cabc3 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -495,7 +495,10 @@ [PcdsFixedAtBuild]
>>  ################################################################################
>>  
>>  [PcdsDynamicDefault]
>> +  # only set when
>> +  #   ($(SMM_REQUIRE) == FALSE)
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
>> +
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 887964cd27c2..08f471fbc542 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -501,7 +501,10 @@ [PcdsFixedAtBuild.X64]
>>  ################################################################################
>>  
>>  [PcdsDynamicDefault]
>> +  # only set when
>> +  #   ($(SMM_REQUIRE) == FALSE)
> 
> I don't think we should bother adding these comments into the .dsc.
> 
> Ultimately, I would prefer to always allocate this, even when SMM is
> set to be required. It'd be nice if we could always fallback to
> EmuFvb, but I understand that this might not be possible given how
> difficult it is to determine if QEMU actually has SMM enabled. Anyway,
> I think this patch makes sense until we can potentially fix that.
> (Which may or may not be worth fixing.)
> 
> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Series committed as 11a6cc5bda81..639c7dd86d1d.

Thank you,
Laszlo

> 
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
>> +
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index dc5fea3577d4..24053e5ff82d 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -500,7 +500,10 @@ [PcdsFixedAtBuild]
>>  ################################################################################
>>  
>>  [PcdsDynamicDefault]
>> +  # only set when
>> +  #   ($(SMM_REQUIRE) == FALSE)
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
>> +
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
>> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
>> index 5e983a8dcea9..1b4dc00b0180 100644
>> --- a/OvmfPkg/PlatformPei/Platform.c
>> +++ b/OvmfPkg/PlatformPei/Platform.c
>> @@ -672,7 +672,9 @@ InitializePlatform (
>>    mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
>>  
>>    if (mBootMode != BOOT_ON_S3_RESUME) {
>> -    ReserveEmuVariableNvStore ();
>> +    if (!FeaturePcdGet (PcdSmmSmramRequire)) {
>> +      ReserveEmuVariableNvStore ();
>> +    }
>>      PeiFvInitialization ();
>>      MemMapInitialization ();
>>      NoexecDxeInitialization ();
>> -- 
>> 2.9.3
>>
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.