public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH 5/5] OvmfPkg/Q35TsegSizeLib: recognize an extended TSEG when QEMU offers it
Date: Thu,  8 Jun 2017 19:13:33 +0200	[thread overview]
Message-ID: <20170608171333.17937-6-lersek@redhat.com> (raw)
In-Reply-To: <20170608171333.17937-1-lersek@redhat.com>

This patch interfaces with the QEMU feature introduced in QEMU patch

  q35/mch: implement extended TSEG sizes

Excerpt:

> The q35 machine type currently lets the guest firmware select a 1MB, 2MB
> or 8MB TSEG (basically, SMRAM) size. In edk2/OVMF, we use 8MB, but even
> that is not enough when a lot of VCPUs (more than approx. 224) are
> configured -- SMRAM footprint scales largely proportionally with VCPU
> count.
>
> Introduce a new property for "mch" called "extended-tseg-mbytes", which
> expresses (in megabytes) the user's choice of TSEG (SMRAM) size.
>
> Invent a new, QEMU-specific register in the config space of the DRAM
> Controller, at offset 0x50, in order to allow guest firmware to query
> the TSEG (SMRAM) size.
>
> According to Intel Document Number 316966-002, Table 5-1 "DRAM
> Controller Register Address Map (D0:F0)":
>
> [...]
>
> Offsets 0x50 and 0x51 are not listed in Table 5-1. They are also not
> part of the standard PCI config space header. And they precede the
> capability list as well, which starts at 0xe0 for this device.
>
> When the guest writes value 0xffff to this register, the value that can
> be read back is that of "mch.extended-tseg-mbytes" -- unless it remains
> 0xffff. The guest is required to write 0xffff first (as opposed to a
> read-only register) because PCI config space is generally not cleared on
> QEMU reset, and after S3 resume or reboot, new guest firmware running on
> old QEMU could read a guest OS-injected value from this register.
>
> After reading the available "extended" TSEG size, the guest firmware may
> actually request that TSEG size by writing pattern 11b to the ESMRAMC
> register's TSEG_SZ bit-field. (The Intel spec referenced above defines
> only patterns 00b (1MB), 01b (2MB) and 10b (8MB); 11b is reserved.)
>
> On the QEMU command line, the value can be set with
>
>   -global mch.extended-tseg-mbytes=N
>
> The default value for 2.10+ q35 machine types is 16. [...] Users are
> responsible for choosing sensible TSEG sizes.
>
> On 2.9 and earlier q35 machine types, the default value is 0. This lets
> the 11b bit pattern in ESMRAMC.TSEG_SZ, and the register at offset 0x50,
> keep their original behavior.

Relegate PcdQ35TsegMbytes to a fallback role, renaming it to
PcdQ35TsegDefaultMbytes.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1447027
Ref: https://lists.01.org/pipermail/edk2-devel/2017-May/010456.html
Ref: http://mid.mail-archive.com/20170608161013.17920-1-lersek@redhat.com
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/OvmfPkg.dec                               |  8 ++--
 OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.inf |  2 +-
 OvmfPkg/Include/IndustryStandard/Q35MchIch9.h     |  4 ++
 OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.c   | 46 +++++++++++++++++++-
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 7b9220369b95..2cae6a189d62 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -92,19 +92,21 @@ [PcdsFixedAtBuild]
   #  MaxTarget and MaxLun, independently, should the host report higher values,
   #  so that scanning the number of devices given by their product is still
   #  acceptably fast.
   gUefiOvmfPkgTokenSpaceGuid.PcdVirtioScsiMaxTargetLimit|31|UINT16|6
   gUefiOvmfPkgTokenSpaceGuid.PcdVirtioScsiMaxLunLimit|7|UINT32|7
 
   ## The following setting controls how many megabytes we configure as TSEG on
-  #  Q35, for SMRAM purposes. Permitted values are: 1, 2, 8. Other values cause
-  #  undefined behavior.
+  #  Q35, for SMRAM purposes, by default. Permitted values are: 1, 2, 8. Other
+  #  values cause undefined behavior.
   #
   #  This PCD is only consulted if PcdSmmSmramRequire is TRUE (see below).
-  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8|UINT8|0x20
+  #  Furthermore, if QEMU offers an extended TSEG (as a Q35 extension), then
+  #  this PCD is ignored.
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegDefaultMbytes|8|UINT8|0x20
 
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize|0|UINT32|0xb
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase|0x0|UINT32|0xc
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase|0x0|UINT32|0xd
diff --git a/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.inf b/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.inf
index 8f99e55b8c48..b7467d1a716c 100644
--- a/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.inf
+++ b/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.inf
@@ -40,8 +40,8 @@ [LibraryClasses]
   PcdLib
   PciLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
 [FixedPcd]
-  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegDefaultMbytes
diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index f480455ae432..68485bec71f7 100644
--- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
+++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
@@ -29,14 +29,17 @@
 #define INTEL_Q35_MCH_DEVICE_ID 0x29C0
 
 //
 // B/D/F/Type: 0/0/0/PCI
 //
 #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
@@ -50,14 +53,15 @@
 
 #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
diff --git a/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.c b/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.c
index db57a8b308de..01054a093f51 100644
--- a/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.c
+++ b/OvmfPkg/Library/Q35TsegSizeLib/Q35TsegSizeLib.c
@@ -17,14 +17,15 @@
 #include <Library/PcdLib.h>
 #include <Library/PciLib.h>
 #include <Library/Q35TsegSizeLib.h>
 #include <OvmfPlatforms.h>
 
 STATIC BOOLEAN mPreferencesInitialized;
 STATIC UINT8   mPreferredEsmramcTsegSzMask;
+STATIC UINT16  mExtendedTsegMbytes;
 
 /**
   Fetch the preferences into static variables that are going to be used by the
   public functions of this library instance.
 
   The Q35 board requirement documented on those interfaces is commonly enforced
   here.
@@ -59,15 +60,49 @@ Q35TsegSizeGetPreferences (
       ));
     ASSERT (FALSE);
     CpuDeadLoop ();
   }
 
   mPreferencesInitialized = TRUE;
 
-  switch (FixedPcdGet8 (PcdQ35TsegMbytes)) {
+  //
+  // Check if QEMU offers an extended TSEG.
+  //
+  // This can be seen from writing MCH_EXT_TSEG_MB_QUERY to the MCH_EXT_TSEG_MB
+  // register, and reading back the register.
+  //
+  // On a QEMU machine type that does not offer an extended TSEG, the initial
+  // write overwrites whatever value a malicious guest OS may have placed in
+  // the (unimplemented) register, before entering S3 or rebooting.
+  // Subsequently, the read returns MCH_EXT_TSEG_MB_QUERY unchanged.
+  //
+  // On a QEMU machine type that offers an extended TSEG, the initial write
+  // triggers an update to the register. Subsequently, the value read back
+  // (which is guaranteed to differ from MCH_EXT_TSEG_MB_QUERY) tells us the
+  // number of megabytes.
+  //
+  PciWrite16 (DRAMC_REGISTER_Q35 (MCH_EXT_TSEG_MB), MCH_EXT_TSEG_MB_QUERY);
+  mExtendedTsegMbytes = PciRead16 (DRAMC_REGISTER_Q35 (MCH_EXT_TSEG_MB));
+  if (mExtendedTsegMbytes != MCH_EXT_TSEG_MB_QUERY) {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: %a: QEMU offers an extended TSEG (%d MB)\n",
+      gEfiCallerBaseName,
+      __FUNCTION__,
+      mExtendedTsegMbytes
+      ));
+
+    mPreferredEsmramcTsegSzMask = MCH_ESMRAMC_TSEG_EXT;
+    return;
+  }
+
+  //
+  // Fall back to the default TSEG size otherwise.
+  //
+  switch (FixedPcdGet8 (PcdQ35TsegDefaultMbytes)) {
   case 1:
     mPreferredEsmramcTsegSzMask = MCH_ESMRAMC_TSEG_1MB;
     break;
   case 2:
     mPreferredEsmramcTsegSzMask = MCH_ESMRAMC_TSEG_2MB;
     break;
   case 8:
@@ -161,14 +196,23 @@ Q35TsegSizeConvertEsmramcValToMbytes (
     break;
   case MCH_ESMRAMC_TSEG_2MB:
     Mbytes = 2;
     break;
   case MCH_ESMRAMC_TSEG_8MB:
     Mbytes = 8;
     break;
+  case MCH_ESMRAMC_TSEG_EXT:
+    if (mExtendedTsegMbytes != MCH_EXT_TSEG_MB_QUERY) {
+      Mbytes = mExtendedTsegMbytes;
+      break;
+    }
+    //
+    // Fall through otherwise -- QEMU didn't offer an extended TSEG, so this
+    // should never happen.
+    //
   default:
     DEBUG ((
       DEBUG_ERROR,
       "%a: %a: unknown TsegSizeBits=0x%02x\n",
       gEfiCallerBaseName,
       __FUNCTION__,
       TsegSizeBits
-- 
2.9.3



  parent reply	other threads:[~2017-06-08 17:12 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-08 17:13 [PATCH 0/5] OvmfPkg: recognize an extended TSEG when QEMU offers it Laszlo Ersek
2017-06-08 17:13 ` [PATCH 1/5] OvmfPkg: introduce Q35TsegSizeLib (class header and sole lib instance) Laszlo Ersek
2017-06-19 17:30   ` Jordan Justen
2017-06-19 19:39     ` Laszlo Ersek
2017-06-26 17:59       ` Jordan Justen
2017-06-26 19:12         ` Laszlo Ersek
2017-06-26 22:36           ` Jordan Justen
2017-06-26 23:04             ` Laszlo Ersek
2017-06-29 19:14               ` Jordan Justen
2017-07-01 20:42                 ` Laszlo Ersek
2017-07-02  5:50                   ` Jordan Justen
2017-06-21  0:52     ` Yao, Jiewen
2017-06-22 16:22       ` Laszlo Ersek
2017-06-08 17:13 ` [PATCH 2/5] OvmfPkg/PlatformPei: rebase to Q35TsegSizeLib Laszlo Ersek
2017-06-08 17:13 ` [PATCH 3/5] OvmfPkg/SmmAccess: rebase code unique to SmmAccessPei " Laszlo Ersek
2017-06-08 17:13 ` [PATCH 4/5] OvmfPkg/SmmAccess: rebase shared PEIM/DXE code " Laszlo Ersek
2017-06-08 17:13 ` Laszlo Ersek [this message]
2017-06-16  8:15 ` [PATCH 0/5] OvmfPkg: recognize an extended TSEG when QEMU offers it Laszlo Ersek
2017-06-19 18:09 ` Jordan Justen
2017-06-19 22:39   ` Laszlo Ersek
2017-06-20 22:29     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170608171333.17937-6-lersek@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox