[PATCH v2 00/11] support QEMU's "SMRAM at default SMBASE" feature

[PATCH v2 00/11] support QEMU's "SMRAM at default SMBASE" feature

[Laszlo Ersek]

Ref:        https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Repo:       https://github.com/lersek/edk2.git
Branch:     smram_at_default_smbase_bz_1512_wave_1_v2
Supersedes: <20190924113505.27272-1-lersek@redhat.com>

V1 is archived at:
- http://mid.mail-archive.com/20190924113505.27272-1-lersek@redhat.com
- https://edk2.groups.io/g/devel/message/47924

Igor's patch set, mentioned in the v1 blurb, has been merged into QEMU
meanwhile. The relevant QEMU commit is f404220e279c ("q35: implement
128K SMRAM at default SMBASE address", 2020-01-22).

In v2:
- trim the Cc list

- pick up Jiewen's R-b for patches #1 through #9, from:
  - http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503F7CBCB2@shsmsx102.ccr.corp.intel.com
  - https://edk2.groups.io/g/devel/message/48166

- add patch #10, and update patch #11, for satisfying Jiewen's condition
  on his R-b.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien@xen.org>

Thanks,
Laszlo

Laszlo Ersek (11):
  OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase
  OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro
    defs
  OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros
  OvmfPkg/PlatformPei: factor out Q35BoardVerification()
  OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton)
  OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default
    SMBASE
  OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it
    exists
  OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default
    SMBASE
  OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE
  OvmfPkg: introduce PcdCsmEnable feature flag
  OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real)

 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h           | 106 +++++++++++---------
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   |  21 +++-
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |   4 +
 OvmfPkg/OvmfPkg.dec                                     |  11 ++
 OvmfPkg/OvmfPkgIa32.dsc                                 |   4 +
 OvmfPkg/OvmfPkgIa32X64.dsc                              |   4 +
 OvmfPkg/OvmfPkgX64.dsc                                  |   4 +
 OvmfPkg/OvmfXen.dsc                                     |   3 +
 OvmfPkg/PlatformPei/AmdSev.c                            |  24 ++++-
 OvmfPkg/PlatformPei/MemDetect.c                         |  94 ++++++++++++++---
 OvmfPkg/PlatformPei/Platform.c                          |  24 +++++
 OvmfPkg/PlatformPei/Platform.h                          |   7 ++
 OvmfPkg/PlatformPei/PlatformPei.inf                     |   2 +
 OvmfPkg/SmmAccess/SmmAccess2Dxe.c                       |   7 ++
 OvmfPkg/SmmAccess/SmmAccess2Dxe.inf                     |   1 +
 OvmfPkg/SmmAccess/SmmAccessPei.c                        |   6 ++
 OvmfPkg/SmmAccess/SmmAccessPei.inf                      |   1 +
 OvmfPkg/SmmAccess/SmramInternal.c                       |  25 +++++
 OvmfPkg/SmmAccess/SmramInternal.h                       |   8 ++
 19 files changed, 285 insertions(+), 71 deletions(-)

-- 
2.19.1.3.g30247aa5d201

[PATCH v2 01/11] OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase

[Laszlo Ersek]

For supporting VCPU hotplug with SMM enabled/required, QEMU offers the
(dynamically detectable) feature called "SMRAM at default SMBASE". When
the feature is enabled, the firmware can lock down the 128 KB range
starting at the default SMBASE; that is, the [0x3_0000, 0x4_FFFF]
interval. The goal is to shield the very first SMI handler of the
hotplugged VCPU from OS influence.

Multiple modules in OVMF will have to inter-operate for locking down this
range. Introduce a dynamic PCD that will reflect the feature (to be
negotiated by PlatformPei), for coordination between drivers.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/OvmfPkg.dec        | 6 ++++++
 OvmfPkg/OvmfPkgIa32.dsc    | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc     | 1 +
 4 files changed, 9 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index d5fee805ef4a..57034c784f88 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -257,6 +257,12 @@ [PcdsDynamic, PcdsDynamicEx]
   #  This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below).
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8|UINT16|0x20
 
+  ## Set to TRUE by PlatformPei if the Q35 board supports the "SMRAM at default
+  #  SMBASE" feature.
+  #
+  #  This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below).
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE|BOOLEAN|0x34
+
 [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 4568b78cadf1..43ff51933609 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -565,6 +565,7 @@ [PcdsDynamicDefault]
 
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 152b5d067116..5b3e217e9220 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -577,6 +577,7 @@ [PcdsDynamicDefault]
 
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 4bfad441bd9f..a378d0a449aa 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -576,6 +576,7 @@ [PcdsDynamicDefault]
 
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 02/11] OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro defs

[Laszlo Ersek]

In a subsequent patch, we'll introduce new DRAM controller macros in
"Q35MchIch9.h". Their names are too long for the currently available
vertical whitespace, so increase the latter first.

There is no functional change in this patch ("git show -b" displays
nothing).

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h | 100 ++++++++++----------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index 2ac16f19c62e..80379c223a1c 100644
--- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
+++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
@@ -27,56 +27,56 @@
 //
 #define DRAMC_REGISTER_Q35(Offset) PCI_LIB_ADDRESS (0, 0, 0, (Offset))
 
-#define MCH_EXT_TSEG_MB       0x50
-#define MCH_EXT_TSEG_MB_QUERY   0xFFFF
-
-#define MCH_GGC               0x52
-#define MCH_GGC_IVD             BIT1
-
-#define MCH_PCIEXBAR_LOW      0x60
-#define MCH_PCIEXBAR_LOWMASK    0x0FFFFFFF
-#define MCH_PCIEXBAR_BUS_FF     0
-#define MCH_PCIEXBAR_EN         BIT0
-
-#define MCH_PCIEXBAR_HIGH     0x64
-#define MCH_PCIEXBAR_HIGHMASK   0xFFFFFFF0
-
-#define MCH_PAM0              0x90
-#define MCH_PAM1              0x91
-#define MCH_PAM2              0x92
-#define MCH_PAM3              0x93
-#define MCH_PAM4              0x94
-#define MCH_PAM5              0x95
-#define MCH_PAM6              0x96
-
-#define MCH_SMRAM             0x9D
-#define MCH_SMRAM_D_LCK         BIT4
-#define MCH_SMRAM_G_SMRAME      BIT3
-
-#define MCH_ESMRAMC           0x9E
-#define MCH_ESMRAMC_H_SMRAME    BIT7
-#define MCH_ESMRAMC_E_SMERR     BIT6
-#define MCH_ESMRAMC_SM_CACHE    BIT5
-#define MCH_ESMRAMC_SM_L1       BIT4
-#define MCH_ESMRAMC_SM_L2       BIT3
-#define MCH_ESMRAMC_TSEG_EXT    (BIT2 | BIT1)
-#define MCH_ESMRAMC_TSEG_8MB    BIT2
-#define MCH_ESMRAMC_TSEG_2MB    BIT1
-#define MCH_ESMRAMC_TSEG_1MB    0
-#define MCH_ESMRAMC_TSEG_MASK   (BIT2 | BIT1)
-#define MCH_ESMRAMC_T_EN        BIT0
-
-#define MCH_GBSM              0xA4
-#define MCH_GBSM_MB_SHIFT       20
-
-#define MCH_BGSM              0xA8
-#define MCH_BGSM_MB_SHIFT       20
-
-#define MCH_TSEGMB            0xAC
-#define MCH_TSEGMB_MB_SHIFT     20
-
-#define MCH_TOLUD             0xB0
-#define MCH_TOLUD_MB_SHIFT      4
+#define MCH_EXT_TSEG_MB           0x50
+#define MCH_EXT_TSEG_MB_QUERY       0xFFFF
+
+#define MCH_GGC                   0x52
+#define MCH_GGC_IVD                 BIT1
+
+#define MCH_PCIEXBAR_LOW          0x60
+#define MCH_PCIEXBAR_LOWMASK        0x0FFFFFFF
+#define MCH_PCIEXBAR_BUS_FF         0
+#define MCH_PCIEXBAR_EN             BIT0
+
+#define MCH_PCIEXBAR_HIGH         0x64
+#define MCH_PCIEXBAR_HIGHMASK       0xFFFFFFF0
+
+#define MCH_PAM0                  0x90
+#define MCH_PAM1                  0x91
+#define MCH_PAM2                  0x92
+#define MCH_PAM3                  0x93
+#define MCH_PAM4                  0x94
+#define MCH_PAM5                  0x95
+#define MCH_PAM6                  0x96
+
+#define MCH_SMRAM                 0x9D
+#define MCH_SMRAM_D_LCK             BIT4
+#define MCH_SMRAM_G_SMRAME          BIT3
+
+#define MCH_ESMRAMC               0x9E
+#define MCH_ESMRAMC_H_SMRAME        BIT7
+#define MCH_ESMRAMC_E_SMERR         BIT6
+#define MCH_ESMRAMC_SM_CACHE        BIT5
+#define MCH_ESMRAMC_SM_L1           BIT4
+#define MCH_ESMRAMC_SM_L2           BIT3
+#define MCH_ESMRAMC_TSEG_EXT        (BIT2 | BIT1)
+#define MCH_ESMRAMC_TSEG_8MB        BIT2
+#define MCH_ESMRAMC_TSEG_2MB        BIT1
+#define MCH_ESMRAMC_TSEG_1MB        0
+#define MCH_ESMRAMC_TSEG_MASK       (BIT2 | BIT1)
+#define MCH_ESMRAMC_T_EN            BIT0
+
+#define MCH_GBSM                  0xA4
+#define MCH_GBSM_MB_SHIFT           20
+
+#define MCH_BGSM                  0xA8
+#define MCH_BGSM_MB_SHIFT           20
+
+#define MCH_TSEGMB                0xAC
+#define MCH_TSEGMB_MB_SHIFT         20
+
+#define MCH_TOLUD                 0xB0
+#define MCH_TOLUD_MB_SHIFT          4
 
 //
 // B/D/F/Type: 0/0x1f/0/PCI
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 03/11] OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros

[Laszlo Ersek]

In Intel datasheet 316966-002 (the "q35 spec"), Table 5-1 "DRAM Controller
Register Address Map (D0:F0)" leaves the byte register at config space
offset 0x9C unused.

On QEMU's Q35 board, for detecting the "SMRAM at default SMBASE" feature,
firmware is expected to write MCH_DEFAULT_SMBASE_QUERY (0xFF) to offset
MCH_DEFAULT_SMBASE_CTL (0x9C), and read back the register. If the value is
MCH_DEFAULT_SMBASE_IN_RAM (0x01), then the feature is available, and the
range mentioned below is open (accessible to code running outside of SMM).

Then, once firmware writes MCH_DEFAULT_SMBASE_LCK (0x02) to the register,
the MCH_DEFAULT_SMBASE_SIZE (128KB) range at 0x3_0000 (SMM_DEFAULT_SMBASE)
gets closed and locked down, and the register becomes read-only. The area
is reopened, and the register becomes read/write, at platform reset.

Add the above-listed macros to "Q35MchIch9.h".

(There are some other unused offsets in Table 5-1; for example we had
scavenged 0x50 for implementing the extended TSEG feature. 0x9C is the
first byte-wide register standing in isolation after 0x50.)

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index 80379c223a1c..cb705fee92ca 100644
--- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
+++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
@@ -49,6 +49,12 @@
 #define MCH_PAM5                  0x95
 #define MCH_PAM6                  0x96
 
+#define MCH_DEFAULT_SMBASE_CTL    0x9C
+#define MCH_DEFAULT_SMBASE_QUERY    0xFF
+#define MCH_DEFAULT_SMBASE_IN_RAM   0x01
+#define MCH_DEFAULT_SMBASE_LCK      0x02
+#define MCH_DEFAULT_SMBASE_SIZE     SIZE_128KB
+
 #define MCH_SMRAM                 0x9D
 #define MCH_SMRAM_D_LCK             BIT4
 #define MCH_SMRAM_G_SMRAME          BIT3
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 04/11] OvmfPkg/PlatformPei: factor out Q35BoardVerification()

[Laszlo Ersek]

Before adding another SMM-related, and therefore Q35-only, dynamically
detectable feature, extract the current board type check from
Q35TsegMbytesInitialization() to a standalone function.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/PlatformPei/MemDetect.c | 13 +----------
 OvmfPkg/PlatformPei/Platform.c  | 23 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index d451989f31c9..58b171fba1c8 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -53,18 +53,7 @@ Q35TsegMbytesInitialization (
   UINT16        ExtendedTsegMbytes;
   RETURN_STATUS PcdStatus;
 
-  if (mHostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) {
-    DEBUG ((
-      DEBUG_ERROR,
-      "%a: no TSEG (SMRAM) on host bridge DID=0x%04x; "
-      "only DID=0x%04x (Q35) is supported\n",
-      __FUNCTION__,
-      mHostBridgeDevId,
-      INTEL_Q35_MCH_DEVICE_ID
-      ));
-    ASSERT (FALSE);
-    CpuDeadLoop ();
-  }
+  ASSERT (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID);
 
   //
   // Check if QEMU offers an extended TSEG.
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index e5e8581752b5..510d6d7477ec 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -566,6 +566,28 @@ S3Verification (
 }
 
 
+VOID
+Q35BoardVerification (
+  VOID
+  )
+{
+  if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
+    return;
+  }
+
+  DEBUG ((
+    DEBUG_ERROR,
+    "%a: no TSEG (SMRAM) on host bridge DID=0x%04x; "
+    "only DID=0x%04x (Q35) is supported\n",
+    __FUNCTION__,
+    mHostBridgeDevId,
+    INTEL_Q35_MCH_DEVICE_ID
+    ));
+  ASSERT (FALSE);
+  CpuDeadLoop ();
+}
+
+
 /**
   Fetch the boot CPU count and the possible CPU count from QEMU, and expose
   them to UefiCpuPkg modules. Set the mMaxCpuCount variable.
@@ -768,6 +790,7 @@ InitializePlatform (
   MaxCpuCountInitialization ();
 
   if (FeaturePcdGet (PcdSmmSmramRequire)) {
+    Q35BoardVerification ();
     Q35TsegMbytesInitialization ();
   }
 
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 05/11] OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton)

[Laszlo Ersek]

Introduce the Q35SmramAtDefaultSmbaseInitialization() function for
detecting the "SMRAM at default SMBASE" feature.

For now, the function is only a skeleton, so that we can gradually build
upon the result while the result is hard-coded as FALSE. The actual
detection will occur in a later patch.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/PlatformPei/Platform.h      |  7 +++++++
 OvmfPkg/PlatformPei/MemDetect.c     | 18 ++++++++++++++++++
 OvmfPkg/PlatformPei/Platform.c      |  1 +
 4 files changed, 27 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 30eaebdfae63..25229618ed13 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -84,6 +84,7 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 2f3cebcd3a6a..43f20f067f22 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -52,6 +52,11 @@ Q35TsegMbytesInitialization (
   VOID
   );
 
+VOID
+Q35SmramAtDefaultSmbaseInitialization (
+  VOID
+  );
+
 EFI_STATUS
 PublishPeiMemory (
   VOID
@@ -119,6 +124,8 @@ extern UINT32 mMaxCpuCount;
 
 extern UINT16 mHostBridgeDevId;
 
+extern BOOLEAN mQ35SmramAtDefaultSmbase;
+
 extern UINT32 mQemuUc32Base;
 
 #endif // _PLATFORM_PEI_H_INCLUDED_
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 58b171fba1c8..2bc1c46dffc2 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -43,6 +43,8 @@ STATIC UINT32 mS3AcpiReservedMemorySize;
 
 STATIC UINT16 mQ35TsegMbytes;
 
+BOOLEAN mQ35SmramAtDefaultSmbase;
+
 UINT32 mQemuUc32Base;
 
 VOID
@@ -90,6 +92,22 @@ Q35TsegMbytesInitialization (
 }
 
 
+VOID
+Q35SmramAtDefaultSmbaseInitialization (
+  VOID
+  )
+{
+  RETURN_STATUS PcdStatus;
+
+  ASSERT (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID);
+
+  mQ35SmramAtDefaultSmbase = FALSE;
+  PcdStatus = PcdSetBoolS (PcdQ35SmramAtDefaultSmbase,
+                mQ35SmramAtDefaultSmbase);
+  ASSERT_RETURN_ERROR (PcdStatus);
+}
+
+
 VOID
 QemuUc32BaseInitialization (
   VOID
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 510d6d7477ec..473af102783a 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -792,6 +792,7 @@ InitializePlatform (
   if (FeaturePcdGet (PcdSmmSmramRequire)) {
     Q35BoardVerification ();
     Q35TsegMbytesInitialization ();
+    Q35SmramAtDefaultSmbaseInitialization ();
   }
 
   PublishPeiMemory ();
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 06/11] OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default SMBASE

[Laszlo Ersek]

The permanent PEI RAM that is published on the normal boot path starts
strictly above MEMFD_BASE_ADDRESS (8 MB -- see the FDF files), regardless
of whether PEI decompression will be necessary on S3 resume due to
SMM_REQUIRE. Therefore the normal boot permanent PEI RAM never overlaps
with the SMRAM at the default SMBASE (192 KB).

The S3 resume permanent PEI RAM is strictly above the normal boot one.
Therefore the no-overlap statement holds true on the S3 resume path as
well.

Assert the no-overlap condition commonly for both boot paths in
PublishPeiMemory().

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/PlatformPei/MemDetect.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 2bc1c46dffc2..4879ee87b797 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -17,6 +17,7 @@ Module Name:
 #include <IndustryStandard/I440FxPiix4.h>
 #include <IndustryStandard/Q35MchIch9.h>
 #include <PiPei.h>
+#include <Register/Intel/SmramSaveStateMap.h>
 
 //
 // The Library classes this module consumes
@@ -626,6 +627,15 @@ PublishPeiMemory (
     }
   }
 
+  //
+  // MEMFD_BASE_ADDRESS separates the SMRAM at the default SMBASE from the
+  // normal boot permanent PEI RAM. Regarding the S3 boot path, the S3
+  // permanent PEI RAM is located even higher.
+  //
+  if (FeaturePcdGet (PcdSmmSmramRequire) && mQ35SmramAtDefaultSmbase) {
+    ASSERT (SMM_DEFAULT_SMBASE + MCH_DEFAULT_SMBASE_SIZE <= MemoryBase);
+  }
+
   //
   // Publish this memory to the PEI Core
   //
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 07/11] OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it exists

[Laszlo Ersek]

The 128KB SMRAM at the default SMBASE will be used for protecting the
initial SMI handler for hot-plugged VCPUs. After platform reset, the SMRAM
in question is open (and looks just like RAM). When BDS signals
EFI_DXE_MM_READY_TO_LOCK_PROTOCOL (and so TSEG is locked down), we're
going to lock the SMRAM at the default SMBASE too.

For this, we have to reserve said SMRAM area as early as possible, from
components in PEI, DXE, and OS runtime.

* QemuInitializeRam() currently produces a single resource descriptor HOB,
  for exposing the system RAM available under 1GB. This occurs during both
  normal boot and S3 resume identically (the latter only for the sake of
  CpuMpPei borrowing low RAM for the AP startup vector).

  But, the SMRAM at the default SMBASE falls in the middle of the current
  system RAM HOB. Split the HOB, and cover the SMRAM with a reserved
  memory HOB in the middle. CpuMpPei (via MpInitLib) skips reserved memory
  HOBs.

* InitializeRamRegions() is responsible for producing memory allocation
  HOBs, carving out parts of the resource descriptor HOBs produced in
  QemuInitializeRam(). Allocate the above-introduced reserved memory
  region in full, similarly to how we treat TSEG, so that DXE and the OS
  avoid the locked SMRAM (black hole) in this area.

  (Note that these allocations only occur on the normal boot path, as they
  matter for the UEFI memory map, which cannot be changed during S3
  resume.)

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/PlatformPei/MemDetect.c | 37 ++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 4879ee87b797..8fdc9c2ed7c9 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -646,6 +646,28 @@ PublishPeiMemory (
 }
 
 
+STATIC
+VOID
+QemuInitializeRamBelow1gb (
+  VOID
+  )
+{
+  if (FeaturePcdGet (PcdSmmSmramRequire) && mQ35SmramAtDefaultSmbase) {
+    AddMemoryRangeHob (0, SMM_DEFAULT_SMBASE);
+    AddReservedMemoryBaseSizeHob (SMM_DEFAULT_SMBASE, MCH_DEFAULT_SMBASE_SIZE,
+      TRUE /* Cacheable */);
+    STATIC_ASSERT (
+      SMM_DEFAULT_SMBASE + MCH_DEFAULT_SMBASE_SIZE < BASE_512KB + BASE_128KB,
+      "end of SMRAM at default SMBASE ends at, or exceeds, 640KB"
+      );
+    AddMemoryRangeHob (SMM_DEFAULT_SMBASE + MCH_DEFAULT_SMBASE_SIZE,
+      BASE_512KB + BASE_128KB);
+  } else {
+    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
+  }
+}
+
+
 /**
   Peform Memory Detection for QEMU / KVM
 
@@ -690,12 +712,12 @@ QemuInitializeRam (
     // allocation HOBs, and to honor preexistent memory allocation HOBs when
     // looking for an area to borrow.
     //
-    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
+    QemuInitializeRamBelow1gb ();
   } else {
     //
     // Create memory HOBs
     //
-    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
+    QemuInitializeRamBelow1gb ();
 
     if (FeaturePcdGet (PcdSmmSmramRequire)) {
       UINT32 TsegSize;
@@ -861,6 +883,17 @@ InitializeRamRegions (
         TsegSize,
         EfiReservedMemoryType
         );
+      //
+      // Similarly, allocate away the (already reserved) SMRAM at the default
+      // SMBASE, if it exists.
+      //
+      if (mQ35SmramAtDefaultSmbase) {
+        BuildMemoryAllocationHob (
+          SMM_DEFAULT_SMBASE,
+          MCH_DEFAULT_SMBASE_SIZE,
+          EfiReservedMemoryType
+          );
+      }
     }
   }
 }
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 08/11] OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default SMBASE

[Laszlo Ersek]

When OVMF runs in a SEV guest, the initial SMM Save State Map is

(1) allocated as EfiBootServicesData type memory in OvmfPkg/PlatformPei,
    function AmdSevInitialize(), for preventing unintended information
    sharing with the hypervisor;

(2) decrypted in AmdSevDxe;

(3) re-encrypted in OvmfPkg/Library/SmmCpuFeaturesLib, function
    SmmCpuFeaturesSmmRelocationComplete(), which is called by
    PiSmmCpuDxeSmm right after initial SMBASE relocation;

(4) released to DXE at the same location.

The SMRAM at the default SMBASE is a superset of the initial Save State
Map. The reserved memory allocation in InitializeRamRegions(), from the
previous patch, must override the allocating and freeing in (1) and (4),
respectively. (Note: the decrypting and re-encrypting in (2) and (3) are
unaffected.)

In AmdSevInitialize(), only assert the containment of the initial Save
State Map, in the larger area already allocated by InitializeRamRegions().

In SmmCpuFeaturesSmmRelocationComplete(), preserve the allocation of the
initial Save State Map into OS runtime, as part of the allocation done by
InitializeRamRegions(). Only assert containment.

These changes only affect the normal boot path (the UEFI memory map is
untouched during S3 resume).

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |  4 ++++
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   | 21 +++++++++++++++--
 OvmfPkg/PlatformPei/AmdSev.c                            | 24 ++++++++++++++++----
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index dd316f2b1bd8..97a10afb6e27 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -30,5 +30,9 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   MemEncryptSevLib
+  PcdLib
   SmmServicesTableLib
   UefiBootServicesTableLib
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 0bfdeda78d33..7ef7ed98342e 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -6,14 +6,17 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
+#include <IndustryStandard/Q35MchIch9.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemEncryptSevLib.h>
+#include <Library/PcdLib.h>
 #include <Library/SmmCpuFeaturesLib.h>
 #include <Library/SmmServicesTableLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <PiSmm.h>
+#include <Register/Intel/SmramSaveStateMap.h>
 #include <Register/QemuSmramSaveStateMap.h>
 
 //
@@ -215,8 +218,22 @@ SmmCpuFeaturesSmmRelocationComplete (
 
   ZeroMem ((VOID *)MapPagesBase, EFI_PAGES_TO_SIZE (MapPagesCount));
 
-  Status = gBS->FreePages (MapPagesBase, MapPagesCount);
-  ASSERT_EFI_ERROR (Status);
+  if (PcdGetBool (PcdQ35SmramAtDefaultSmbase)) {
+    //
+    // The initial SMRAM Save State Map has been covered as part of a larger
+    // reserved memory allocation in PlatformPei's InitializeRamRegions(). That
+    // allocation is supposed to survive into OS runtime; we must not release
+    // any part of it. Only re-assert the containment here.
+    //
+    ASSERT (SMM_DEFAULT_SMBASE <= MapPagesBase);
+    ASSERT (
+      (MapPagesBase + EFI_PAGES_TO_SIZE (MapPagesCount) <=
+       SMM_DEFAULT_SMBASE + MCH_DEFAULT_SMBASE_SIZE)
+      );
+  } else {
+    Status = gBS->FreePages (MapPagesBase, MapPagesCount);
+    ASSERT_EFI_ERROR (Status);
+  }
 }
 
 /**
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 2ae8126ccf8a..e484f4b311fe 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -9,6 +9,7 @@
 //
 // The package level header files this module uses
 //
+#include <IndustryStandard/Q35MchIch9.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/MemEncryptSevLib.h>
@@ -16,6 +17,7 @@
 #include <PiPei.h>
 #include <Register/Amd/Cpuid.h>
 #include <Register/Cpuid.h>
+#include <Register/Intel/SmramSaveStateMap.h>
 
 #include "Platform.h"
 
@@ -83,10 +85,22 @@ AmdSevInitialize (
                         );
     ASSERT_RETURN_ERROR (LocateMapStatus);
 
-    BuildMemoryAllocationHob (
-      MapPagesBase,                      // BaseAddress
-      EFI_PAGES_TO_SIZE (MapPagesCount), // Length
-      EfiBootServicesData                // MemoryType
-      );
+    if (mQ35SmramAtDefaultSmbase) {
+      //
+      // The initial SMRAM Save State Map has been covered as part of a larger
+      // reserved memory allocation in InitializeRamRegions().
+      //
+      ASSERT (SMM_DEFAULT_SMBASE <= MapPagesBase);
+      ASSERT (
+        (MapPagesBase + EFI_PAGES_TO_SIZE (MapPagesCount) <=
+         SMM_DEFAULT_SMBASE + MCH_DEFAULT_SMBASE_SIZE)
+        );
+    } else {
+      BuildMemoryAllocationHob (
+        MapPagesBase,                      // BaseAddress
+        EFI_PAGES_TO_SIZE (MapPagesCount), // Length
+        EfiBootServicesData                // MemoryType
+        );
+    }
   }
 }
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 09/11] OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE

[Laszlo Ersek]

During normal boot, when EFI_DXE_SMM_READY_TO_LOCK_PROTOCOL is installed
by platform BDS, the SMM IPL locks SMRAM (TSEG) through
EFI_SMM_ACCESS2_PROTOCOL.Lock(). See SmmIplReadyToLockEventNotify() in
"MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c".

During S3 resume, S3Resume2Pei locks SMRAM (TSEG) through
PEI_SMM_ACCESS_PPI.Lock(), before executing the boot script. See
S3ResumeExecuteBootScript() in
"UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c".

Those are precisely the places where the SMRAM at the default SMBASE
should be locked too. Add such an action to SmramAccessLock().

Notes:

- The SMRAM at the default SMBASE doesn't support the "closed and
  unlocked" state (and so it can't be closed without locking it, and it
  cannot be opened after closing it).

- The SMRAM at the default SMBASE isn't (and shouldn't) be exposed with
  another EFI_SMRAM_DESCRIPTOR in the GetCapabilities() members of
  EFI_SMM_ACCESS2_PROTOCOL / PEI_SMM_ACCESS_PPI. That's because the SMRAM
  in question is not "general purpose"; it's only QEMU's solution to
  protect the initial SMI handler from the OS, when a VCPU is hot-plugged.

  Consequently, the state of the SMRAM at the default SMBASE is not
  reflected in the "OpenState" / "LockState" fields of the protocol and
  PPI.

- An alternative to extending SmramAccessLock() would be to register an
  EFI_DXE_SMM_READY_TO_LOCK_PROTOCOL notify in SmmAccess2Dxe (for locking
  at normal boot), and an EDKII_S3_SMM_INIT_DONE_GUID PPI notify in
  SmmAccessPei (for locking at S3 resume).

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---

Notes:
    v2:
    - trim Cc list
    - pick up Jiewen's R-b <https://edk2.groups.io/g/devel/message/48166>

 OvmfPkg/SmmAccess/SmmAccess2Dxe.inf |  1 +
 OvmfPkg/SmmAccess/SmmAccessPei.inf  |  1 +
 OvmfPkg/SmmAccess/SmramInternal.h   |  8 +++++++
 OvmfPkg/SmmAccess/SmmAccess2Dxe.c   |  7 ++++++
 OvmfPkg/SmmAccess/SmmAccessPei.c    |  6 +++++
 OvmfPkg/SmmAccess/SmramInternal.c   | 25 ++++++++++++++++++++
 6 files changed, 48 insertions(+)

diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
index 7ced6b4e7ff4..d86381d0fbe2 100644
--- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
+++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
@@ -49,6 +49,7 @@ [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
 [Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [Depex]
diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf b/OvmfPkg/SmmAccess/SmmAccessPei.inf
index d73a029cc790..1698c4ce6c92 100644
--- a/OvmfPkg/SmmAccess/SmmAccessPei.inf
+++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf
@@ -54,6 +54,7 @@ [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
 [Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [Ppis]
diff --git a/OvmfPkg/SmmAccess/SmramInternal.h b/OvmfPkg/SmmAccess/SmramInternal.h
index 74d962b4ecae..a4d8827adfe4 100644
--- a/OvmfPkg/SmmAccess/SmramInternal.h
+++ b/OvmfPkg/SmmAccess/SmramInternal.h
@@ -38,6 +38,14 @@ InitQ35TsegMbytes (
   VOID
   );
 
+/**
+  Save PcdQ35SmramAtDefaultSmbase into mQ35SmramAtDefaultSmbase.
+**/
+VOID
+InitQ35SmramAtDefaultSmbase (
+  VOID
+  );
+
 /**
   Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
   OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
index e098f6f15f77..3691a6cd1f10 100644
--- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
+++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
@@ -145,6 +145,13 @@ SmmAccess2DxeEntryPoint (
 
   InitQ35TsegMbytes ();
   GetStates (&mAccess2.LockState, &mAccess2.OpenState);
+
+  //
+  // SmramAccessLock() depends on "mQ35SmramAtDefaultSmbase"; init the latter
+  // just before exposing the former via EFI_SMM_ACCESS2_PROTOCOL.Lock().
+  //
+  InitQ35SmramAtDefaultSmbase ();
+
   return gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
                 &gEfiSmmAccess2ProtocolGuid, &mAccess2,
                 NULL);
diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c b/OvmfPkg/SmmAccess/SmmAccessPei.c
index d67850651c58..c8bbc17e907a 100644
--- a/OvmfPkg/SmmAccess/SmmAccessPei.c
+++ b/OvmfPkg/SmmAccess/SmmAccessPei.c
@@ -372,6 +372,12 @@ SmmAccessPeiEntryPoint (
   CopyMem (GuidHob, &SmramMap[DescIdxSmmS3ResumeState],
     sizeof SmramMap[DescIdxSmmS3ResumeState]);
 
+  //
+  // SmramAccessLock() depends on "mQ35SmramAtDefaultSmbase"; init the latter
+  // just before exposing the former via PEI_SMM_ACCESS_PPI.Lock().
+  //
+  InitQ35SmramAtDefaultSmbase ();
+
   //
   // We're done. The next step should succeed, but even if it fails, we can't
   // roll back the above BuildGuidHob() allocation, because PEI doesn't support
diff --git a/OvmfPkg/SmmAccess/SmramInternal.c b/OvmfPkg/SmmAccess/SmramInternal.c
index 09657d0f9b0f..0b07dc667b3f 100644
--- a/OvmfPkg/SmmAccess/SmramInternal.c
+++ b/OvmfPkg/SmmAccess/SmramInternal.c
@@ -21,6 +21,12 @@
 //
 UINT16 mQ35TsegMbytes;
 
+//
+// The value of PcdQ35SmramAtDefaultSmbase is saved into this variable at
+// module startup.
+//
+STATIC BOOLEAN mQ35SmramAtDefaultSmbase;
+
 /**
   Save PcdQ35TsegMbytes into mQ35TsegMbytes.
 **/
@@ -32,6 +38,17 @@ InitQ35TsegMbytes (
   mQ35TsegMbytes = PcdGet16 (PcdQ35TsegMbytes);
 }
 
+/**
+  Save PcdQ35SmramAtDefaultSmbase into mQ35SmramAtDefaultSmbase.
+**/
+VOID
+InitQ35SmramAtDefaultSmbase (
+  VOID
+  )
+{
+  mQ35SmramAtDefaultSmbase = PcdGetBool (PcdQ35SmramAtDefaultSmbase);
+}
+
 /**
   Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
   OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
@@ -125,6 +142,14 @@ SmramAccessLock (
   PciOr8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), MCH_ESMRAMC_T_EN);
   PciOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM),   MCH_SMRAM_D_LCK);
 
+  //
+  // Close & lock the SMRAM at the default SMBASE, if it exists.
+  //
+  if (mQ35SmramAtDefaultSmbase) {
+    PciWrite8 (DRAMC_REGISTER_Q35 (MCH_DEFAULT_SMBASE_CTL),
+      MCH_DEFAULT_SMBASE_LCK);
+  }
+
   GetStates (LockState, OpenState);
   if (*OpenState || !*LockState) {
     return EFI_DEVICE_ERROR;
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 10/11] OvmfPkg: introduce PcdCsmEnable feature flag

[Laszlo Ersek]

In the DXE phase and later, it is possible for a module to dynamically
determine whether a CSM is enabled. An example can be seen in commit
855743f71774 ("OvmfPkg: prevent 64-bit MMIO BAR degradation if there is no
CSM", 2016-05-25).

SEC and PEI phase modules cannot check the Legacy BIOS Protocol however.
For their sake, introduce a new feature PCD that simply reflects the
CSM_ENABLE build flag.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien@xen.org>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - new patch

 OvmfPkg/OvmfPkg.dec        | 5 +++++
 OvmfPkg/OvmfPkgIa32.dsc    | 3 +++
 OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
 OvmfPkg/OvmfPkgX64.dsc     | 3 +++
 OvmfPkg/OvmfXen.dsc        | 3 +++
 5 files changed, 17 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 57034c784f88..4c5b6511cb97 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -276,3 +276,8 @@ [PcdsFeatureFlag]
   #  runtime OS from tampering with firmware structures (special memory ranges
   #  used by OVMF, the varstore pflash chip, LockBox etc).
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|FALSE|BOOLEAN|0x1e
+
+  ## Informs modules (including pre-DXE-phase modules) whether the platform
+  #  firmware contains a CSM (Compatibility Support Module).
+  #
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|FALSE|BOOLEAN|0x35
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 43ff51933609..19728f20b34e 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -428,6 +428,9 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+!ifdef $(CSM_ENABLE)
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
+!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 5b3e217e9220..3c0c229e3a72 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -433,6 +433,9 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+!ifdef $(CSM_ENABLE)
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
+!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a378d0a449aa..f6c1d8d228c6 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -433,6 +433,9 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+!ifdef $(CSM_ENABLE)
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
+!endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 1b18f1769bbc..5751ff1f0352 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -361,6 +361,9 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+!ifdef $(CSM_ENABLE)
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
+!endif
 
 [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 11/11] OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real)

[Laszlo Ersek]

Now that the SMRAM at the default SMBASE is honored everywhere necessary,
implement the actual detection. The (simple) steps are described in
previous patch "OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register
macros".

Regarding CSM_ENABLE builds: according to the discussion with Jiewen at

  https://edk2.groups.io/g/devel/message/48082
  http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503F7C9D2F@shsmsx102.ccr.corp.intel.com

if the platform has SMRAM at the default SMBASE, then we have to

(a) either punch a hole in the legacy E820 map as well, in
    LegacyBiosBuildE820() [OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c],

(b) or document, or programmatically catch, the incompatibility between
    the "SMRAM at default SMBASE" and "CSM" features.

Because CSM is out of scope for the larger "VCPU hotplug with SMM"
feature, option (b) applies. Therefore, if the CSM is enabled in the OVMF
build, then PlatformPei will not attempt to detect SMRAM at the default
SMBASE, at all. This is approach (4) -- the most flexible one, for
end-users -- from:

  http://mid.mail-archive.com/868dcff2-ecaa-e1c6-f018-abe7087d640c@redhat.com
  https://edk2.groups.io/g/devel/message/48348

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - In CSM_ENABLE builds, pretend that SMRAM never exists at the default
      SMBASE [Jiewen]
    - trim Cc list

 OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/PlatformPei/MemDetect.c     | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 25229618ed13..c51a6176aa2e 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -106,6 +106,7 @@ [FixedPcd]
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
 
 [FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
 [Ppis]
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 8fdc9c2ed7c9..47dc9c543719 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -103,6 +103,22 @@ Q35SmramAtDefaultSmbaseInitialization (
   ASSERT (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID);
 
   mQ35SmramAtDefaultSmbase = FALSE;
+  if (FeaturePcdGet (PcdCsmEnable)) {
+    DEBUG ((DEBUG_INFO, "%a: SMRAM at default SMBASE not checked due to CSM\n",
+      __FUNCTION__));
+  } else {
+    UINTN CtlReg;
+    UINT8 CtlRegVal;
+
+    CtlReg = DRAMC_REGISTER_Q35 (MCH_DEFAULT_SMBASE_CTL);
+    PciWrite8 (CtlReg, MCH_DEFAULT_SMBASE_QUERY);
+    CtlRegVal = PciRead8 (CtlReg);
+    mQ35SmramAtDefaultSmbase = (BOOLEAN)(CtlRegVal ==
+                                         MCH_DEFAULT_SMBASE_IN_RAM);
+    DEBUG ((DEBUG_INFO, "%a: SMRAM at default SMBASE %a\n", __FUNCTION__,
+      mQ35SmramAtDefaultSmbase ? "found" : "not found"));
+  }
+
   PcdStatus = PcdSetBoolS (PcdQ35SmramAtDefaultSmbase,
                 mQ35SmramAtDefaultSmbase);
   ASSERT_RETURN_ERROR (PcdStatus);
-- 
2.19.1.3.g30247aa5d201

Re: [PATCH v2 00/11] support QEMU's "SMRAM at default SMBASE" feature

[Ard Biesheuvel]

On Wed, 29 Jan 2020 at 21:44, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Ref:        https://bugzilla.tianocore.org/show_bug.cgi?id=1512
> Repo:       https://github.com/lersek/edk2.git
> Branch:     smram_at_default_smbase_bz_1512_wave_1_v2
> Supersedes: <20190924113505.27272-1-lersek@redhat.com>
>
> V1 is archived at:
> - http://mid.mail-archive.com/20190924113505.27272-1-lersek@redhat.com
> - https://edk2.groups.io/g/devel/message/47924
>
> Igor's patch set, mentioned in the v1 blurb, has been merged into QEMU
> meanwhile. The relevant QEMU commit is f404220e279c ("q35: implement
> 128K SMRAM at default SMBASE address", 2020-01-22).
>
> In v2:
> - trim the Cc list
>
> - pick up Jiewen's R-b for patches #1 through #9, from:
>   - http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503F7CBCB2@shsmsx102.ccr.corp.intel.com
>   - https://edk2.groups.io/g/devel/message/48166
>
> - add patch #10, and update patch #11, for satisfying Jiewen's condition
>   on his R-b.
>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien@xen.org>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (11):
>   OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase
>   OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro
>     defs
>   OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros
>   OvmfPkg/PlatformPei: factor out Q35BoardVerification()
>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton)
>   OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default
>     SMBASE
>   OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it
>     exists
>   OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default
>     SMBASE
>   OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE
>   OvmfPkg: introduce PcdCsmEnable feature flag
>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real)
>

For the series,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h           | 106 +++++++++++---------
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   |  21 +++-
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |   4 +
>  OvmfPkg/OvmfPkg.dec                                     |  11 ++
>  OvmfPkg/OvmfPkgIa32.dsc                                 |   4 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                              |   4 +
>  OvmfPkg/OvmfPkgX64.dsc                                  |   4 +
>  OvmfPkg/OvmfXen.dsc                                     |   3 +
>  OvmfPkg/PlatformPei/AmdSev.c                            |  24 ++++-
>  OvmfPkg/PlatformPei/MemDetect.c                         |  94 ++++++++++++++---
>  OvmfPkg/PlatformPei/Platform.c                          |  24 +++++
>  OvmfPkg/PlatformPei/Platform.h                          |   7 ++
>  OvmfPkg/PlatformPei/PlatformPei.inf                     |   2 +
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c                       |   7 ++
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf                     |   1 +
>  OvmfPkg/SmmAccess/SmmAccessPei.c                        |   6 ++
>  OvmfPkg/SmmAccess/SmmAccessPei.inf                      |   1 +
>  OvmfPkg/SmmAccess/SmramInternal.c                       |  25 +++++
>  OvmfPkg/SmmAccess/SmramInternal.h                       |   8 ++
>  19 files changed, 285 insertions(+), 71 deletions(-)
>
> --
> 2.19.1.3.g30247aa5d201
>

Re: [edk2-devel] [PATCH v2 00/11] support QEMU's "SMRAM at default SMBASE" feature

[Laszlo Ersek]

On 02/05/20 01:22, Ard Biesheuvel wrote:
> On Wed, 29 Jan 2020 at 21:44, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> Ref:        https://bugzilla.tianocore.org/show_bug.cgi?id=1512
>> Repo:       https://github.com/lersek/edk2.git
>> Branch:     smram_at_default_smbase_bz_1512_wave_1_v2
>> Supersedes: <20190924113505.27272-1-lersek@redhat.com>
>>
>> V1 is archived at:
>> - http://mid.mail-archive.com/20190924113505.27272-1-lersek@redhat.com
>> - https://edk2.groups.io/g/devel/message/47924
>>
>> Igor's patch set, mentioned in the v1 blurb, has been merged into QEMU
>> meanwhile. The relevant QEMU commit is f404220e279c ("q35: implement
>> 128K SMRAM at default SMBASE address", 2020-01-22).
>>
>> In v2:
>> - trim the Cc list
>>
>> - pick up Jiewen's R-b for patches #1 through #9, from:
>>   - http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503F7CBCB2@shsmsx102.ccr.corp.intel.com
>>   - https://edk2.groups.io/g/devel/message/48166
>>
>> - add patch #10, and update patch #11, for satisfying Jiewen's condition
>>   on his R-b.
>>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien@xen.org>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (11):
>>   OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase
>>   OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro
>>     defs
>>   OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros
>>   OvmfPkg/PlatformPei: factor out Q35BoardVerification()
>>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton)
>>   OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default
>>     SMBASE
>>   OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it
>>     exists
>>   OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default
>>     SMBASE
>>   OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE
>>   OvmfPkg: introduce PcdCsmEnable feature flag
>>   OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real)
>>
> 
> For the series,
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Much appreciated!

Pushed via <https://github.com/tianocore/edk2/pull/332>, commit range
422da35375c6..75839f977d37.

Laszlo

> 
> 
>>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h           | 106 +++++++++++---------
>>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   |  21 +++-
>>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |   4 +
>>  OvmfPkg/OvmfPkg.dec                                     |  11 ++
>>  OvmfPkg/OvmfPkgIa32.dsc                                 |   4 +
>>  OvmfPkg/OvmfPkgIa32X64.dsc                              |   4 +
>>  OvmfPkg/OvmfPkgX64.dsc                                  |   4 +
>>  OvmfPkg/OvmfXen.dsc                                     |   3 +
>>  OvmfPkg/PlatformPei/AmdSev.c                            |  24 ++++-
>>  OvmfPkg/PlatformPei/MemDetect.c                         |  94 ++++++++++++++---
>>  OvmfPkg/PlatformPei/Platform.c                          |  24 +++++
>>  OvmfPkg/PlatformPei/Platform.h                          |   7 ++
>>  OvmfPkg/PlatformPei/PlatformPei.inf                     |   2 +
>>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c                       |   7 ++
>>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf                     |   1 +
>>  OvmfPkg/SmmAccess/SmmAccessPei.c                        |   6 ++
>>  OvmfPkg/SmmAccess/SmmAccessPei.inf                      |   1 +
>>  OvmfPkg/SmmAccess/SmramInternal.c                       |  25 +++++
>>  OvmfPkg/SmmAccess/SmramInternal.h                       |   8 ++
>>  19 files changed, 285 insertions(+), 71 deletions(-)
>>
>> --
>> 2.19.1.3.g30247aa5d201
>>
> 
> 
>