OvmfPkg/CpuHotplugSmm: collect hot-unplug events

Process fw_remove events in QemuCpuhpCollectApicIds(), and collect APIC IDs
and QEMU CPU Selectors for CPUs being hot-unplugged.

In addition, we now ignore CPUs which only have remove set. These
CPUs haven't been processed by OSPM yet.

This is based on the QEMU hot-unplug protocol documented here:
  https://lore.kernel.org/qemu-devel/20201204170939.1815522-3-imammedo@redhat.com/

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Aaron Young <aaron.young@oracle.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Message-Id: <20210312062656.2477515-3-ankur.a.arora@oracle.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index bf68fcd42914188f5c89be1180d687a877c24383..ee1497b931407ed2482fbe3ea8cadf3741c4c818 100644 (file)
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -45,13 +45,16 @@ STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
 // don't want to allocate SMRAM at OS runtime, and potentially fail (or\r
 // fragment the SMRAM map).\r
 //\r
-// These arrays provide room for ("possible CPU count" minus one) APIC IDs\r
-// each, as we don't expect every possible CPU to appear, or disappear, in a\r
-// single MMI. The numbers of used (populated) elements in the arrays are\r
+// The first array stores APIC IDs for hot-plug events, the second and the\r
+// third store APIC IDs and QEMU CPU Selectors (both indexed similarly) for\r
+// hot-unplug events. All of these provide room for "possible CPU count" minus\r
+// one elements as we don't expect every possible CPU to appear, or disappear,\r
+// in a single MMI. The numbers of used (populated) elements in the arrays are\r
 // determined on every MMI separately.\r
 //\r
 STATIC APIC_ID *mPluggedApicIds;\r
 STATIC APIC_ID *mToUnplugApicIds;\r
+STATIC UINT32  *mToUnplugSelectors;\r
 //\r
 // Address of the non-SMRAM reserved memory page that contains the Post-SMM Pen\r
 // for hot-added CPUs.\r
@@ -289,6 +292,7 @@ CpuHotplugMmi (
              mPluggedApicIds,\r
              &PluggedCount,\r
              mToUnplugApicIds,\r
+             mToUnplugSelectors,\r
              &ToUnplugCount\r
              );\r
   if (EFI_ERROR (Status)) {\r
@@ -333,7 +337,9 @@ CpuHotplugEntry (
   )\r
 {\r
   EFI_STATUS Status;\r
+  UINTN      Len;\r
   UINTN      Size;\r
+  UINTN      SizeSel;\r
 \r
   //\r
   // This module should only be included when SMM support is required.\r
@@ -387,8 +393,9 @@ CpuHotplugEntry (
   //\r
   // Allocate the data structures that depend on the possible CPU count.\r
   //\r
-  if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Size)) ||\r
-      RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Size, &Size))) {\r
+  if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Len)) ||\r
+      RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Len, &Size)) ||\r
+      RETURN_ERROR (SafeUintnMult (sizeof (UINT32), Len, &SizeSel))) {\r
     Status = EFI_ABORTED;\r
     DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_PLUG_DATA\n", __FUNCTION__));\r
     goto Fatal;\r
@@ -405,6 +412,12 @@ CpuHotplugEntry (
     DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));\r
     goto ReleasePluggedApicIds;\r
   }\r
+  Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,\r
+                    (VOID **)&mToUnplugSelectors);\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));\r
+    goto ReleaseToUnplugApicIds;\r
+  }\r
 \r
   //\r
   // Allocate the Post-SMM Pen for hot-added CPUs.\r
@@ -412,7 +425,7 @@ CpuHotplugEntry (
   Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress,\r
              SystemTable->BootServices);\r
   if (EFI_ERROR (Status)) {\r
-    goto ReleaseToUnplugApicIds;\r
+    goto ReleaseToUnplugSelectors;\r
   }\r
 \r
   //\r
@@ -472,6 +485,10 @@ ReleasePostSmmPen:
   SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);\r
   mPostSmmPenAddress = 0;\r
 \r
+ReleaseToUnplugSelectors:\r
+  gMmst->MmFreePool (mToUnplugSelectors);\r
+  mToUnplugSelectors = NULL;\r
+\r
 ReleaseToUnplugApicIds:\r
   gMmst->MmFreePool (mToUnplugApicIds);\r
   mToUnplugApicIds = NULL;\r

diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 8d4a6693c8d63c1ef224a3debb8bf58bf9f29e1f..8434dd446b96712d4b6aaafa239802a08affe3e0 100644 (file)
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -145,27 +145,30 @@ QemuCpuhpWriteCommand (
 \r
   On error, the contents of the output parameters are undefined.\r
 \r
-  @param[in] MmCpuIo           The EFI_MM_CPU_IO_PROTOCOL instance for\r
-                               accessing IO Ports.\r
+  @param[in] MmCpuIo             The EFI_MM_CPU_IO_PROTOCOL instance for\r
+                                 accessing IO Ports.\r
 \r
-  @param[in] PossibleCpuCount  The number of possible CPUs in the system. Must\r
-                               be positive.\r
+  @param[in] PossibleCpuCount    The number of possible CPUs in the system. Must\r
+                                 be positive.\r
 \r
-  @param[in] ApicIdCount       The number of elements each one of the\r
-                               PluggedApicIds and ToUnplugApicIds arrays can\r
-                               accommodate. Must be positive.\r
+  @param[in] ApicIdCount         The number of elements each one of the\r
+                                 PluggedApicIds and ToUnplugApicIds arrays can\r
+                                 accommodate. Must be positive.\r
 \r
-  @param[out] PluggedApicIds   The APIC IDs of the CPUs that have been\r
-                               hot-plugged.\r
+  @param[out] PluggedApicIds     The APIC IDs of the CPUs that have been\r
+                                 hot-plugged.\r
 \r
-  @param[out] PluggedCount     The number of filled-in APIC IDs in\r
-                               PluggedApicIds.\r
+  @param[out] PluggedCount       The number of filled-in APIC IDs in\r
+                                 PluggedApicIds.\r
 \r
-  @param[out] ToUnplugApicIds  The APIC IDs of the CPUs that are about to be\r
-                               hot-unplugged.\r
+  @param[out] ToUnplugApicIds    The APIC IDs of the CPUs that are about to be\r
+                                 hot-unplugged.\r
 \r
-  @param[out] ToUnplugCount    The number of filled-in APIC IDs in\r
-                               ToUnplugApicIds.\r
+  @param[out] ToUnplugSelectors  The QEMU Selectors of the CPUs that are about\r
+                                 to be hot-unplugged.\r
+\r
+  @param[out] ToUnplugCount      The number of filled-in APIC IDs in\r
+                                 ToUnplugApicIds.\r
 \r
   @retval EFI_INVALID_PARAMETER  PossibleCpuCount is zero, or ApicIdCount is\r
                                  zero.\r
@@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds (
   OUT APIC_ID                      *PluggedApicIds,\r
   OUT UINT32                       *PluggedCount,\r
   OUT APIC_ID                      *ToUnplugApicIds,\r
+  OUT UINT32                       *ToUnplugSelectors,\r
   OUT UINT32                       *ToUnplugCount\r
   )\r
 {\r
@@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds (
     UINT32  PendingSelector;\r
     UINT8   CpuStatus;\r
     APIC_ID *ExtendIds;\r
+    UINT32  *ExtendSels;\r
     UINT32  *ExtendCount;\r
     APIC_ID NewApicId;\r
 \r
@@ -245,10 +250,10 @@ QemuCpuhpCollectApicIds (
     if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) {\r
       //\r
       // The "insert" event guarantees the "enabled" status; plus it excludes\r
-      // the "remove" event.\r
+      // the "fw_remove" event.\r
       //\r
       if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 ||\r
-          (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {\r
+          (CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {\r
         DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "\r
           "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,\r
           CpuStatus));\r
@@ -259,33 +264,63 @@ QemuCpuhpCollectApicIds (
         CurrentSelector));\r
 \r
       ExtendIds   = PluggedApicIds;\r
+      ExtendSels  = NULL;\r
       ExtendCount = PluggedCount;\r
-    } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {\r
-      DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n", __FUNCTION__,\r
-        CurrentSelector));\r
+    } else if ((CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {\r
+      //\r
+      // "fw_remove" event guarantees "enabled".\r
+      //\r
+      if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0) {\r
+        DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "\r
+          "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,\r
+          CpuStatus));\r
+        return EFI_PROTOCOL_ERROR;\r
+      }\r
+\r
+      DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: fw_remove\n",\r
+        __FUNCTION__, CurrentSelector));\r
 \r
       ExtendIds   = ToUnplugApicIds;\r
+      ExtendSels  = ToUnplugSelectors;\r
       ExtendCount = ToUnplugCount;\r
+    } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {\r
+      //\r
+      // Let the OSPM deal with the "remove" event.\r
+      //\r
+      DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove (ignored)\n",\r
+        __FUNCTION__, CurrentSelector));\r
+\r
+      ExtendIds   = NULL;\r
+      ExtendSels  = NULL;\r
+      ExtendCount = NULL;\r
     } else {\r
       DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: no event\n",\r
         __FUNCTION__, CurrentSelector));\r
       break;\r
     }\r
 \r
-    //\r
-    // Save the APIC ID of the CPU with the pending event, to the corresponding\r
-    // APIC ID array.\r
-    //\r
-    if (*ExtendCount == ApicIdCount) {\r
-      DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));\r
-      return EFI_BUFFER_TOO_SMALL;\r
-    }\r
-    QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);\r
-    NewApicId = QemuCpuhpReadCommandData (MmCpuIo);\r
-    DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,\r
-      NewApicId));\r
-    ExtendIds[(*ExtendCount)++] = NewApicId;\r
+    ASSERT ((ExtendIds == NULL) == (ExtendCount == NULL));\r
+    ASSERT ((ExtendSels == NULL) || (ExtendIds != NULL));\r
 \r
+    if (ExtendIds != NULL) {\r
+      //\r
+      // Save the APIC ID of the CPU with the pending event, to the\r
+      // corresponding APIC ID array.\r
+      // For unplug events, also save the CurrentSelector.\r
+      //\r
+      if (*ExtendCount == ApicIdCount) {\r
+        DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));\r
+        return EFI_BUFFER_TOO_SMALL;\r
+      }\r
+      QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);\r
+      NewApicId = QemuCpuhpReadCommandData (MmCpuIo);\r
+      DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,\r
+        NewApicId));\r
+      if (ExtendSels != NULL) {\r
+        ExtendSels[(*ExtendCount)] = CurrentSelector;\r
+      }\r
+      ExtendIds[(*ExtendCount)++] = NewApicId;\r
+    }\r
     //\r
     // We've processed the CPU with (known) pending events, but we must never\r
     // clear events. Therefore we need to advance past this CPU manually;\r

diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 8adaa0ad91f0bac8530c7213212f05395e591f3e..3e2c2192e1c01478e7f3c259871ae5473c82fd85 100644 (file)
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
@@ -55,6 +55,7 @@ QemuCpuhpCollectApicIds (
   OUT APIC_ID                      *PluggedApicIds,\r
   OUT UINT32                       *PluggedCount,\r
   OUT APIC_ID                      *ToUnplugApicIds,\r
+  OUT UINT32                       *ToUnplugSelectors,\r
   OUT UINT32                       *ToUnplugCount\r
   );\r
 \r

diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index a34a6d3fae61b0cdd70f81d893a3e6a2082db9d7..2ec7a107a64d20be3fd7c7b3dc3561a9bfab56d8 100644 (file)
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
 #define QEMU_CPUHP_STAT_ENABLED                BIT0\r
 #define QEMU_CPUHP_STAT_INSERT                 BIT1\r
 #define QEMU_CPUHP_STAT_REMOVE                 BIT2\r
+#define QEMU_CPUHP_STAT_FW_REMOVE              BIT4\r
 \r
 #define QEMU_CPUHP_RW_CMD_DATA               0x8\r
 \r