public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 00/12] OvmfPkg: Enable variable access in PEI
@ 2017-03-27  8:05 Jordan Justen
  2017-03-27  8:05 ` [PATCH 01/12] OvmfPkg/build.sh: Add support for --disable-flash switch Jordan Justen
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen

web: https://github.com/jljusten/edk2/tree/pei-vars-v1

git: https://github.com/jljusten/edk2.git pei-vars-v1

This series moves flash detection into PEI to allow the PEI variable
access drivers to run. If flash is writable, the PCDs are set to point
at the flash memory. If flash is not writable, the PCDs are set to
point at a memory buffer.

I tested KVM with ROM and writable flash, with S3 sleep/resume. I did
not test SMM.

Jordan Justen (10):
  OvmfPkg/build.sh: Add support for --disable-flash switch
  OvmfPkg QemuFlash: Make QemuFlash.* Base class safe
  OvmfPkg QemuFlash: Make QemuFlashDetected external
  OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library
  OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable
  OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable
  OvmfPkg PlatformPei: Set flash variable PCDs
  OvmfPkg PlatformPei: Initialize memory based variable store buffer
  OvmfPkg: Enable PEI variable access
  OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI

Laszlo Ersek (2):
  OvmfPkg: resolve PcdLib for all PEIMs individually
  OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default

 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c             |   4 +-
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf           |   3 +-
 OvmfPkg/OvmfPkgIa32.dsc                            |  27 +-
 OvmfPkg/OvmfPkgIa32.fdf                            |   5 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                         |  27 +-
 OvmfPkg/OvmfPkgIa32X64.fdf                         |   5 +-
 OvmfPkg/OvmfPkgX64.dsc                             |  27 +-
 OvmfPkg/OvmfPkgX64.fdf                             |   5 +-
 OvmfPkg/PlatformPei/Platform.c                     |  34 +--
 OvmfPkg/PlatformPei/Platform.h                     |   7 +-
 OvmfPkg/PlatformPei/PlatformPei.inf                |  11 +-
 OvmfPkg/PlatformPei/Vars.c                         | 283 +++++++++++++++++++++
 .../DetectFlashNullLib.c                           |  41 +++
 .../DetectFlashNullLib.inf                         |  60 +++++
 .../FvbServicesRuntimeDxe.inf                      |   4 -
 .../FvbServicesSmm.inf                             |   4 -
 .../FwBlockService.c                               |  28 +-
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c |  59 +++--
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h |  30 ++-
 OvmfPkg/build.sh                                   |  10 +-
 20 files changed, 530 insertions(+), 144 deletions(-)
 create mode 100644 OvmfPkg/PlatformPei/Vars.c
 create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
 create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf

-- 
2.11.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/12] OvmfPkg/build.sh: Add support for --disable-flash switch
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 02/12] OvmfPkg: resolve PcdLib for all PEIMs individually Jordan Justen
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

This is mainly useful to test the boot paths when flash hasn't been
enabled.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 6821552025..7294198e2c 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -50,6 +50,7 @@ THREADNUMBER=1
 LAST_ARG=
 RUN_QEMU=no
 ENABLE_FLASH=no
+DISABLE_FLASH=no
 
 #
 # Pick a default tool type for a given OS
@@ -127,6 +128,9 @@ do
         shift
         break
         ;;
+      --disable-flash)
+        DISABLE_FLASH=yes
+        ;;
       --enable-flash)
         ENABLE_FLASH=yes
         ;;
@@ -228,7 +232,9 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
                      awk '{print $2}')
   case $qemu_version in
     1.[6-9].*|1.[1-9][0-9].*|2.*.*)
-      ENABLE_FLASH=yes
+      if [[ "$DISABLE_FLASH" != "yes" ]]; then
+        ENABLE_FLASH=yes
+      fi
       ;;
   esac
 
@@ -283,7 +289,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
   if [[ "$ENABLE_FLASH" == "yes" ]]; then
     QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin"
   else
-    QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR"
+    QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR -bios $QEMU_FIRMWARE_DIR/bios.bin"
   fi
   if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
     QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/12] OvmfPkg: resolve PcdLib for all PEIMs individually
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
  2017-03-27  8:05 ` [PATCH 01/12] OvmfPkg/build.sh: Add support for --disable-flash switch Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 03/12] OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default Jordan Justen
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Laszlo Ersek, Jordan Justen

From: Laszlo Ersek <lersek@redhat.com>

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>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.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 0796b0db81..7c588a0499 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -533,8 +533,14 @@
     <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 71ac62f023..6b540ae2ed 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -541,8 +541,14 @@
     <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 2ceb31d7ff..5486ecb1e1 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -540,8 +540,14 @@
     <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.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/12] OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
  2017-03-27  8:05 ` [PATCH 01/12] OvmfPkg/build.sh: Add support for --disable-flash switch Jordan Justen
  2017-03-27  8:05 ` [PATCH 02/12] OvmfPkg: resolve PcdLib for all PEIMs individually Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 04/12] OvmfPkg QemuFlash: Make QemuFlash.* Base class safe Jordan Justen
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Laszlo Ersek, Jordan Justen

From: Laszlo Ersek <lersek@redhat.com>

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>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.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 7c588a0499..56ff1557dc 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -237,6 +237,7 @@
   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
@@ -541,32 +542,19 @@
     <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 6b540ae2ed..e16a7a5a04 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -242,6 +242,7 @@
   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
@@ -549,32 +550,19 @@
     <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 5486ecb1e1..2c798ea7f2 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -242,6 +242,7 @@
   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
@@ -548,32 +549,19 @@
     <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.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/12] OvmfPkg QemuFlash: Make QemuFlash.* Base class safe
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (2 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 03/12] OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 05/12] OvmfPkg QemuFlash: Make QemuFlashDetected external Jordan Justen
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

We will create a small 'NULL' base library to detect flash in PEI.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c | 30 +++++++++++-----------
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h | 16 +++++-------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
index 5677b5ee11..68388048f3 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
@@ -1,7 +1,7 @@
 /** @file
   OVMF support for QEMU system firmware flash device
 
-  Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
 
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
@@ -124,7 +124,7 @@ QemuFlashDetected (
   @param[in] Buffer   Pointer to the buffer to read into.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashRead (
   IN        EFI_LBA                              Lba,
   IN        UINTN                                Offset,
@@ -139,7 +139,7 @@ QemuFlashRead (
   // block into the flash memory.
   //
   if (Lba >= mFdBlockCount) {
-    return EFI_INVALID_PARAMETER;
+    return RETURN_INVALID_PARAMETER;
   }
 
   //
@@ -149,7 +149,7 @@ QemuFlashRead (
 
   CopyMem (Buffer, Ptr, *NumBytes);
 
-  return EFI_SUCCESS;
+  return RETURN_SUCCESS;
 }
 
 
@@ -163,7 +163,7 @@ QemuFlashRead (
   @param[in] Buffer   Pointer to the data to write.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashWrite (
   IN        EFI_LBA                             Lba,
   IN        UINTN                               Offset,
@@ -179,7 +179,7 @@ QemuFlashWrite (
   // block into the flash memory.
   //
   if (Lba >= mFdBlockCount) {
-    return EFI_INVALID_PARAMETER;
+    return RETURN_INVALID_PARAMETER;
   }
 
   //
@@ -199,7 +199,7 @@ QemuFlashWrite (
     *(Ptr - 1) = READ_ARRAY_CMD;
   }
 
-  return EFI_SUCCESS;
+  return RETURN_SUCCESS;
 }
 
 
@@ -209,7 +209,7 @@ QemuFlashWrite (
   @param Lba    The logical block index to erase.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashEraseBlock (
   IN   EFI_LBA      Lba
   )
@@ -217,24 +217,24 @@ QemuFlashEraseBlock (
   volatile UINT8  *Ptr;
 
   if (Lba >= mFdBlockCount) {
-    return EFI_INVALID_PARAMETER;
+    return RETURN_INVALID_PARAMETER;
   }
 
   Ptr = QemuFlashPtr (Lba, 0);
   *Ptr = BLOCK_ERASE_CMD;
   *Ptr = BLOCK_ERASE_CONFIRM_CMD;
-  return EFI_SUCCESS;
+  return RETURN_SUCCESS;
 }
 
 
 /**
   Initializes QEMU flash memory support
 
-  @retval EFI_WRITE_PROTECTED   The QEMU flash device is not present.
-  @retval EFI_SUCCESS           The QEMU flash device is supported.
+  @retval RETURN_WRITE_PROTECTED   The QEMU flash device is not present.
+  @retval RETURN_SUCCESS           The QEMU flash device is supported.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashInitialize (
   VOID
   )
@@ -246,9 +246,9 @@ QemuFlashInitialize (
 
   if (!QemuFlashDetected ()) {
     ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
-    return EFI_WRITE_PROTECTED;
+    return RETURN_WRITE_PROTECTED;
   }
 
-  return EFI_SUCCESS;
+  return RETURN_SUCCESS;
 }
 
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
index 8d83dca7a5..4bd971b0d1 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
@@ -1,7 +1,7 @@
 /** @file
   OVMF support for QEMU system firmware flash device
 
-  Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
 
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
@@ -16,8 +16,6 @@
 #ifndef __QEMU_FLASH_H__
 #define __QEMU_FLASH_H__
 
-#include <Protocol/FirmwareVolumeBlock.h>
-
 extern UINT8 *mFlashBase;
 
 /**
@@ -30,7 +28,7 @@ extern UINT8 *mFlashBase;
   @param[in] Buffer   Pointer to the buffer to read into.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashRead (
   IN        EFI_LBA                              Lba,
   IN        UINTN                                Offset,
@@ -49,7 +47,7 @@ QemuFlashRead (
   @param[in] Buffer   Pointer to the data to write.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashWrite (
   IN        EFI_LBA                              Lba,
   IN        UINTN                                Offset,
@@ -64,7 +62,7 @@ QemuFlashWrite (
   @param Lba    The logical block index to erase.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashEraseBlock (
   IN   EFI_LBA      Lba
   );
@@ -73,11 +71,11 @@ QemuFlashEraseBlock (
 /**
   Initializes QEMU flash memory support
 
-  @retval EFI_WRITE_PROTECTED   The QEMU flash device is not present.
-  @retval EFI_SUCCESS           The QEMU flash device is supported.
+  @retval RETURN_WRITE_PROTECTED   The QEMU flash device is not present.
+  @retval RETURN_SUCCESS           The QEMU flash device is supported.
 
 **/
-EFI_STATUS
+RETURN_STATUS
 QemuFlashInitialize (
   VOID
   );
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/12] OvmfPkg QemuFlash: Make QemuFlashDetected external
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (3 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 04/12] OvmfPkg QemuFlash: Make QemuFlash.* Base class safe Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 06/12] OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library Jordan Justen
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

QemuFlashDetected is also changed to not use global variables.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c | 29 +++++++++++++++++-----
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h | 14 +++++++++++
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
index 68388048f3..90e7810733 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
@@ -16,6 +16,7 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
+#include <Uefi.h>
 
 #include "QemuFlash.h"
 
@@ -37,12 +38,25 @@ STATIC UINTN       mFdBlockCount = 0;
 
 STATIC
 volatile UINT8*
+QemuFlashPtrEx (
+  UINT8     *FlashBase,
+  UINTN     BlockSize,
+  IN        EFI_LBA                             Lba,
+  IN        UINTN                               Offset
+  )
+{
+  return FlashBase + ((UINTN)Lba * BlockSize) + Offset;
+}
+
+
+STATIC
+volatile UINT8*
 QemuFlashPtr (
   IN        EFI_LBA                             Lba,
   IN        UINTN                               Offset
   )
 {
-  return mFlashBase + ((UINTN)Lba * mFdBlockSize) + Offset;
+  return QemuFlashPtrEx (mFlashBase, mFdBlockSize, Lba, Offset);
 }
 
 
@@ -53,7 +67,6 @@ QemuFlashPtr (
   @retval TRUE    The QEMU flash device is present.
 
 **/
-STATIC
 BOOLEAN
 QemuFlashDetected (
   VOID
@@ -66,11 +79,15 @@ QemuFlashDetected (
   UINT8 OriginalUint8;
   UINT8 ProbeUint8;
 
+  UINT8 *FlashBase = (UINT8*)(UINTN) PcdGet32 (PcdOvmfFdBaseAddress);
+  UINTN BlockSize = PcdGet32 (PcdOvmfFirmwareBlockSize);
+  ASSERT(PcdGet32 (PcdOvmfFirmwareFdSize) % BlockSize == 0);
+
   FlashDetected = FALSE;
-  Ptr = QemuFlashPtr (0, 0);
+  Ptr = QemuFlashPtrEx (FlashBase, BlockSize, 0, 0);
 
-  for (Offset = 0; Offset < mFdBlockSize; Offset++) {
-    Ptr = QemuFlashPtr (0, Offset);
+  for (Offset = 0; Offset < BlockSize; Offset++) {
+    Ptr = QemuFlashPtrEx (FlashBase, BlockSize, 0, Offset);
     ProbeUint8 = *Ptr;
     if (ProbeUint8 != CLEAR_STATUS_CMD &&
         ProbeUint8 != READ_STATUS_CMD &&
@@ -79,7 +96,7 @@ QemuFlashDetected (
     }
   }
 
-  if (Offset >= mFdBlockSize) {
+  if (Offset >= BlockSize) {
     DEBUG ((EFI_D_INFO, "QEMU Flash: Failed to find probe location\n"));
     return FALSE;
   }
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
index 4bd971b0d1..04f89364ab 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h
@@ -19,6 +19,20 @@
 extern UINT8 *mFlashBase;
 
 /**
+  Detect if QEMU Flash is available and writable
+
+  Note: This function does not use read or write global variables.
+
+  @retval TRUE  Flash is writable
+
+**/
+BOOLEAN
+QemuFlashDetected (
+  VOID
+  );
+
+
+/**
   Read from QEMU Flash
 
   @param[in] Lba      The starting logical block index to read from.
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/12] OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (4 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 05/12] OvmfPkg QemuFlash: Make QemuFlashDetected external Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 07/12] OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable Jordan Justen
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

This library attempts to detect QEMU flash. If writable flash is
detected, then PcdOvmfFlashVariablesEnable is set to TRUE.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 .../DetectFlashNullLib.c                           | 41 +++++++++++++++
 .../DetectFlashNullLib.inf                         | 60 ++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
 create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
new file mode 100644
index 0000000000..cfa6026ff2
--- /dev/null
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
@@ -0,0 +1,41 @@
+/** @file
+  OVMF support for QEMU system firmware flash device
+
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Uefi.h>
+
+#include "QemuFlash.h"
+
+
+/**
+  Initializes QEMU flash memory support
+
+  @retval !RETURN_SUCCESS          Failed to set the PCD
+  @retval RETURN_SUCCESS           The constructor was successful
+
+**/
+RETURN_STATUS
+EFIAPI
+DetectFlashConstructor (
+  VOID
+  )
+{
+  if (QemuFlashDetected ()) {
+    return PcdSetBoolS (PcdOvmfFlashVariablesEnable, TRUE);
+  }
+
+  return RETURN_SUCCESS;
+}
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
new file mode 100644
index 0000000000..1b7717af3e
--- /dev/null
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
@@ -0,0 +1,60 @@
+## @file
+#  This is a small library with a constructor that will set a PCD if
+#  writable flash is detected.
+#
+#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+#  IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DetectFlashNullLib
+  FILE_GUID                      = 6ec93337-513a-4188-bf57-7bb746c6e8ac
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL|PEIM DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
+  CONSTRUCTOR                    = DetectFlashConstructor
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  DetectFlashNullLib.c
+  QemuFlash.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  PcdLib
+
+[FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
+
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
+
+[Depex]
+  TRUE
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/12] OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (5 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 06/12] OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 08/12] OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable Jordan Justen
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

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

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 56ff1557dc..ee1af50a14 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -544,7 +544,10 @@
   }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf
+  OvmfPkg/PlatformPei/PlatformPei.inf {
+    <LibraryClasses>
+      NULL|OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
+  }
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
 !if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index e16a7a5a04..a429ceff6b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -552,7 +552,10 @@
   }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf
+  OvmfPkg/PlatformPei/PlatformPei.inf {
+    <LibraryClasses>
+      NULL|OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
+  }
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
 !if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 2c798ea7f2..21f52a0976 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -551,7 +551,10 @@
   }
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
-  OvmfPkg/PlatformPei/PlatformPei.inf
+  OvmfPkg/PlatformPei/PlatformPei.inf {
+    <LibraryClasses>
+      NULL|OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
+  }
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf {
     <LibraryClasses>
 !if $(SMM_REQUIRE) == TRUE
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/12] OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (6 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 07/12] OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 09/12] OvmfPkg PlatformPei: Set flash variable PCDs Jordan Justen
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

Previously we looked at PcdFlashNvStorageVariableBase64, which would
not be set unless flash was in use, but soon we will set
PcdFlashNvStorageVariableBase64 even for the memory based variable
store.

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

diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
index dec6d4af50..6abed7d5b6 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
@@ -2,7 +2,7 @@
   Firmware Block Services to support emulating non-volatile variables
   by pretending that a memory buffer is storage for the NV variables.
 
-  Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -820,7 +820,7 @@ FvbInitialize (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+  if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {
     DEBUG ((EFI_D_INFO, "Disabling EMU Variable FVB since "
                         "flash variables appear to be supported.\n"));
     return EFI_ABORTED;
diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
index 4d4827decb..05e9dd8915 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
@@ -2,7 +2,7 @@
 #  Firmware Block Services to support emulating non-volatile variables
 #  by pretending that a memory buffer is storage for the NV variables.
 #
-#  Copyright (c) 2008 - 2011, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -67,6 +67,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/12] OvmfPkg PlatformPei: Set flash variable PCDs
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (7 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 08/12] OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 10/12] OvmfPkg PlatformPei: Initialize memory based variable store buffer Jordan Justen
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

This allows the PEI based variable drivers to run.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/PlatformPei/Platform.c      |  34 +---------
 OvmfPkg/PlatformPei/Platform.h      |   7 ++-
 OvmfPkg/PlatformPei/PlatformPei.inf |  10 ++-
 OvmfPkg/PlatformPei/Vars.c          | 122 ++++++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+), 33 deletions(-)
 create mode 100644 OvmfPkg/PlatformPei/Vars.c

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 77a8a16c15..2e943d6e7b 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -1,7 +1,7 @@
 /**@file
   Platform PEI driver
 
-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2011, Andrei Warkentin <andreiw@motorola.com>
 
   This program and the accompanying materials
@@ -499,35 +499,6 @@ BootModeInitialization (
 
 
 VOID
-ReserveEmuVariableNvStore (
-  )
-{
-  EFI_PHYSICAL_ADDRESS VariableStore;
-  RETURN_STATUS        PcdStatus;
-
-  //
-  // 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 =
-    (EFI_PHYSICAL_ADDRESS)(UINTN)
-      AllocateAlignedRuntimePages (
-        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)),
-        PcdGet32 (PcdFlashNvStorageFtwSpareSize)
-        );
-  DEBUG ((EFI_D_INFO,
-          "Reserved variable store memory: 0x%lX; size: %dkb\n",
-          VariableStore,
-          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
-        ));
-  PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
-  ASSERT_RETURN_ERROR (PcdStatus);
-}
-
-
-VOID
 DebugDumpCmos (
   VOID
   )
@@ -660,8 +631,9 @@ InitializePlatform (
   //
   mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
 
+  SetupVariables ();
+
   if (mBootMode != BOOT_ON_S3_RESUME) {
-    ReserveEmuVariableNvStore ();
     PeiFvInitialization ();
     MemMapInitialization ();
     NoexecDxeInitialization ();
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 18f42c3f0e..dfbdb8b75d 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -1,7 +1,7 @@
 /** @file
   Platform PEI module include file.
 
-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -95,6 +95,11 @@ XenPublishRamRegions (
   VOID
   );
 
+VOID
+SetupVariables (
+  VOID
+  );
+
 extern EFI_BOOT_MODE mBootMode;
 
 extern BOOLEAN mS3Supported;
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 53c6dd445a..0eaf27e553 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -2,7 +2,7 @@
 #  Platform PEI driver
 #
 #  This module provides platform specific function to detect boot mode.
-#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -34,6 +34,7 @@
   Fv.c
   MemDetect.c
   Platform.c
+  Vars.c
   Xen.c
 
 [Packages]
@@ -83,9 +84,16 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
diff --git a/OvmfPkg/PlatformPei/Vars.c b/OvmfPkg/PlatformPei/Vars.c
new file mode 100644
index 0000000000..563f847a55
--- /dev/null
+++ b/OvmfPkg/PlatformPei/Vars.c
@@ -0,0 +1,122 @@
+/**@file
+  Platform PEI Variable Store Initialization
+
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+
+  This program and the accompanying materials are licensed and made
+  available under the terms and conditions of the BSD License which
+  accompanies this distribution. The full text of the license may be
+  found at http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
+  BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
+  EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiPei.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+
+#include "Platform.h"
+
+#define OVMF_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
+#define OVMF_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
+#define OVMF_FV_HEADER_LENGTH OFFSET_OF (FVB_FV_HDR_AND_VARS_TEMPLATE, VarHdr)
+
+
+VOID
+ReserveEmuVariableNvStore (
+  )
+{
+  EFI_PHYSICAL_ADDRESS VariableStore;
+  UINT32               Offset;
+  RETURN_STATUS        PcdStatus;
+
+  //
+  // 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 =
+    (EFI_PHYSICAL_ADDRESS)(UINTN)
+      AllocateAlignedRuntimePages (
+        EFI_SIZE_TO_PAGES (OVMF_FVB_SIZE),
+        PcdGet32 (PcdFlashNvStorageFtwSpareSize)
+        );
+  ASSERT (VariableStore != 0);
+  ASSERT ((VariableStore + OVMF_FVB_SIZE) <= MAX_ADDRESS);
+  DEBUG ((EFI_D_INFO,
+          "Reserved variable store memory: 0x%lX; size: %dkb\n",
+          VariableStore,
+          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
+        ));
+  PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  //
+  // Initialize the main FV header and variable store header
+  //
+  PcdStatus = PcdSet64S (
+                PcdFlashNvStorageVariableBase64,
+                VariableStore);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  //
+  // Initialize the Fault Tolerant Write data area
+  //
+  Offset = PcdGet32 (PcdVariableStoreSize);
+  PcdStatus = PcdSet32S (
+                PcdFlashNvStorageFtwWorkingBase,
+                VariableStore + Offset);
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  //
+  // Initialize the Fault Tolerant Write spare block
+  //
+  Offset = FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  PcdStatus = PcdSet32S (
+                PcdFlashNvStorageFtwSpareBase,
+                VariableStore + Offset);
+  ASSERT_RETURN_ERROR (PcdStatus);
+}
+
+
+VOID
+SetupVariables (
+  VOID
+  )
+{
+  RETURN_STATUS        PcdStatus;
+
+  if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {
+    //
+    // If flash is enabled, then set the variable PCD to point
+    // directly at flash.
+    //
+    PcdStatus = PcdSet64S (
+      PcdFlashNvStorageVariableBase64,
+      (UINTN) PcdGet32 (PcdOvmfFlashNvStorageVariableBase)
+      );
+    ASSERT_RETURN_ERROR (PcdStatus);
+    PcdStatus = PcdSet32S (
+      PcdFlashNvStorageFtwWorkingBase,
+      PcdGet32 (PcdOvmfFlashNvStorageFtwWorkingBase)
+      );
+    ASSERT_RETURN_ERROR (PcdStatus);
+    PcdStatus = PcdSet32S (
+      PcdFlashNvStorageFtwSpareBase,
+      PcdGet32 (PcdOvmfFlashNvStorageFtwSpareBase)
+      );
+    ASSERT_RETURN_ERROR (PcdStatus);
+  } else {
+    //
+    // If flash is not enabled, then allocate a buffer and initialize
+    // it if necessary for variable operations.
+    //
+    ReserveEmuVariableNvStore ();
+  }
+}
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/12] OvmfPkg PlatformPei: Initialize memory based variable store buffer
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (8 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 09/12] OvmfPkg PlatformPei: Set flash variable PCDs Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 11/12] OvmfPkg: Enable PEI variable access Jordan Justen
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

This is similar to the initialization in
OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
 OvmfPkg/PlatformPei/Vars.c          | 161 ++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 0eaf27e553..7a7f5c23b0 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -50,6 +50,7 @@
 
 [LibraryClasses]
   BaseLib
+  BaseMemoryLib
   DebugLib
   HobLib
   IoLib
diff --git a/OvmfPkg/PlatformPei/Vars.c b/OvmfPkg/PlatformPei/Vars.c
index 563f847a55..452b06e399 100644
--- a/OvmfPkg/PlatformPei/Vars.c
+++ b/OvmfPkg/PlatformPei/Vars.c
@@ -15,18 +15,172 @@
 **/
 
 #include <PiPei.h>
+#include <Guid/VariableFormat.h>
+#include <Guid/SystemNvDataGuid.h>
 #include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 
 #include "Platform.h"
 
+typedef struct {
+
+  EFI_FIRMWARE_VOLUME_HEADER FvHdr;
+  EFI_FV_BLOCK_MAP_ENTRY     EndBlockMap;
+  VARIABLE_STORE_HEADER      VarHdr;
+
+} FVB_FV_HDR_AND_VARS_TEMPLATE;
+
 #define OVMF_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
 #define OVMF_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
 #define OVMF_FV_HEADER_LENGTH OFFSET_OF (FVB_FV_HDR_AND_VARS_TEMPLATE, VarHdr)
 
 
+/**
+  Check the integrity of firmware volume header.
+
+  @param[in] FwVolHeader - A pointer to a firmware volume header
+
+  @retval  EFI_SUCCESS   - The firmware volume is consistent
+  @retval  EFI_NOT_FOUND - The firmware volume has been corrupted.
+
+**/
+STATIC EFI_STATUS
+ValidateFvHeader (
+  IN EFI_FIRMWARE_VOLUME_HEADER   *FwVolHeader
+  )
+{
+  UINT16  Checksum;
+
+  //
+  // Verify the header revision, header signature, length
+  // Length of FvBlock cannot be 2**64-1
+  // HeaderLength cannot be an odd number
+  //
+  if ((FwVolHeader->Revision != EFI_FVH_REVISION) ||
+      (FwVolHeader->Signature != EFI_FVH_SIGNATURE) ||
+      (FwVolHeader->FvLength != OVMF_FVB_SIZE) ||
+      (FwVolHeader->HeaderLength != OVMF_FV_HEADER_LENGTH)
+      ) {
+    DEBUG ((EFI_D_INFO, "EMU Variable FVB: Basic FV headers were invalid\n"));
+    return EFI_NOT_FOUND;
+  }
+  //
+  // Verify the header checksum
+  //
+  Checksum = CalculateSum16((VOID*) FwVolHeader, FwVolHeader->HeaderLength);
+
+  if (Checksum != 0) {
+    DEBUG ((EFI_D_INFO, "EMU Variable FVB: FV checksum was invalid\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Initializes the FV Header and Variable Store Header
+  to support variable operations.
+
+  @param[in]  Ptr - Location to initialize the headers
+
+**/
+STATIC VOID
+InitializeFvAndVariableStoreHeaders (
+  IN  VOID   *Ptr
+  )
+{
+  //
+  // Templates for authenticated variable FV header
+  //
+  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 },
+
+      // EFI_GUID                  FileSystemGuid;
+      EFI_SYSTEM_NV_DATA_FV_GUID,
+
+      // UINT64                    FvLength;
+      OVMF_FVB_SIZE,
+
+      // UINT32                    Signature;
+      EFI_FVH_SIGNATURE,
+
+      // EFI_FVB_ATTRIBUTES_2      Attributes;
+      0x4feff,
+
+      // UINT16                    HeaderLength;
+      OVMF_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;
+          OVMF_FVB_BLOCK_SIZE  // UINT32 Length;
+        }
+      }
+    },
+    // EFI_FV_BLOCK_MAP_ENTRY     EndBlockMap;
+    { 0, 0 }, // End of block map
+    { // VARIABLE_STORE_HEADER      VarHdr;
+        // EFI_GUID  Signature;     // need authenticated variables for secure boot
+        EFI_AUTHENTICATED_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
+    }
+  };
+
+  EFI_FIRMWARE_VOLUME_HEADER  *Fv;
+
+  //
+  // Copy the template structure into the location
+  //
+  CopyMem (
+    Ptr,
+    (VOID*) &FvAndAuthenticatedVarTemplate,
+    sizeof (FvAndAuthenticatedVarTemplate)
+    );
+
+  //
+  // Update the checksum for the FV header
+  //
+  Fv = (EFI_FIRMWARE_VOLUME_HEADER*) Ptr;
+  Fv->Checksum = CalculateCheckSum16 (Ptr, Fv->HeaderLength);
+}
+
+
 VOID
 ReserveEmuVariableNvStore (
   )
@@ -57,6 +211,13 @@ ReserveEmuVariableNvStore (
   PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
   ASSERT_RETURN_ERROR (PcdStatus);
 
+  VOID *Ptr = (VOID*)(UINTN) VariableStore;
+
+  if (EFI_ERROR (ValidateFvHeader (Ptr))) {
+    SetMem (Ptr, OVMF_FVB_SIZE, 0xff);
+    InitializeFvAndVariableStoreHeaders (Ptr);
+  }
+
   //
   // Initialize the main FV header and variable store header
   //
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 11/12] OvmfPkg: Enable PEI variable access
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (9 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 10/12] OvmfPkg PlatformPei: Initialize memory based variable store buffer Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27  8:05 ` [PATCH 12/12] OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI Jordan Justen
  2017-03-27 18:03 ` [PATCH 00/12] OvmfPkg: Enable variable access " Laszlo Ersek
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

We have to make sure PlatformPei runs early to set flash variable
PCDs, so we add it to the APIORI list.

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
 OvmfPkg/OvmfPkgIa32.fdf    | 5 ++++-
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
 OvmfPkg/OvmfPkgIa32X64.fdf | 5 ++++-
 OvmfPkg/OvmfPkgX64.dsc     | 2 ++
 OvmfPkg/OvmfPkgX64.fdf     | 5 ++++-
 6 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index ee1af50a14..d3d0cb0af0 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -558,6 +558,8 @@
   OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
   #
   # DXE Phase modules
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 09c165882c..45d303d57c 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -148,6 +148,7 @@ READ_LOCK_STATUS   = TRUE
 
 APRIORI PEI {
   INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF  OvmfPkg/PlatformPei/PlatformPei.inf
 }
 
 #
@@ -164,6 +165,8 @@ INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
 INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+INF  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
 ################################################################################
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a429ceff6b..011f97771f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -566,6 +566,8 @@
   OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
 [Components.X64]
   #
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 5233314139..79928fb44b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -148,6 +148,7 @@ READ_LOCK_STATUS   = TRUE
 
 APRIORI PEI {
   INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF  OvmfPkg/PlatformPei/PlatformPei.inf
 }
 
 #
@@ -164,6 +165,8 @@ INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
 INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+INF  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
 ################################################################################
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 21f52a0976..c1e02f65a4 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -565,6 +565,8 @@
   OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
   #
   # DXE Phase modules
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 36150101e7..602f4bd710 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -148,6 +148,7 @@ READ_LOCK_STATUS   = TRUE
 
 APRIORI PEI {
   INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF  OvmfPkg/PlatformPei/PlatformPei.inf
 }
 
 #
@@ -164,6 +165,8 @@ INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
 INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+INF  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
 ################################################################################
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 12/12] OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (10 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 11/12] OvmfPkg: Enable PEI variable access Jordan Justen
@ 2017-03-27  8:05 ` Jordan Justen
  2017-03-27 18:03 ` [PATCH 00/12] OvmfPkg: Enable variable access " Laszlo Ersek
  12 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-27  8:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek

This is now done in PEI since commit "OvmfPkg PlatformPei: Set flash
variable PCDs".

Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 .../FvbServicesRuntimeDxe.inf                      |  4 ----
 .../FvbServicesSmm.inf                             |  4 ----
 .../FwBlockService.c                               | 28 ++++------------------
 3 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
index c0dda75bf7..8656d185f6 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
@@ -78,11 +78,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize
 
 [Pcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
 
 [FeaturePcd]
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
index ba2d3679a4..c9cbc34901 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
@@ -77,11 +77,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize
 
 [Pcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
 
 [FeaturePcd]
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index ff27c1100c..857b1e3878 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -965,9 +965,9 @@ FvbInitialize (
   EFI_PHYSICAL_ADDRESS                BaseAddress;
   UINTN                               Length;
   UINTN                               NumOfBlocks;
-  RETURN_STATUS                       PcdStatus;
 
-  if (EFI_ERROR (QemuFlashInitialize ())) {
+  if (!PcdGetBool (PcdOvmfFlashVariablesEnable)) {
+    ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
     //
     // Return an error so image will be unloaded
     //
@@ -976,6 +976,9 @@ FvbInitialize (
     return EFI_WRITE_PROTECTED;
   }
 
+  Status = QemuFlashInitialize ();
+  ASSERT_RETURN_ERROR (Status);
+
   //
   // Allocate runtime services data for global variable, which contains
   // the private data of all firmware volume block instances
@@ -1093,25 +1096,6 @@ FvbInitialize (
 
   MarkMemoryRangeForRuntimeAccess (BaseAddress, Length);
 
-  //
-  // Set several PCD values to point to flash
-  //
-  PcdStatus = PcdSet64S (
-    PcdFlashNvStorageVariableBase64,
-    (UINTN) PcdGet32 (PcdOvmfFlashNvStorageVariableBase)
-    );
-  ASSERT_RETURN_ERROR (PcdStatus);
-  PcdStatus = PcdSet32S (
-    PcdFlashNvStorageFtwWorkingBase,
-    PcdGet32 (PcdOvmfFlashNvStorageFtwWorkingBase)
-    );
-  ASSERT_RETURN_ERROR (PcdStatus);
-  PcdStatus = PcdSet32S (
-    PcdFlashNvStorageFtwSpareBase,
-    PcdGet32 (PcdOvmfFlashNvStorageFtwSpareBase)
-    );
-  ASSERT_RETURN_ERROR (PcdStatus);
-
   FwhInstance = (EFI_FW_VOL_INSTANCE *)
     (
       (UINTN) ((UINT8 *) FwhInstance) + FwVolHeader->HeaderLength +
@@ -1123,7 +1107,5 @@ FvbInitialize (
   //
   InstallVirtualAddressChangeHandler ();
 
-  PcdStatus = PcdSetBoolS (PcdOvmfFlashVariablesEnable, TRUE);
-  ASSERT_RETURN_ERROR (PcdStatus);
   return EFI_SUCCESS;
 }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/12] OvmfPkg: Enable variable access in PEI
  2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
                   ` (11 preceding siblings ...)
  2017-03-27  8:05 ` [PATCH 12/12] OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI Jordan Justen
@ 2017-03-27 18:03 ` Laszlo Ersek
  2017-03-27 21:47   ` Jordan Justen
  12 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2017-03-27 18:03 UTC (permalink / raw)
  To: Jordan Justen, edk2-devel

On 03/27/17 10:05, Jordan Justen wrote:
> web: https://github.com/jljusten/edk2/tree/pei-vars-v1
> 
> git: https://github.com/jljusten/edk2.git pei-vars-v1
> 
> This series moves flash detection into PEI to allow the PEI variable
> access drivers to run. If flash is writable, the PCDs are set to point
> at the flash memory. If flash is not writable, the PCDs are set to
> point at a memory buffer.

Can you please state the use case for the latter option? That is, when
the PCDs are set to point at the memory buffer.

In that case, the memory buffer is guaranteed to be empty (modulo
internal formatting that PlatformPei would do just-in-time), unless the
VM has just been rebooted within the same QEMU instance.

In other words, clients of the r/o variable2 PPI will get false results,
most of the time -- if I understand correctly. After a fresh boot (which
is most of the boots), no variable will be found in the PEI phase, even
if the \NvVars file contains it, and the variable services in DXE will
later find it.

... I'm not sure if my understanding above is correct, but if it is,
then I think this feature only contributes to the confusing nature of
the memory-emulated variables.

Thanks
Laszlo

> 
> I tested KVM with ROM and writable flash, with S3 sleep/resume. I did
> not test SMM.
> 
> Jordan Justen (10):
>   OvmfPkg/build.sh: Add support for --disable-flash switch
>   OvmfPkg QemuFlash: Make QemuFlash.* Base class safe
>   OvmfPkg QemuFlash: Make QemuFlashDetected external
>   OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library
>   OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable
>   OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable
>   OvmfPkg PlatformPei: Set flash variable PCDs
>   OvmfPkg PlatformPei: Initialize memory based variable store buffer
>   OvmfPkg: Enable PEI variable access
>   OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI
> 
> Laszlo Ersek (2):
>   OvmfPkg: resolve PcdLib for all PEIMs individually
>   OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default
> 
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c             |   4 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf           |   3 +-
>  OvmfPkg/OvmfPkgIa32.dsc                            |  27 +-
>  OvmfPkg/OvmfPkgIa32.fdf                            |   5 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                         |  27 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf                         |   5 +-
>  OvmfPkg/OvmfPkgX64.dsc                             |  27 +-
>  OvmfPkg/OvmfPkgX64.fdf                             |   5 +-
>  OvmfPkg/PlatformPei/Platform.c                     |  34 +--
>  OvmfPkg/PlatformPei/Platform.h                     |   7 +-
>  OvmfPkg/PlatformPei/PlatformPei.inf                |  11 +-
>  OvmfPkg/PlatformPei/Vars.c                         | 283 +++++++++++++++++++++
>  .../DetectFlashNullLib.c                           |  41 +++
>  .../DetectFlashNullLib.inf                         |  60 +++++
>  .../FvbServicesRuntimeDxe.inf                      |   4 -
>  .../FvbServicesSmm.inf                             |   4 -
>  .../FwBlockService.c                               |  28 +-
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c |  59 +++--
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h |  30 ++-
>  OvmfPkg/build.sh                                   |  10 +-
>  20 files changed, 530 insertions(+), 144 deletions(-)
>  create mode 100644 OvmfPkg/PlatformPei/Vars.c
>  create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
>  create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/12] OvmfPkg: Enable variable access in PEI
  2017-03-27 18:03 ` [PATCH 00/12] OvmfPkg: Enable variable access " Laszlo Ersek
@ 2017-03-27 21:47   ` Jordan Justen
  2017-03-28  9:22     ` Laszlo Ersek
  0 siblings, 1 reply; 17+ messages in thread
From: Jordan Justen @ 2017-03-27 21:47 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel

On 2017-03-27 11:03:42, Laszlo Ersek wrote:
> On 03/27/17 10:05, Jordan Justen wrote:
> > web: https://github.com/jljusten/edk2/tree/pei-vars-v1
> > 
> > git: https://github.com/jljusten/edk2.git pei-vars-v1
> > 
> > This series moves flash detection into PEI to allow the PEI variable
> > access drivers to run. If flash is writable, the PCDs are set to point
> > at the flash memory. If flash is not writable, the PCDs are set to
> > point at a memory buffer.
> 
> Can you please state the use case for the latter option? That is, when
> the PCDs are set to point at the memory buffer.
> 
> In that case, the memory buffer is guaranteed to be empty (modulo
> internal formatting that PlatformPei would do just-in-time), unless the
> VM has just been rebooted within the same QEMU instance.
> 
> In other words, clients of the r/o variable2 PPI will get false results,
> most of the time -- if I understand correctly. After a fresh boot (which
> is most of the boots), no variable will be found in the PEI phase, even
> if the \NvVars file contains it, and the variable services in DXE will
> later find it.
> 
> ... I'm not sure if my understanding above is correct, but if it is,
> then I think this feature only contributes to the confusing nature of
> the memory-emulated variables.

I think we should continue to support the memory vars, even if they
have some obvious drawbacks.

-Jordan

> 
> > 
> > I tested KVM with ROM and writable flash, with S3 sleep/resume. I did
> > not test SMM.
> > 
> > Jordan Justen (10):
> >   OvmfPkg/build.sh: Add support for --disable-flash switch
> >   OvmfPkg QemuFlash: Make QemuFlash.* Base class safe
> >   OvmfPkg QemuFlash: Make QemuFlashDetected external
> >   OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library
> >   OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable
> >   OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable
> >   OvmfPkg PlatformPei: Set flash variable PCDs
> >   OvmfPkg PlatformPei: Initialize memory based variable store buffer
> >   OvmfPkg: Enable PEI variable access
> >   OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI
> > 
> > Laszlo Ersek (2):
> >   OvmfPkg: resolve PcdLib for all PEIMs individually
> >   OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default
> > 
> >  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c             |   4 +-
> >  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf           |   3 +-
> >  OvmfPkg/OvmfPkgIa32.dsc                            |  27 +-
> >  OvmfPkg/OvmfPkgIa32.fdf                            |   5 +-
> >  OvmfPkg/OvmfPkgIa32X64.dsc                         |  27 +-
> >  OvmfPkg/OvmfPkgIa32X64.fdf                         |   5 +-
> >  OvmfPkg/OvmfPkgX64.dsc                             |  27 +-
> >  OvmfPkg/OvmfPkgX64.fdf                             |   5 +-
> >  OvmfPkg/PlatformPei/Platform.c                     |  34 +--
> >  OvmfPkg/PlatformPei/Platform.h                     |   7 +-
> >  OvmfPkg/PlatformPei/PlatformPei.inf                |  11 +-
> >  OvmfPkg/PlatformPei/Vars.c                         | 283 +++++++++++++++++++++
> >  .../DetectFlashNullLib.c                           |  41 +++
> >  .../DetectFlashNullLib.inf                         |  60 +++++
> >  .../FvbServicesRuntimeDxe.inf                      |   4 -
> >  .../FvbServicesSmm.inf                             |   4 -
> >  .../FwBlockService.c                               |  28 +-
> >  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c |  59 +++--
> >  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.h |  30 ++-
> >  OvmfPkg/build.sh                                   |  10 +-
> >  20 files changed, 530 insertions(+), 144 deletions(-)
> >  create mode 100644 OvmfPkg/PlatformPei/Vars.c
> >  create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.c
> >  create mode 100644 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/DetectFlashNullLib.inf
> > 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/12] OvmfPkg: Enable variable access in PEI
  2017-03-27 21:47   ` Jordan Justen
@ 2017-03-28  9:22     ` Laszlo Ersek
  2017-03-29  5:24       ` Jordan Justen
  0 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2017-03-28  9:22 UTC (permalink / raw)
  To: Jordan Justen, edk2-devel

On 03/27/17 23:47, Jordan Justen wrote:
> On 2017-03-27 11:03:42, Laszlo Ersek wrote:
>> On 03/27/17 10:05, Jordan Justen wrote:
>>> web: https://github.com/jljusten/edk2/tree/pei-vars-v1
>>>
>>> git: https://github.com/jljusten/edk2.git pei-vars-v1
>>>
>>> This series moves flash detection into PEI to allow the PEI variable
>>> access drivers to run. If flash is writable, the PCDs are set to
>>> point at the flash memory. If flash is not writable, the PCDs are
>>> set to point at a memory buffer.
>>
>> Can you please state the use case for the latter option? That is,
>> when the PCDs are set to point at the memory buffer.
>>
>> In that case, the memory buffer is guaranteed to be empty (modulo
>> internal formatting that PlatformPei would do just-in-time), unless
>> the VM has just been rebooted within the same QEMU instance.
>>
>> In other words, clients of the r/o variable2 PPI will get false
>> results, most of the time -- if I understand correctly. After a fresh
>> boot (which is most of the boots), no variable will be found in the
>> PEI phase, even if the \NvVars file contains it, and the variable
>> services in DXE will later find it.
>>
>> ... I'm not sure if my understanding above is correct, but if it is,
>> then I think this feature only contributes to the confusing nature of
>> the memory-emulated variables.
>
> I think we should continue to support the memory vars, even if they
> have some obvious drawbacks.

I don't understand how you can call this "continued support" when Xen
(and any other similarly pflash-less virt host scenarios) will see zero
benefit from the increased complexity. The r/o variable2 PPI will be
available to them, and it will almost always lie to them.

Is that "continued support"? Is the availability of a lying PPI better
than the absence of a functional PPI? I'm out of arguments.

Anyway, I tested the series with the SMM_REQUIRE build -- you mentioned
in the blurb that you didn't test SMM --; there is a regression.
PlatformPei fails to detect flash:

> Loading PEIM at 0x00000844B20 EntryPoint=0x00000844D40 PlatformPei.efi
> Select Item: 0x0
> FW CFG Signature: 0x554D4551
> Select Item: 0x1
> FW CFG Revision: 0x3
> QemuFwCfg interface (DMA) is supported.
> QEMU Flash: Attempting flash detection at FFE00010
> QemuFlashDetected => FD behaves as ROM
> QemuFlashDetected => No
> Platform PEIM Loaded

the SMBASE relocation succeeds:

> Loading SMM driver at 0x0007FFA8000 EntryPoint=0x0007FFA9034 PiSmmCpuDxeSmm.efi
> SMRR Base: 0x7F800000, SMRR Size: 0x800000
> PcdCpuSmmCodeAccessCheckEnable = 1
> mAddressEncMask = 0x0
> SMRAM TileSize = 0x00002000 (0x00001000, 0x00001000)
> SMRAM SaveState Buffer (0x7FF9A000, 0x0000E000)
> CPU[000]  APIC ID=0000  SMBASE=7FF92000  SaveState=7FFA1C00  Size=00000400
> CPU[001]  APIC ID=0001  SMBASE=7FF94000  SaveState=7FFA3C00  Size=00000400
> CPU[002]  APIC ID=0002  SMBASE=7FF96000  SaveState=7FFA5C00  Size=00000400
> CPU[003]  APIC ID=0003  SMBASE=7FF98000  SaveState=7FFA7C00  Size=00000400
> mXdSupported - 0x0
> One Semaphore Size    = 0x40
> Total Semaphores Size = 0x840
> InstallProtocolInterface: [gEfiSmmConfigurationProtocolGuid] 7FFC2040
> SMM IPL registered SMM Entry Point address 7FFE099D
> SmmInstallProtocolInterface: [EfiSmmCpuProtocol] 7FFC2068
> SmmInstallProtocolInterface: [EfiSmmCpuServiceProtocol] 7FFC2084
> SMM S3 SMRAM Structure = 7F47DAD8
> SMM S3 Structure = 7F800000
> SMM CPU Module exit from SMRAM with EFI_SUCCESS
> PageTable is 0!
> SMM IPL closed SMRAM window

and then FvbServicesSmm blows up due to the originally failed flash
detection:

> InstallProtocolInterface: [EfiLoadedImageProtocol] 7E9EEC10
> SmmInstallProtocolInterface: [EfiLoadedImageProtocol] 7FFDD47C
> Loading SMM driver at 0x0007FF5F000 EntryPoint=0x0007FF60034 FvbServicesSmm.efi
> ASSERT .../OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c(970): !_gPcd_FixedAtBuild_PcdSmmSmramRequire

The reason for the failed flash detection, in PlatformPei, is that
PlatformPei does not run in System Management Mode. Therefore QEMU
denies it write access to the pflash chip.

Without the series applied, the flash detection (which involves a write)
only occurs in FvbServicesSmm, which is an SMM driver; it runs in System
Management Mode, so QEMU lets it write to the pflash chip.

NB, the assertion failure (the boot regression) is just one problem. The
real problem (concerning the feature itself) is that the failed dynamic
flash detection in PlatformPei will prevent the PEI phase, in the
SMM_REQUIRE build, from reading the actual pflash variable store.

In the SMM_REQUIRE build, the real variables in the pflash store *would
be* available for reading (via the r/o variable2 PPI), but PlatformPei
falls back to memory emulation, because it cannot successfully write the
pflash. That kind of defeats the entire feature.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/12] OvmfPkg: Enable variable access in PEI
  2017-03-28  9:22     ` Laszlo Ersek
@ 2017-03-29  5:24       ` Jordan Justen
  0 siblings, 0 replies; 17+ messages in thread
From: Jordan Justen @ 2017-03-29  5:24 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel

On 2017-03-28 02:22:37, Laszlo Ersek wrote:
> 
> > InstallProtocolInterface: [EfiLoadedImageProtocol] 7E9EEC10
> > SmmInstallProtocolInterface: [EfiLoadedImageProtocol] 7FFDD47C
> > Loading SMM driver at 0x0007FF5F000 EntryPoint=0x0007FF60034 FvbServicesSmm.efi
> > ASSERT .../OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c(970): !_gPcd_FixedAtBuild_PcdSmmSmramRequire
> 
> The reason for the failed flash detection, in PlatformPei, is that
> PlatformPei does not run in System Management Mode. Therefore QEMU
> denies it write access to the pflash chip.

Ah. Yet another discrepancy from hardware. Anyway, I installed GCC
5.4, and I can now build IA32 again and reproduce the SMM issue. (GCC
6.3 is generating some unsupported relocations for me in the OVMF SEC
image.)

-Jordan


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-03-29  5:24 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-27  8:05 [PATCH 00/12] OvmfPkg: Enable variable access in PEI Jordan Justen
2017-03-27  8:05 ` [PATCH 01/12] OvmfPkg/build.sh: Add support for --disable-flash switch Jordan Justen
2017-03-27  8:05 ` [PATCH 02/12] OvmfPkg: resolve PcdLib for all PEIMs individually Jordan Justen
2017-03-27  8:05 ` [PATCH 03/12] OvmfPkg: resolve PcdLib for PEIMs to PeiPcdLib by default Jordan Justen
2017-03-27  8:05 ` [PATCH 04/12] OvmfPkg QemuFlash: Make QemuFlash.* Base class safe Jordan Justen
2017-03-27  8:05 ` [PATCH 05/12] OvmfPkg QemuFlash: Make QemuFlashDetected external Jordan Justen
2017-03-27  8:05 ` [PATCH 06/12] OvmfPkg QemuFlash: Add DetectFlashBaseLib.inf 'NULL' library Jordan Justen
2017-03-27  8:05 ` [PATCH 07/12] OvmfPkg PlatformPei: Detect and set PcdOvmfFlashVariablesEnable Jordan Justen
2017-03-27  8:05 ` [PATCH 08/12] OvmfPkg/EmuVariableFvbRuntimeDxe: Use PcdOvmfFlashVariablesEnable Jordan Justen
2017-03-27  8:05 ` [PATCH 09/12] OvmfPkg PlatformPei: Set flash variable PCDs Jordan Justen
2017-03-27  8:05 ` [PATCH 10/12] OvmfPkg PlatformPei: Initialize memory based variable store buffer Jordan Justen
2017-03-27  8:05 ` [PATCH 11/12] OvmfPkg: Enable PEI variable access Jordan Justen
2017-03-27  8:05 ` [PATCH 12/12] OvmfPkg QemuFlashFvbServicesRuntimeDxe: Cleanup init now done in PEI Jordan Justen
2017-03-27 18:03 ` [PATCH 00/12] OvmfPkg: Enable variable access " Laszlo Ersek
2017-03-27 21:47   ` Jordan Justen
2017-03-28  9:22     ` Laszlo Ersek
2017-03-29  5:24       ` Jordan Justen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox