From: Min M Xu <min.m.xu@intel.com>

There are 2 functions added for EmuVariableNvStore:
 - PlatformReserveEmuVariableNvStore
 - PlatformInitEmuVariableNvStore

PlatformReserveEmuVariableNvStore allocate storage for NV variables early
on so it will be at a consistent address.

PlatformInitEmuVariableNvStore copies the content in
PcdOvmfFlashNvStorageVariableBase to the storage allocated by
PlatformReserveEmuVariableNvStore. This is used in the case that OVMF is
launched with -bios parameter. Because in that situation UEFI variables
will be partially emulated, and non-volatile variables may lose their
contents after a reboot. This makes the secure boot feature not working.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com> [jejb]
Cc: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
Cc: Tom Lendacky <thomas.lendacky@amd.com> [tlendacky]
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Include/Library/PlatformInitLib.h     | 34 ++++++++
 OvmfPkg/Library/PlatformInitLib/Platform.c    | 77 +++++++++++++++++++
 .../PlatformInitLib/PlatformInitLib.inf       |  2 +
 3 files changed, 113 insertions(+)

diff --git a/OvmfPkg/Include/Library/PlatformInitLib.h b/OvmfPkg/Include/Library/PlatformInitLib.h
index a3acfb1fb196..3a84a56be3c1 100644
--- a/OvmfPkg/Include/Library/PlatformInitLib.h
+++ b/OvmfPkg/Include/Library/PlatformInitLib.h
@@ -251,4 +251,38 @@ TdxValidateCfv (
   IN UINT32  TdxCfvSize
   );
 
+/**
+ Allocate storage for NV variables early on so it will be
+ at a consistent address.  Since VM memory is preserved
+ across reboots, this allows the NV variable storage to survive
+ a VM reboot.
+
+ *
+ * @retval VOID* The pointer to the storage for NV Variables
+ */
+VOID *
+EFIAPI
+PlatformReserveEmuVariableNvStore (
+  VOID
+  );
+
+/**
+ When OVMF is lauched with -bios parameter, UEFI variables will be
+ partially emulated, and non-volatile variables may lose their contents
+ after a reboot. This makes the secure boot feature not working.
+
+ This function is used to initialize the EmuVariableNvStore
+ with the conent in PcdOvmfFlashNvStorageVariableBase.
+
+ @param[in] EmuVariableNvStore      - A pointer to EmuVariableNvStore
+
+ @retval  EFI_SUCCESS   - Successfully init the EmuVariableNvStore
+ @retval  Others        - As the error code indicates
+ */
+EFI_STATUS
+EFIAPI
+PlatformInitEmuVariableNvStore (
+  IN VOID  *EmuVariableNvStore
+  );
+
 #endif // PLATFORM_INIT_LIB_H_
diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c
index c3d34e43af5a..194768379f2b 100644
--- a/OvmfPkg/Library/PlatformInitLib/Platform.c
+++ b/OvmfPkg/Library/PlatformInitLib/Platform.c
@@ -25,6 +25,7 @@
 #include <IndustryStandard/Pci22.h>
 #include <IndustryStandard/Q35MchIch9.h>
 #include <IndustryStandard/QemuCpuHotplug.h>
+#include <Library/MemoryAllocationLib.h>
 #include <Library/QemuFwCfgLib.h>
 #include <Library/QemuFwCfgS3Lib.h>
 #include <Library/QemuFwCfgSimpleParserLib.h>
@@ -576,3 +577,79 @@ PlatformMaxCpuCountInitialization (
   PlatformInfoHob->PcdCpuMaxLogicalProcessorNumber  = MaxCpuCount;
   PlatformInfoHob->PcdCpuBootLogicalProcessorNumber = BootCpuCount;
 }
+
+/**
+ Allocate storage for NV variables early on so it will be
+ at a consistent address.  Since VM memory is preserved
+ across reboots, this allows the NV variable storage to survive
+ a VM reboot.
+
+ *
+ * @retval VOID* The pointer to the storage for NV Variables
+ */
+VOID *
+EFIAPI
+PlatformReserveEmuVariableNvStore (
+  VOID
+  )
+{
+  VOID    *VariableStore;
+  UINT32  VarStoreSize;
+
+  VarStoreSize = 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  //
+  // Allocate storage for NV variables early on so it will be
+  // at a consistent address.  Since VM memory is preserved
+  // across reboots, this allows the NV variable storage to survive
+  // a VM reboot.
+  //
+  VariableStore =
+    AllocateRuntimePages (
+      EFI_SIZE_TO_PAGES (VarStoreSize)
+      );
+  DEBUG ((
+    DEBUG_INFO,
+    "Reserved variable store memory: 0x%p; size: %dkb\n",
+    VariableStore,
+    VarStoreSize / 1024
+    ));
+
+  return VariableStore;
+}
+
+/**
+ When OVMF is lauched with -bios parameter, UEFI variables will be
+ partially emulated, and non-volatile variables may lose their contents
+ after a reboot. This makes the secure boot feature not working.
+
+ This function is used to initialize the EmuVariableNvStore
+ with the conent in PcdOvmfFlashNvStorageVariableBase.
+
+ @param[in] EmuVariableNvStore      - A pointer to EmuVariableNvStore
+
+ @retval  EFI_SUCCESS   - Successfully init the EmuVariableNvStore
+ @retval  Others        - As the error code indicates
+ */
+EFI_STATUS
+EFIAPI
+PlatformInitEmuVariableNvStore (
+  IN VOID  *EmuVariableNvStore
+  )
+{
+  UINT8   *Base;
+  UINT32  Size;
+  UINT32  EmuVariableNvStoreSize;
+
+  EmuVariableNvStoreSize = 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  if ((EmuVariableNvStore == NULL) || (EmuVariableNvStoreSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Base = (UINT8 *)(UINTN)PcdGet32 (PcdOvmfFlashNvStorageVariableBase);
+  Size = (UINT32)PcdGet32 (PcdFlashNvStorageVariableSize);
+  ASSERT (Size < EmuVariableNvStoreSize);
+
+  CopyMem (EmuVariableNvStore, Base, Size);
+
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
index d2fa2d998df8..fec1f8f24314 100644
--- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
@@ -47,6 +47,7 @@
   HobLib
   QemuFwCfgLib
   QemuFwCfgSimpleParserLib
+  MemoryAllocationLib
   MtrrLib
   PcdLib
   PciLib
@@ -96,6 +97,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
 
   gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90756): https://edk2.groups.io/g/devel/message/90756
Mute This Topic: https://groups.io/mt/91995191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> +VOID *
> +EFIAPI
> +PlatformReserveEmuVariableNvStore (
> +  VOID
> +  )

> +  DEBUG ((
> +    DEBUG_INFO,
> +    "Reserved variable store memory: 0x%p; size: %dkb\n",
> +    VariableStore,
> +    VarStoreSize / 1024
> +    ));

> +EFI_STATUS
> +EFIAPI
> +PlatformInitEmuVariableNvStore (
> +  IN VOID  *EmuVariableNvStore
> +  )
> +{

I think it would be good to add a log message to this function too,
to make it easier to check variable initialization actually works
the way it is supposed to work.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90789): https://edk2.groups.io/g/devel/message/90789
Mute This Topic: https://groups.io/mt/91995191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On June 27, 2022 5:10 PM, Gerd Hoffmann wrote:
> > +VOID *
> > +EFIAPI
> > +PlatformReserveEmuVariableNvStore (
> > +  VOID
> > +  )
> 
> > +  DEBUG ((
> > +    DEBUG_INFO,
> > +    "Reserved variable store memory: 0x%p; size: %dkb\n",
> > +    VariableStore,
> > +    VarStoreSize / 1024
> > +    ));
> 
> > +EFI_STATUS
> > +EFIAPI
> > +PlatformInitEmuVariableNvStore (
> > +  IN VOID  *EmuVariableNvStore
> > +  )
> > +{
> 
> I think it would be good to add a log message to this function too, to make it
> easier to check variable initialization actually works the way it is supposed
> to work.
> 
Thanks for reminder. Log will be added.

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90830): https://edk2.groups.io/g/devel/message/90830
Mute This Topic: https://groups.io/mt/91995191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-