[PATCH 0/7] OvmfPkg: small cleanups and tweaks

[PATCH 0/7] OvmfPkg: small cleanups and tweaks

[Laszlo Ersek]

(I'm posting this on my free time. As if there is such a thing.)

Most of these patches have been lying around in my personal tree for
quite some time. They should be uncontroversial easy-to-verify small
improvements (famous last words I guess), so I'll try to flush them now.

They are loosely related to:
- https://bugzilla.tianocore.org/show_bug.cgi?id=386
- https://bugzilla.tianocore.org/show_bug.cgi?id=527

Yes, I tested this with "-bios".

Repo:   https://github.com/lersek/edk2.git
Branch: small_tweaks

Cc: Jordan Justen <jordan.l.justen@intel.com>

Thanks
Laszlo

Laszlo Ersek (7):
  OvmfPkg/EmuVariableFvbRuntimeDxe: always format an auth varstore
    header
  OvmfPkg: remove gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable
  OvmfPkg/PlatformPei: remove unused PcdVariableStoreSize dependency
  OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize
  OvmfPkg/PlatformPei: don't allocate reserved mem varstore if
    SMM_REQUIRE
  OvmfPkg: resolve PcdLib for all PEIMs individually
  OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default

 OvmfPkg/OvmfPkg.dec                      |  1 -
 OvmfPkg/OvmfPkgIa32.dsc                  | 37 ++++-----
 OvmfPkg/OvmfPkgIa32X64.dsc               | 37 ++++-----
 OvmfPkg/OvmfPkgX64.dsc                   | 37 ++++-----
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf |  3 -
 OvmfPkg/PlatformPei/PlatformPei.inf      |  1 -
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c   | 79 ++------------------
 OvmfPkg/PlatformPei/Platform.c           |  4 +-
 8 files changed, 56 insertions(+), 143 deletions(-)

-- 
2.9.3

[PATCH 1/7] OvmfPkg/EmuVariableFvbRuntimeDxe: always format an auth varstore header

[Laszlo Ersek]

In this patch, we extend commit d92eaabefbe0 ("OvmfPkg: simplify
VARIABLE_STORE_HEADER generation", 2016-02-05) to
EmuVariableFvbRuntimeDxe.

This is the difference between FvAndVarTemplate and
FvAndAuthenticatedVarTemplate:

> --- non-auth    2017-05-05 22:32:06.001512283 +0200
> +++ auth        2017-05-05 22:32:18.841364882 +0200
> @@ -1,7 +1,7 @@
>    //
> -  // Templates for standard (non-authenticated) variable FV header
> +  // Templates for authenticated variable FV header
>    //
> -  STATIC FVB_FV_HDR_AND_VARS_TEMPLATE FvAndVarTemplate = {
> +  STATIC FVB_FV_HDR_AND_VARS_TEMPLATE FvAndAuthenticatedVarTemplate = {
>      { // EFI_FIRMWARE_VOLUME_HEADER FvHdr;
>        // UINT8                     ZeroVector[16];
>        { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
> @@ -34,7 +34,7 @@
>        EFI_FVH_REVISION,
>
>        // EFI_FV_BLOCK_MAP_ENTRY    BlockMap[1];
> -      {
> +      {
>          {
>            2, // UINT32 NumBlocks;
>            EMU_FVB_BLOCK_SIZE  // UINT32 Length;
> @@ -44,8 +44,8 @@
>      // EFI_FV_BLOCK_MAP_ENTRY     EndBlockMap;
>      { 0, 0 }, // End of block map
>      { // VARIABLE_STORE_HEADER      VarHdr;
> -      // EFI_GUID  Signature;
> -      EFI_VARIABLE_GUID,
> +        // EFI_GUID  Signature;     // need authenticated variables for secure boot
> +        EFI_AUTHENTICATED_VARIABLE_GUID,
>
>        // UINT32  Size;
>        (

After this change, using "-bios", the variable driver logs:

- with the SB feature enabled:
> Variable driver will work with auth variable format!
> Variable driver will work with auth variable support!

- with the SB feature disabled:
> Variable driver will work with auth variable format!
> Variable driver will continue to work without auth variable support!

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf |  3 -
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c   | 79 ++------------------
 2 files changed, 5 insertions(+), 77 deletions(-)

diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
index 4d4827decb52..69b3c9972a76 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
@@ -68,9 +68,6 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
 
-[FeaturePcd]
-  gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable
-
 [Depex]
   TRUE
 
diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
index dec6d4af50df..7a6d3153ec8c 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
@@ -626,75 +626,6 @@ InitializeFvAndVariableStoreHeaders (
   )
 {
   //
-  // Templates for standard (non-authenticated) variable FV header
-  //
-  STATIC FVB_FV_HDR_AND_VARS_TEMPLATE FvAndVarTemplate = {
-    { // EFI_FIRMWARE_VOLUME_HEADER FvHdr;
-      // UINT8                     ZeroVector[16];
-      { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
-
-      // EFI_GUID                  FileSystemGuid;
-      EFI_SYSTEM_NV_DATA_FV_GUID,
-
-      // UINT64                    FvLength;
-      EMU_FVB_SIZE,
-
-      // UINT32                    Signature;
-      EFI_FVH_SIGNATURE,
-
-      // EFI_FVB_ATTRIBUTES_2      Attributes;
-      0x4feff,
-
-      // UINT16                    HeaderLength;
-      EMU_FV_HEADER_LENGTH,
-
-      // UINT16                    Checksum;
-      0,
-
-      // UINT16                    ExtHeaderOffset;
-      0,
-
-      // UINT8                     Reserved[1];
-      {0},
-
-      // UINT8                     Revision;
-      EFI_FVH_REVISION,
-
-      // EFI_FV_BLOCK_MAP_ENTRY    BlockMap[1];
-      { 
-        {
-          2, // UINT32 NumBlocks;
-          EMU_FVB_BLOCK_SIZE  // UINT32 Length;
-        }
-      }
-    },
-    // EFI_FV_BLOCK_MAP_ENTRY     EndBlockMap;
-    { 0, 0 }, // End of block map
-    { // VARIABLE_STORE_HEADER      VarHdr;
-      // EFI_GUID  Signature;
-      EFI_VARIABLE_GUID,
-
-      // UINT32  Size;
-      (
-        FixedPcdGet32 (PcdVariableStoreSize) -
-        OFFSET_OF (FVB_FV_HDR_AND_VARS_TEMPLATE, VarHdr)
-      ),
-
-      // UINT8   Format;
-      VARIABLE_STORE_FORMATTED,
-
-      // UINT8   State;
-      VARIABLE_STORE_HEALTHY,
-
-      // UINT16  Reserved;
-      0,
-
-      // UINT32  Reserved1;
-      0
-    }
-  };
-
-  //
   // Templates for authenticated variable FV header
   //
   STATIC FVB_FV_HDR_AND_VARS_TEMPLATE FvAndAuthenticatedVarTemplate = {
@@ -768,11 +699,11 @@ InitializeFvAndVariableStoreHeaders (
   //
   // Copy the template structure into the location
   //
-  if (FeaturePcdGet (PcdSecureBootEnable) == FALSE) {
-    CopyMem (Ptr, (VOID*)&FvAndVarTemplate, sizeof (FvAndVarTemplate));
-  } else {
-    CopyMem (Ptr, (VOID*)&FvAndAuthenticatedVarTemplate, sizeof (FvAndAuthenticatedVarTemplate));
-  }
+  CopyMem (
+    Ptr,
+    &FvAndAuthenticatedVarTemplate,
+    sizeof FvAndAuthenticatedVarTemplate
+    );
 
   //
   // Update the checksum for the FV header
-- 
2.9.3

[PATCH 2/7] OvmfPkg: remove gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable

[Laszlo Ersek]

This PCD is no longer used.

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

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 3490f7ca7c07..5627be0bab0a 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -136,7 +136,6 @@ [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x0|UINT64|0x27
 
 [PcdsFeatureFlag]
-  gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|FALSE|BOOLEAN|3
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
 
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index e0779ddaa426..8f8d3472102a 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -400,9 +400,6 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
-!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index bbe26e2cf452..fd554f8f375c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -405,9 +405,6 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
-!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ff795815f65f..3bcd488fc7a8 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -405,9 +405,6 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
-!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
-- 
2.9.3

[PATCH 3/7] OvmfPkg/PlatformPei: remove unused PcdVariableStoreSize dependency

[Laszlo Ersek]

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/PlatformPei/PlatformPei.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 53c6dd445a0e..a1e12c1fc7e2 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -84,7 +84,6 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
-- 
2.9.3

[PATCH 4/7] OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize

[Laszlo Ersek]

"MdeModulePkg/MdeModulePkg.dec" declares PcdVariableStoreSize like this:

> The size of volatile buffer. This buffer is used to store VOLATILE
> attribute variables.

There is no inherent reason why the size of the volatile variable store
should match the same of the non-volatile variable store. Indeed flash
variables in the 4MB build work fine without this equality.

However, OvmfPkg/EmuVariableFvbRuntimeDxe uses PcdVariableStoreSize to
initialize the non-volatile VARIABLE_STORE_HEADER too. (Presumably based
on the fact that ultimately that storage will not be permanent.) When
using EmuVariableFvbRuntimeDxe in the 4MB build, the mismatch between the
two mentioned PCDs (which is apparent through EmuVariableFvbRuntimeDxe's
VARIABLE_STORE_HEADER) triggers an assertion in the variable driver:

> ASSERT MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c(3772):
> mNvVariableCache->Size == VariableStoreLength

Bringing PcdVariableStoreSize in sync with PcdFlashNvStorageVariableSize
fixes this. It also happens to ensure a volatile store size in the 4MB
build that equals the non-volatile store size, which likely doesn't hurt
for symmetry.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Fixes: b24fca05751f8222acf264853709012e0ab7bf49
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 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 8f8d3472102a..64427716c53c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -414,12 +414,13 @@ [PcdsFixedAtBuild]
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 !endif
 !if $(FD_SIZE_IN_KB) == 4096
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
 !endif
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index fd554f8f375c..887964cd27c2 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -419,12 +419,13 @@ [PcdsFixedAtBuild]
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 !endif
 !if $(FD_SIZE_IN_KB) == 4096
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
 !endif
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3bcd488fc7a8..dc5fea3577d4 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -419,12 +419,13 @@ [PcdsFixedAtBuild]
 !if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 !endif
 !if $(FD_SIZE_IN_KB) == 4096
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
 !endif
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
-- 
2.9.3

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

[Laszlo Ersek]

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

[PATCH 6/7] OvmfPkg: resolve PcdLib for all PEIMs individually

[Laszlo Ersek]

Currently the default (module type independent) PcdLib resolution is to
BasePcdLibNull.inf, which is inherited by all PEIMs. In the next patch,
we'll flip the PEIM default resolution to PeiPcdLib.inf, but in order to
keep that patch both correct and simple to review, we should spell out the
Null resolution for those two PEIMs (ReportStatusCodeRouterPei and
StatusCodeHandlerPei) that are now the only ones that don't specify an
explicit resolution.

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    | 10 ++++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc | 10 ++++++++--
 OvmfPkg/OvmfPkgX64.dsc     | 10 ++++++++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index b46eef6cabc3..4fc26c0c66a0 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -559,8 +559,14 @@ [Components]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
-  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
+  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
     <LibraryClasses>
       PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 08f471fbc542..b82c30656a83 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -567,8 +567,14 @@ [Components.IA32]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
-  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
+  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
     <LibraryClasses>
       PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 24053e5ff82d..f553a0a1674a 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -566,8 +566,14 @@ [Components]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
-  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
+  MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
     <LibraryClasses>
       PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-- 
2.9.3

[PATCH 7/7] OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default

[Laszlo Ersek]

In the previous patch we had to add two explicit Null resolutions, but
here we can remove five PeiPcdLib ones, after setting the default to it.

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    | 22 +++++---------------
 OvmfPkg/OvmfPkgIa32X64.dsc | 22 +++++---------------
 OvmfPkg/OvmfPkgX64.dsc     | 22 +++++---------------
 3 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 4fc26c0c66a0..bd115c9ced93 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -256,6 +256,7 @@ [LibraryClasses.common.PEIM]
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
+  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -567,32 +568,19 @@ [Components]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/PlatformPei/PlatformPei.inf
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 !if $(SMM_REQUIRE) == TRUE
       LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
 !endif
   }
 !if $(SMM_REQUIRE) == TRUE
-  OvmfPkg/SmmAccess/SmmAccessPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
-  UefiCpuPkg/CpuMpPei/CpuMpPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
   #
   # DXE Phase modules
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b82c30656a83..9727db842922 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -261,6 +261,7 @@ [LibraryClasses.common.PEIM]
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
+  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -575,32 +576,19 @@ [Components.IA32]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/PlatformPei/PlatformPei.inf
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 !if $(SMM_REQUIRE) == TRUE
       LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
 !endif
   }
 !if $(SMM_REQUIRE) == TRUE
-  OvmfPkg/SmmAccess/SmmAccessPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
-  UefiCpuPkg/CpuMpPei/CpuMpPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
 [Components.X64]
   #
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f553a0a1674a..61aaed761657 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -261,6 +261,7 @@ [LibraryClasses.common.PEIM]
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
+  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -574,32 +575,19 @@ [Components]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
-  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/PlatformPei/PlatformPei.inf
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
 !if $(SMM_REQUIRE) == TRUE
       LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
 !endif
   }
 !if $(SMM_REQUIRE) == TRUE
-  OvmfPkg/SmmAccess/SmmAccessPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
-  UefiCpuPkg/CpuMpPei/CpuMpPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
+  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
   #
   # DXE Phase modules
-- 
2.9.3

Re: [PATCH 0/7] OvmfPkg: small cleanups and tweaks

[Gary Lin]

On Fri, May 05, 2017 at 11:02:51PM +0200, Laszlo Ersek wrote:
> (I'm posting this on my free time. As if there is such a thing.)
> 
> Most of these patches have been lying around in my personal tree for
> quite some time. They should be uncontroversial easy-to-verify small
> improvements (famous last words I guess), so I'll try to flush them now.
> 
> They are loosely related to:
> - https://bugzilla.tianocore.org/show_bug.cgi?id=386
> - https://bugzilla.tianocore.org/show_bug.cgi?id=527
> 
> Yes, I tested this with "-bios".
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: small_tweaks
> 
For the series, Regression-tested-by: Gary Lin <glin@suse.com>

> Cc: Jordan Justen <jordan.l.justen@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (7):
>   OvmfPkg/EmuVariableFvbRuntimeDxe: always format an auth varstore
>     header
>   OvmfPkg: remove gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable
>   OvmfPkg/PlatformPei: remove unused PcdVariableStoreSize dependency
>   OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize
>   OvmfPkg/PlatformPei: don't allocate reserved mem varstore if
>     SMM_REQUIRE
>   OvmfPkg: resolve PcdLib for all PEIMs individually
>   OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default
> 
>  OvmfPkg/OvmfPkg.dec                      |  1 -
>  OvmfPkg/OvmfPkgIa32.dsc                  | 37 ++++-----
>  OvmfPkg/OvmfPkgIa32X64.dsc               | 37 ++++-----
>  OvmfPkg/OvmfPkgX64.dsc                   | 37 ++++-----
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf |  3 -
>  OvmfPkg/PlatformPei/PlatformPei.inf      |  1 -
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c   | 79 ++------------------
>  OvmfPkg/PlatformPei/Platform.c           |  4 +-
>  8 files changed, 56 insertions(+), 143 deletions(-)
> 
> -- 
> 2.9.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

[Jordan Justen]

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
> 
> 

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

[Laszlo Ersek]

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
>>
>>