From 4e5edfcdf5986d9e0801a976a3aa558b5f370099 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 27 Aug 2020 00:21:28 +0200 Subject: [PATCH 2/5] OvmfPkg/CpuHotplugSmm: fix CPU hotplug race just before SMI broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Laszlo Ersek (lersek) RH-MergeRequest: 1: [RHEL-8.4.0] complete the "VCPU hotplug with SMI" OVMF feature RH-Commit: [2/3] ea3ff703dfb7bd4f77b6807f06c89e754cc9d980 (lersek/edk2) RH-Bugzilla: 1849177 The "virsh setvcpus" (plural) command may hot-plug several VCPUs in quick succession -- it means a series of "device_add" QEMU monitor commands, back-to-back. If a "device_add" occurs *just before* ACPI raises the broadcast SMI, then: - OVMF processes the hot-added CPU well. - However, QEMU's post-SMI ACPI loop -- which clears the pending events for the hot-added CPUs that were collected before raising the SMI -- is unaware of the stray CPU. Thus, the pending event is not cleared for it. As a result of the stuck event, at the next hot-plug, OVMF tries to re-add (relocate for the 2nd time) the already-known CPU. At that time, the AP is already in the normal edk2 SMM busy-wait however, so it doesn't respond to the exchange that the BSP intends to do in SmbaseRelocate(). Thus the VM gets stuck in SMM. (Because of the above symptom, this is not considered a security patch; it doesn't seem exploitable by a malicious guest OS.) In CpuHotplugMmi(), skip the supposedly hot-added CPU if it's already known. The post-SMI ACPI loop will clear the pending event for it this time. Cc: Ard Biesheuvel Cc: Igor Mammedov Cc: Jordan Justen Cc: Philippe Mathieu-Daudé Fixes: bc498ac4ca7590479cfd91ad1bb8a36286b0dc21 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2929 Signed-off-by: Laszlo Ersek Message-Id: <20200826222129.25798-2-lersek@redhat.com> Reviewed-by: Ard Biesheuvel (cherry picked from commit 020bb4b46d6f6708bb3358e1c738109b7908f0de) Signed-off-by: Laszlo Ersek --- OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c index 20e6bec04f..cfe698ed2b 100644 --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c @@ -193,9 +193,28 @@ CpuHotplugMmi ( NewSlot = 0; while (PluggedIdx < PluggedCount) { APIC_ID NewApicId; + UINT32 CheckSlot; UINTN NewProcessorNumberByProtocol; NewApicId = mPluggedApicIds[PluggedIdx]; + + // + // Check if the supposedly hot-added CPU is already known to us. + // + for (CheckSlot = 0; + CheckSlot < mCpuHotPlugData->ArrayLength; + CheckSlot++) { + if (mCpuHotPlugData->ApicId[CheckSlot] == NewApicId) { + break; + } + } + if (CheckSlot < mCpuHotPlugData->ArrayLength) { + DEBUG ((DEBUG_VERBOSE, "%a: APIC ID " FMT_APIC_ID " was hot-plugged " + "before; ignoring it\n", __FUNCTION__, NewApicId)); + PluggedIdx++; + continue; + } + // // Find the first empty slot in CPU_HOT_PLUG_DATA. // -- 2.27.0