public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Anthony Perard <anthony.perard@citrix.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Eric Dong <eric.dong@intel.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Julien Grall <julien.grall@arm.com>, Ray Ni <ray.ni@intel.com>
Subject: [PATCH 4/4] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug
Date: Tue,  8 Oct 2019 13:27:14 +0200	[thread overview]
Message-ID: <20191008112714.6215-5-lersek@redhat.com> (raw)
In-Reply-To: <20191008112714.6215-1-lersek@redhat.com>

MaxCpuCountInitialization() currently handles the following options:

(1) QEMU does not report the boot CPU count.

    In this case, PlatformPei makes MpInitLib enumerate APs up to the
    default PcdCpuMaxLogicalProcessorNumber value (64) minus 1, or until
    the default PcdCpuApInitTimeOutInMicroSeconds (50,000) elapses.
    (Whichever is reached first.)

    Time-limited AP enumeration had never been reliable on QEMU/KVM, which
    is why commit 45a70db3c3a5 strated handling case (2) below, in OVMF.

(2) QEMU reports the boot CPU count.

    In this case, PlatformPei sets PcdCpuMaxLogicalProcessorNumber to the
    reported boot CPU count, and PcdCpuApInitTimeOutInMicroSeconds to
    practically "infinity" (MAX_UINT32, ~71 minutes). That causes
    MpInitLib to enumerate exactly the available (boot) APs.

    With CPU hotplug in mind, this method is not good enough. While
    UefiCpuPkg expects PcdCpuMaxLogicalProcessorNumber to include both
    boot (i.e., cold-plugged), and all *potentially* hot-plugged, logical
    processors, the boot CPU count reported by QEMU does not cover the
    second category.

Rewrite MaxCpuCountInitialization() for handling the following cases:

(1) The behavior of case (1) does not change. (No UefiCpuPkg PCDs are set
    to values different from the defaults.)

(2) QEMU reports the boot CPU count, but not the potential maximum CPU
    count.

    In this case, the behavior remains unchanged.

    The way MpInitLib is instructed to do the same differs however: we now
    set the new PcdCpuBootLogicalProcessorNumber to the boot CPU count
    (together with PcdCpuMaxLogicalProcessorNumber).
    PcdCpuApInitTimeOutInMicroSeconds is irrelevant.

(3) QEMU reports both the boot CPU count, and the potential maximum CPU
    count.

    We tell UefiCpuPkg about the potential maximum through
    PcdCpuMaxLogicalProcessorNumber. We also tell MpInitLib the boot CPU
    count for precise and quick AP enumeration, via
    PcdCpuBootLogicalProcessorNumber. PcdCpuApInitTimeOutInMicroSeconds is
    irrelevant.

This patch is a pre-requisite for enabling CPU hotplug with SMM_REQUIRE.
As a side effect, it also enables S3 to work with CPU hotplug at once,
*without* SMM_REQUIRE.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc             |  2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc          |  2 +-
 OvmfPkg/OvmfPkgX64.dsc              |  2 +-
 OvmfPkg/PlatformPei/PlatformPei.inf |  2 +-
 OvmfPkg/PlatformPei/Platform.c      | 83 +++++++++++++-------
 5 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 66e944436a69..89b1fc41458d 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -554,7 +554,7 @@ [PcdsDynamicDefault]
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
 
   # Set memory encryption mask
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 51c2bfb44f14..f978d27a0b55 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -566,7 +566,7 @@ [PcdsDynamicDefault]
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
 
   # Set memory encryption mask
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ba7a75884490..04604ed6b35b 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -565,7 +565,7 @@ [PcdsDynamicDefault]
 
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
 
   # Set memory encryption mask
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index d9fd9c8f05b3..30eaebdfae63 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -98,7 +98,7 @@ [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
 
 [FixedPcd]
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 3ba2459872d9..12cec1d60ec1 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -564,43 +564,72 @@ S3Verification (
 
 
 /**
-  Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg modules.
-  Set the mMaxCpuCount variable.
+  Fetch the boot CPU and max CPU numbers from QEMU, and expose them to
+  UefiCpuPkg modules. Set the mMaxCpuCount variable.
 **/
 VOID
 MaxCpuCountInitialization (
   VOID
   )
 {
-  UINT16        ProcessorCount;
-  RETURN_STATUS PcdStatus;
+  UINT16        BootCpuCount;
+  RETURN_STATUS Status;
 
+  //
+  // Try to fetch the boot CPU count.
+  //
   QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount);
-  ProcessorCount = QemuFwCfgRead16 ();
-  //
-  // If the fw_cfg key or fw_cfg entirely is unavailable, load mMaxCpuCount
-  // from the PCD default. No change to PCDs.
-  //
-  if (ProcessorCount == 0) {
+  BootCpuCount = QemuFwCfgRead16 ();
+  if (BootCpuCount == 0) {
+    //
+    // QEMU doesn't report the boot CPU count. Let MpInitLib count APs up to
+    // (PcdCpuMaxLogicalProcessorNumber-1), or until
+    // PcdCpuApInitTimeOutInMicroSeconds elapses (whichever is reached first).
+    //
+    DEBUG ((DEBUG_WARN, "%a: boot CPU count unavailable\n", __FUNCTION__));
     mMaxCpuCount = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
-    return;
+  } else {
+    //
+    // We will expose BootCpuCount to MpInitLib. MpInitLib will count APs up to
+    // (BootCpuCount-1) precisely, regardless of timeout.
+    //
+    // Now try to fetch topology info.
+    //
+    FIRMWARE_CONFIG_ITEM FwCfgItem;
+    UINTN                FwCfgSize;
+    FW_CFG_X86_TOPOLOGY  Topology;
+
+    Status = QemuFwCfgFindFile ("etc/x86-smp-topology", &FwCfgItem,
+               &FwCfgSize);
+    if (RETURN_ERROR (Status) || FwCfgSize != sizeof Topology) {
+      //
+      // QEMU doesn't report the topology. Assume that the maximum CPU count
+      // equals the boot CPU count (implying no hotplug).
+      //
+      DEBUG ((DEBUG_WARN, "%a: topology unavailable\n", __FUNCTION__));
+      mMaxCpuCount = BootCpuCount;
+    } else {
+      //
+      // Grab the maximum CPU count from the topology info.
+      //
+      QemuFwCfgSelectItem (FwCfgItem);
+      QemuFwCfgReadBytes (FwCfgSize, &Topology);
+      DEBUG ((DEBUG_VERBOSE,
+        "%a: DiesPerSocket=%u CoresPerDie=%u ThreadsPerCore=%u\n",
+        __FUNCTION__, Topology.DiesPerSocket, Topology.CoresPerDie,
+        Topology.ThreadsPerCore));
+      mMaxCpuCount = Topology.MaxLogicalProcessors;
+    }
   }
-  //
-  // Otherwise, set mMaxCpuCount to the value reported by QEMU.
-  //
-  mMaxCpuCount = ProcessorCount;
-  //
-  // Additionally, tell UefiCpuPkg modules (a) the exact number of VCPUs, (b)
-  // to wait, in the initial AP bringup, exactly as long as it takes for all of
-  // the APs to report in. For this, we set the longest representable timeout
-  // (approx. 71 minutes).
-  //
-  PcdStatus = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, ProcessorCount);
-  ASSERT_RETURN_ERROR (PcdStatus);
-  PcdStatus = PcdSet32S (PcdCpuApInitTimeOutInMicroSeconds, MAX_UINT32);
-  ASSERT_RETURN_ERROR (PcdStatus);
-  DEBUG ((DEBUG_INFO, "%a: QEMU reports %d processor(s)\n", __FUNCTION__,
-    ProcessorCount));
+
+  DEBUG ((DEBUG_INFO, "%a: BootCpuCount=%d mMaxCpuCount=%u\n", __FUNCTION__,
+    BootCpuCount, mMaxCpuCount));
+  ASSERT (BootCpuCount <= mMaxCpuCount);
+
+  Status = PcdSet32S (PcdCpuBootLogicalProcessorNumber, BootCpuCount);
+  ASSERT_RETURN_ERROR (Status);
+  Status = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, mMaxCpuCount);
+  ASSERT_RETURN_ERROR (Status);
 }
 
 
-- 
2.19.1.3.g30247aa5d201


  parent reply	other threads:[~2019-10-08 11:27 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-08 11:27 [PATCH 0/4] UefiCpuPkg, OvmfPkg: separate PCDs for boot CPU count vs. max CPU count Laszlo Ersek
2019-10-08 11:27 ` [PATCH 1/4] UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection Laszlo Ersek
2019-10-09  0:57   ` [edk2-devel] " Dong, Eric
2019-10-09 14:55     ` Laszlo Ersek
2019-10-10  2:52       ` Ni, Ray
2019-10-10  7:38         ` Laszlo Ersek
2019-10-10  7:50           ` Ni, Ray
2019-10-10  8:28           ` Laszlo Ersek
2019-10-08 11:27 ` [PATCH 2/4] OvmfPkg/OvmfXen.dsc: remove PcdCpu* dynamic defaults Laszlo Ersek
2019-10-08 13:11   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-08 20:27     ` Laszlo Ersek
2019-10-08 11:27 ` [PATCH 3/4] OvmfPkg/IndustryStandard: define FW_CFG_X86_TOPOLOGY structure type Laszlo Ersek
2019-10-08 11:27 ` Laszlo Ersek [this message]
2019-10-08 15:06   ` [PATCH 4/4] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug Igor Mammedov
2019-10-08 21:12     ` Laszlo Ersek
2019-10-09 10:23       ` Igor Mammedov
2019-10-09 14:43         ` Laszlo Ersek
2019-10-08 11:35 ` [PATCH 0/4] UefiCpuPkg, OvmfPkg: separate PCDs for boot CPU count vs. max CPU count Ard Biesheuvel
2019-10-08 20:27   ` 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=20191008112714.6215-5-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