From a752dd07466c4f0bda5c14d001b096e778a44ad5 Mon Sep 17 00:00:00 2001 From: Ankur Arora Date: Thu, 11 Mar 2021 22:26:48 -0800 Subject: 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 Cc: Jordan Justen Cc: Ard Biesheuvel Cc: Igor Mammedov Cc: Boris Ostrovsky Cc: Aaron Young Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 Signed-off-by: Ankur Arora Message-Id: <20210312062656.2477515-3-ankur.a.arora@oracle.com> Reviewed-by: Laszlo Ersek --- OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 29 ++++++++--- OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 101 +++++++++++++++++++++++++------------ OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 1 + 3 files changed, 92 insertions(+), 39 deletions(-) (limited to 'OvmfPkg/CpuHotplugSmm') diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c index bf68fcd429..ee1497b931 100644 --- 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 // fragment the SMRAM map). // -// These arrays provide room for ("possible CPU count" minus one) APIC IDs -// each, as we don't expect every possible CPU to appear, or disappear, in a -// single MMI. The numbers of used (populated) elements in the arrays are +// The first array stores APIC IDs for hot-plug events, the second and the +// third store APIC IDs and QEMU CPU Selectors (both indexed similarly) for +// hot-unplug events. All of these provide room for "possible CPU count" minus +// one elements as we don't expect every possible CPU to appear, or disappear, +// in a single MMI. The numbers of used (populated) elements in the arrays are // determined on every MMI separately. // STATIC APIC_ID *mPluggedApicIds; STATIC APIC_ID *mToUnplugApicIds; +STATIC UINT32 *mToUnplugSelectors; // // Address of the non-SMRAM reserved memory page that contains the Post-SMM Pen // for hot-added CPUs. @@ -289,6 +292,7 @@ CpuHotplugMmi ( mPluggedApicIds, &PluggedCount, mToUnplugApicIds, + mToUnplugSelectors, &ToUnplugCount ); if (EFI_ERROR (Status)) { @@ -333,7 +337,9 @@ CpuHotplugEntry ( ) { EFI_STATUS Status; + UINTN Len; UINTN Size; + UINTN SizeSel; // // This module should only be included when SMM support is required. @@ -387,8 +393,9 @@ CpuHotplugEntry ( // // Allocate the data structures that depend on the possible CPU count. // - if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Size)) || - RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Size, &Size))) { + if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Len)) || + RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Len, &Size)) || + RETURN_ERROR (SafeUintnMult (sizeof (UINT32), Len, &SizeSel))) { Status = EFI_ABORTED; DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_PLUG_DATA\n", __FUNCTION__)); goto Fatal; @@ -405,6 +412,12 @@ CpuHotplugEntry ( DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status)); goto ReleasePluggedApicIds; } + Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel, + (VOID **)&mToUnplugSelectors); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status)); + goto ReleaseToUnplugApicIds; + } // // Allocate the Post-SMM Pen for hot-added CPUs. @@ -412,7 +425,7 @@ CpuHotplugEntry ( Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress, SystemTable->BootServices); if (EFI_ERROR (Status)) { - goto ReleaseToUnplugApicIds; + goto ReleaseToUnplugSelectors; } // @@ -472,6 +485,10 @@ ReleasePostSmmPen: SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices); mPostSmmPenAddress = 0; +ReleaseToUnplugSelectors: + gMmst->MmFreePool (mToUnplugSelectors); + mToUnplugSelectors = NULL; + ReleaseToUnplugApicIds: gMmst->MmFreePool (mToUnplugApicIds); mToUnplugApicIds = NULL; diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c index 8d4a6693c8..8434dd446b 100644 --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c @@ -145,27 +145,30 @@ QemuCpuhpWriteCommand ( On error, the contents of the output parameters are undefined. - @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for - accessing IO Ports. + @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for + accessing IO Ports. - @param[in] PossibleCpuCount The number of possible CPUs in the system. Must - be positive. + @param[in] PossibleCpuCount The number of possible CPUs in the system. Must + be positive. - @param[in] ApicIdCount The number of elements each one of the - PluggedApicIds and ToUnplugApicIds arrays can - accommodate. Must be positive. + @param[in] ApicIdCount The number of elements each one of the + PluggedApicIds and ToUnplugApicIds arrays can + accommodate. Must be positive. - @param[out] PluggedApicIds The APIC IDs of the CPUs that have been - hot-plugged. + @param[out] PluggedApicIds The APIC IDs of the CPUs that have been + hot-plugged. - @param[out] PluggedCount The number of filled-in APIC IDs in - PluggedApicIds. + @param[out] PluggedCount The number of filled-in APIC IDs in + PluggedApicIds. - @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be - hot-unplugged. + @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be + hot-unplugged. - @param[out] ToUnplugCount The number of filled-in APIC IDs in - ToUnplugApicIds. + @param[out] ToUnplugSelectors The QEMU Selectors of the CPUs that are about + to be hot-unplugged. + + @param[out] ToUnplugCount The number of filled-in APIC IDs in + ToUnplugApicIds. @retval EFI_INVALID_PARAMETER PossibleCpuCount is zero, or ApicIdCount is zero. @@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds ( OUT APIC_ID *PluggedApicIds, OUT UINT32 *PluggedCount, OUT APIC_ID *ToUnplugApicIds, + OUT UINT32 *ToUnplugSelectors, OUT UINT32 *ToUnplugCount ) { @@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds ( UINT32 PendingSelector; UINT8 CpuStatus; APIC_ID *ExtendIds; + UINT32 *ExtendSels; UINT32 *ExtendCount; APIC_ID NewApicId; @@ -245,10 +250,10 @@ QemuCpuhpCollectApicIds ( if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) { // // The "insert" event guarantees the "enabled" status; plus it excludes - // the "remove" event. + // the "fw_remove" event. // if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 || - (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) { + (CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) { DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: " "inconsistent CPU status\n", __FUNCTION__, CurrentSelector, CpuStatus)); @@ -259,33 +264,63 @@ QemuCpuhpCollectApicIds ( CurrentSelector)); ExtendIds = PluggedApicIds; + ExtendSels = NULL; ExtendCount = PluggedCount; - } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) { - DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n", __FUNCTION__, - CurrentSelector)); + } else if ((CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) { + // + // "fw_remove" event guarantees "enabled". + // + if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0) { + DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: " + "inconsistent CPU status\n", __FUNCTION__, CurrentSelector, + CpuStatus)); + return EFI_PROTOCOL_ERROR; + } + + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: fw_remove\n", + __FUNCTION__, CurrentSelector)); ExtendIds = ToUnplugApicIds; + ExtendSels = ToUnplugSelectors; ExtendCount = ToUnplugCount; + } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) { + // + // Let the OSPM deal with the "remove" event. + // + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove (ignored)\n", + __FUNCTION__, CurrentSelector)); + + ExtendIds = NULL; + ExtendSels = NULL; + ExtendCount = NULL; } else { DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: no event\n", __FUNCTION__, CurrentSelector)); break; } - // - // Save the APIC ID of the CPU with the pending event, to the corresponding - // APIC ID array. - // - if (*ExtendCount == ApicIdCount) { - DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__)); - return EFI_BUFFER_TOO_SMALL; - } - QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID); - NewApicId = QemuCpuhpReadCommandData (MmCpuIo); - DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__, - NewApicId)); - ExtendIds[(*ExtendCount)++] = NewApicId; + ASSERT ((ExtendIds == NULL) == (ExtendCount == NULL)); + ASSERT ((ExtendSels == NULL) || (ExtendIds != NULL)); + if (ExtendIds != NULL) { + // + // Save the APIC ID of the CPU with the pending event, to the + // corresponding APIC ID array. + // For unplug events, also save the CurrentSelector. + // + if (*ExtendCount == ApicIdCount) { + DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__)); + return EFI_BUFFER_TOO_SMALL; + } + QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID); + NewApicId = QemuCpuhpReadCommandData (MmCpuIo); + DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__, + NewApicId)); + if (ExtendSels != NULL) { + ExtendSels[(*ExtendCount)] = CurrentSelector; + } + ExtendIds[(*ExtendCount)++] = NewApicId; + } // // We've processed the CPU with (known) pending events, but we must never // clear events. Therefore we need to advance past this CPU manually; diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h index 8adaa0ad91..3e2c2192e1 100644 --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h @@ -55,6 +55,7 @@ QemuCpuhpCollectApicIds ( OUT APIC_ID *PluggedApicIds, OUT UINT32 *PluggedCount, OUT APIC_ID *ToUnplugApicIds, + OUT UINT32 *ToUnplugSelectors, OUT UINT32 *ToUnplugCount ); -- cgit v1.2.3