* [PATCH v7 00/10] support CPU hot-unplug
@ 2021-02-19 9:04 Ankur Arora
2021-02-19 9:04 ` [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic Ankur Arora
` (10 more replies)
0 siblings, 11 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel; +Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora
Hi,
This series adds OVMF support for CPU hot-unplug.
QEMU secureboot hot-unplug logic corresponding to this is in upstream.
Also posted here:
https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/
Testing (with QEMU 5.2.50):
- Stable with randomized CPU plug/unplug (guest maxcpus=33,128)
- Synthetic tests with simultaneous multi CPU hot-unplug
Also at:
github.com/terminus/edk2/ hot-unplug-v7
Changelog:
v7:
- Address review comments from v6.
- Fix ejection bug where we were using APIC ID to do the ejection
rather than the Qemu Selector.
- Describes safety properties and ordering needed for concurrent
accesses to CPU_HOT_EJECT_DATA->QemuSelectorMap, and
CPU_HOT_EJECT_DATA->Handler,
v6:
- addresses v5 review comments.
URL: https://patchew.org/EDK2/20210129005950.467638-1-ankur.a.arora@oracle.com/
v5:
- fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
add Qemu Cpu Status helper").
URL: https://patchew.org/EDK2/20210126064440.299596-1-ankur.a.arora@oracle.com/
v4:
- Gets rid of unnecessary UefiCpuPkg changes
URL: https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.arora@oracle.com/
v3:
- Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
and OvmfPkg/CpuHotplugSmm
- Cleaner split of the hot-unplug code
URL: https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.arora@oracle.com/
v2:
- Do the ejection via SmmCpuFeaturesRendezvousExit()
URL: https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.arora@oracle.com/
RFC:
URL: https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.arora@oracle.com/
Please review.
Thanks
Ankur
Ankur Arora (10):
OvmfPkg/CpuHotplugSmm: refactor hotplug logic
OvmfPkg/CpuHotplugSmm: collect hot-unplug events
OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
OvmfPkg/CpuHotplugSmm: add EjectCpu()
OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
OvmfPkg/OvmfPkg.dec | 4 +
OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +
OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 7 +
OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 2 +
OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 ++
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 578 +++++++++++++++++----
OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 106 +++-
.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 136 +++++
OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
10 files changed, 789 insertions(+), 120 deletions(-)
create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h
--
2.9.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-22 10:00 ` [edk2-devel] " Laszlo Ersek
2021-02-19 9:04 ` [PATCH v7 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events Ankur Arora
` (9 subsequent siblings)
10 siblings, 1 reply; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into
ProcessHotAddedCpus(). This is in preparation for supporting CPU
hot-unplug.
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Addresses these review comments from v6:
(1) s/EFI_ERROR(/EFI_ERROR (/
(2) Remove the empty line in the comment block above
ProcessHotAddedCpus().
() Nest the EFI_ERROR handling inside the (PluggedCount > 0) clause.
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 210 ++++++++++++++++++++++---------------
1 file changed, 126 insertions(+), 84 deletions(-)
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index cfe698ed2b5e..bf68fcd42914 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -62,6 +62,129 @@ STATIC UINT32 mPostSmmPenAddress;
//
STATIC EFI_HANDLE mDispatchHandle;
+/**
+ Process CPUs that have been hot-added, per QemuCpuhpCollectApicIds().
+
+ For each such CPU, relocate the SMBASE, and report the CPU to PiSmmCpuDxeSmm
+ via EFI_SMM_CPU_SERVICE_PROTOCOL. If the supposedly hot-added CPU is already
+ known, skip it silently.
+
+ @param[in] PluggedApicIds The APIC IDs of the CPUs that have been
+ hot-plugged.
+
+ @param[in] PluggedCount The number of filled-in APIC IDs in
+ PluggedApicIds.
+
+ @retval EFI_SUCCESS CPUs corresponding to all the APIC IDs are
+ populated.
+
+ @retval EFI_OUT_OF_RESOURCES Out of APIC ID space in "mCpuHotPlugData".
+
+ @return Error codes propagated from SmbaseRelocate()
+ and mMmCpuService->AddProcessor().
+**/
+STATIC
+EFI_STATUS
+ProcessHotAddedCpus (
+ IN APIC_ID *PluggedApicIds,
+ IN UINT32 PluggedCount
+ )
+{
+ EFI_STATUS Status;
+ UINT32 PluggedIdx;
+ UINT32 NewSlot;
+
+ //
+ // The Post-SMM Pen need not be reinstalled multiple times within a single
+ // root MMI handling. Even reinstalling once per root MMI is only prudence;
+ // in theory installing the pen in the driver's entry point function should
+ // suffice.
+ //
+ SmbaseReinstallPostSmmPen (mPostSmmPenAddress);
+
+ PluggedIdx = 0;
+ NewSlot = 0;
+ while (PluggedIdx < PluggedCount) {
+ APIC_ID NewApicId;
+ UINT32 CheckSlot;
+ UINTN NewProcessorNumberByProtocol;
+
+ NewApicId = PluggedApicIds[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.
+ //
+ while (NewSlot < mCpuHotPlugData->ArrayLength &&
+ mCpuHotPlugData->ApicId[NewSlot] != MAX_UINT64) {
+ NewSlot++;
+ }
+ if (NewSlot == mCpuHotPlugData->ArrayLength) {
+ DEBUG ((DEBUG_ERROR, "%a: no room for APIC ID " FMT_APIC_ID "\n",
+ __FUNCTION__, NewApicId));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Store the APIC ID of the new processor to the slot.
+ //
+ mCpuHotPlugData->ApicId[NewSlot] = NewApicId;
+
+ //
+ // Relocate the SMBASE of the new CPU.
+ //
+ Status = SmbaseRelocate (NewApicId, mCpuHotPlugData->SmBase[NewSlot],
+ mPostSmmPenAddress);
+ if (EFI_ERROR (Status)) {
+ goto RevokeNewSlot;
+ }
+
+ //
+ // Add the new CPU with EFI_SMM_CPU_SERVICE_PROTOCOL.
+ //
+ Status = mMmCpuService->AddProcessor (mMmCpuService, NewApicId,
+ &NewProcessorNumberByProtocol);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: AddProcessor(" FMT_APIC_ID "): %r\n",
+ __FUNCTION__, NewApicId, Status));
+ goto RevokeNewSlot;
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: hot-added APIC ID " FMT_APIC_ID ", SMBASE 0x%Lx, "
+ "EFI_SMM_CPU_SERVICE_PROTOCOL assigned number %Lu\n", __FUNCTION__,
+ NewApicId, (UINT64)mCpuHotPlugData->SmBase[NewSlot],
+ (UINT64)NewProcessorNumberByProtocol));
+
+ NewSlot++;
+ PluggedIdx++;
+ }
+
+ //
+ // We've processed this batch of hot-added CPUs.
+ //
+ return EFI_SUCCESS;
+
+RevokeNewSlot:
+ mCpuHotPlugData->ApicId[NewSlot] = MAX_UINT64;
+
+ return Status;
+}
/**
CPU Hotplug MMI handler function.
@@ -122,8 +245,6 @@ CpuHotplugMmi (
UINT8 ApmControl;
UINT32 PluggedCount;
UINT32 ToUnplugCount;
- UINT32 PluggedIdx;
- UINT32 NewSlot;
//
// Assert that we are entering this function due to our root MMI handler
@@ -179,87 +300,11 @@ CpuHotplugMmi (
goto Fatal;
}
- //
- // Process hot-added CPUs.
- //
- // The Post-SMM Pen need not be reinstalled multiple times within a single
- // root MMI handling. Even reinstalling once per root MMI is only prudence;
- // in theory installing the pen in the driver's entry point function should
- // suffice.
- //
- SmbaseReinstallPostSmmPen (mPostSmmPenAddress);
-
- PluggedIdx = 0;
- 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.
- //
- while (NewSlot < mCpuHotPlugData->ArrayLength &&
- mCpuHotPlugData->ApicId[NewSlot] != MAX_UINT64) {
- NewSlot++;
- }
- if (NewSlot == mCpuHotPlugData->ArrayLength) {
- DEBUG ((DEBUG_ERROR, "%a: no room for APIC ID " FMT_APIC_ID "\n",
- __FUNCTION__, NewApicId));
+ if (PluggedCount > 0) {
+ Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount);
+ if (EFI_ERROR (Status)) {
goto Fatal;
}
-
- //
- // Store the APIC ID of the new processor to the slot.
- //
- mCpuHotPlugData->ApicId[NewSlot] = NewApicId;
-
- //
- // Relocate the SMBASE of the new CPU.
- //
- Status = SmbaseRelocate (NewApicId, mCpuHotPlugData->SmBase[NewSlot],
- mPostSmmPenAddress);
- if (EFI_ERROR (Status)) {
- goto RevokeNewSlot;
- }
-
- //
- // Add the new CPU with EFI_SMM_CPU_SERVICE_PROTOCOL.
- //
- Status = mMmCpuService->AddProcessor (mMmCpuService, NewApicId,
- &NewProcessorNumberByProtocol);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a: AddProcessor(" FMT_APIC_ID "): %r\n",
- __FUNCTION__, NewApicId, Status));
- goto RevokeNewSlot;
- }
-
- DEBUG ((DEBUG_INFO, "%a: hot-added APIC ID " FMT_APIC_ID ", SMBASE 0x%Lx, "
- "EFI_SMM_CPU_SERVICE_PROTOCOL assigned number %Lu\n", __FUNCTION__,
- NewApicId, (UINT64)mCpuHotPlugData->SmBase[NewSlot],
- (UINT64)NewProcessorNumberByProtocol));
-
- NewSlot++;
- PluggedIdx++;
}
//
@@ -267,9 +312,6 @@ CpuHotplugMmi (
//
return EFI_SUCCESS;
-RevokeNewSlot:
- mCpuHotPlugData->ApicId[NewSlot] = MAX_UINT64;
-
Fatal:
ASSERT (FALSE);
CpuDeadLoop ();
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
2021-02-19 9:04 ` [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 03/10] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper Ankur Arora
` (8 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Process fw_remove events in QemuCpuhpCollectApicIds() and collect
corresponding APIC IDs for CPUs that are 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>
---
Notes:
Addresses the following review comments from v6:
(1,4) Move (and also rename) QEMU_CPUHP_STAT_EJECTED to patch 8,
where we actually use it.
(2) Downgrade debug mask from DEBUG_INFO to DEBUG_VERBOSE.
(3a,3b,3c) Keep the CurrentSelector increment operation at
the tail of the loop.
() As discussed elsewhere we also need to get the CpuSelector while
collecting ApicIds in QemuCpuhpCollectApicIds(). This patch adds a
separate parameter for the CpuSelector values, because that works
better alongside the hotplug ExtendIds logic.
OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 1 +
OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 21 +++++-
OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 84 ++++++++++++++++-------
4 files changed, 79 insertions(+), 28 deletions(-)
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 8adaa0ad91f0..1e23b150910e 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 *ToUnplugSelector,
OUT UINT32 *ToUnplugCount
);
diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index a34a6d3fae61..2ec7a107a64d 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
#define QEMU_CPUHP_STAT_ENABLED BIT0
#define QEMU_CPUHP_STAT_INSERT BIT1
#define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_FW_REMOVE BIT4
#define QEMU_CPUHP_RW_CMD_DATA 0x8
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index bf68fcd42914..3192bfea1f15 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -52,6 +52,7 @@ STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
//
STATIC APIC_ID *mPluggedApicIds;
STATIC APIC_ID *mToUnplugApicIds;
+STATIC UINT32 *mToUnplugSelector;
//
// Address of the non-SMRAM reserved memory page that contains the Post-SMM Pen
// for hot-added CPUs.
@@ -289,6 +290,7 @@ CpuHotplugMmi (
mPluggedApicIds,
&PluggedCount,
mToUnplugApicIds,
+ mToUnplugSelector,
&ToUnplugCount
);
if (EFI_ERROR (Status)) {
@@ -333,7 +335,9 @@ CpuHotplugEntry (
)
{
EFI_STATUS Status;
+ UINTN Len;
UINTN Size;
+ UINTN SizeSel;
//
// This module should only be included when SMM support is required.
@@ -387,8 +391,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 +410,12 @@ CpuHotplugEntry (
DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
goto ReleasePluggedApicIds;
}
+ Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,
+ (VOID **)&mToUnplugSelector);
+ 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 +423,7 @@ CpuHotplugEntry (
Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress,
SystemTable->BootServices);
if (EFI_ERROR (Status)) {
- goto ReleaseToUnplugApicIds;
+ goto ReleaseToUnplugSelector;
}
//
@@ -472,6 +483,10 @@ ReleasePostSmmPen:
SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);
mPostSmmPenAddress = 0;
+ReleaseToUnplugSelector:
+ gMmst->MmFreePool (mToUnplugSelector);
+ mToUnplugSelector = NULL;
+
ReleaseToUnplugApicIds:
gMmst->MmFreePool (mToUnplugApicIds);
mToUnplugApicIds = NULL;
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 8d4a6693c8d6..36372a5e6193 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -164,6 +164,9 @@ QemuCpuhpWriteCommand (
@param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
hot-unplugged.
+ @param[out] ToUnplugSelector 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.
@@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds (
OUT APIC_ID *PluggedApicIds,
OUT UINT32 *PluggedCount,
OUT APIC_ID *ToUnplugApicIds,
+ OUT UINT32 *ToUnplugSelector,
OUT UINT32 *ToUnplugCount
)
{
@@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds (
UINT32 PendingSelector;
UINT8 CpuStatus;
APIC_ID *ExtendIds;
+ UINT32 *ExtendSel;
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,40 +264,69 @@ QemuCpuhpCollectApicIds (
CurrentSelector));
ExtendIds = PluggedApicIds;
+ ExtendSel = 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;
+ ExtendSel = ToUnplugSelector;
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;
+ ExtendSel = 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));
+ 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 (ExtendSel != NULL) {
+ ExtendSel[(*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;
- // otherwise, QEMU_CPUHP_CMD_GET_PENDING would stick to the currently
- // selected CPU.
- //
- CurrentSelector++;
+ //
+ // We've processed the CPU with (known) pending events, but we must never
+ // clear events. Therefore we need to advance past this CPU manually;
+ // otherwise, QEMU_CPUHP_CMD_GET_PENDING would stick to the currently
+ // selected CPU.
+ //
+ CurrentSelector++;
+ }
} while (CurrentSelector < PossibleCpuCount);
DEBUG ((DEBUG_VERBOSE, "%a: PluggedCount=%u ToUnplugCount=%u\n",
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 03/10] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
2021-02-19 9:04 ` [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic Ankur Arora
2021-02-19 9:04 ` [PATCH v7 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus() Ankur Arora
` (7 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Add QemuCpuhpWriteCpuStatus() which will be used to update the QEMU
CPU status register. On error, it hangs in a similar fashion as
other helper functions.
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>
---
Notes:
Address this review comment:
() Move QemuCpuhpWriteCpuStatus() (declaration and definition) between
QemuCpuhpWriteCpuSelector() and QemuCpuhpWriteCommand() to match
the order of the register descriptions in QEMU.
OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 6 ++++++
OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 22 ++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 1e23b150910e..859412c1a173 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
@@ -42,6 +42,12 @@ QemuCpuhpWriteCpuSelector (
);
VOID
+QemuCpuhpWriteCpuStatus (
+ IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
+ IN UINT8 CpuStatus
+ );
+
+VOID
QemuCpuhpWriteCommand (
IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
IN UINT8 Command
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 36372a5e6193..9434bb14dd4e 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -114,6 +114,28 @@ QemuCpuhpWriteCpuSelector (
}
VOID
+QemuCpuhpWriteCpuStatus (
+ IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
+ IN UINT8 CpuStatus
+ )
+{
+ EFI_STATUS Status;
+
+ Status = MmCpuIo->Io.Write (
+ MmCpuIo,
+ MM_IO_UINT8,
+ ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_R_CPU_STAT,
+ 1,
+ &CpuStatus
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, Status));
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+}
+
+VOID
QemuCpuhpWriteCommand (
IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
IN UINT8 Command
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (2 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 03/10] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA Ankur Arora
` (6 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Introduce UnplugCpus() which maps each APIC ID being unplugged
onto the hardware ID of the processor and informs PiSmmCpuDxeSmm
of removal by calling EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().
With this change we handle the first phase of unplug where we collect
the CPUs that need to be unplugged and mark them for removal in SMM
data structures.
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>
---
Notes:
Addresses these review comments from v6:
(1) Drop the empty line in the comment block around UnplugCpus().
(2) Make the "did not find APIC ID" DEBUG_VERBOSE instead of DEBUG_INFO.
(3) Un-Indented ("Outdented") the line following the comment "Ignore the
unplug if APIC ID.
(4) Remove the empty line between Status assignment and check.
(5) Drop the "goto Fatal" logic and just return Status directly.
(6) Handle both Plugging and Unplugging of CPUs in one go.
(7) Also nest the EFI_STATUS check.
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 84 ++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 3192bfea1f15..f07b5072749a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -188,6 +188,83 @@ RevokeNewSlot:
}
/**
+ Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
+
+ For each such CPU, report the CPU to PiSmmCpuDxeSmm via
+ EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
+ unknown, skip it silently.
+
+ @param[in] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
+ hot-unplugged.
+
+ @param[in] ToUnplugCount The number of filled-in APIC IDs in
+ ToUnplugApicIds.
+
+ @retval EFI_SUCCESS Known APIC IDs have been removed from SMM data
+ structures.
+
+ @return Error codes propagated from
+ mMmCpuService->RemoveProcessor().
+**/
+STATIC
+EFI_STATUS
+UnplugCpus (
+ IN APIC_ID *ToUnplugApicIds,
+ IN UINT32 ToUnplugCount
+ )
+{
+ EFI_STATUS Status;
+ UINT32 ToUnplugIdx;
+ UINTN ProcessorNum;
+
+ ToUnplugIdx = 0;
+ while (ToUnplugIdx < ToUnplugCount) {
+ APIC_ID RemoveApicId;
+
+ RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+
+ //
+ // mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
+ // the ProcessorNum for the APIC ID to be removed.
+ //
+ for (ProcessorNum = 0;
+ ProcessorNum < mCpuHotPlugData->ArrayLength;
+ ProcessorNum++) {
+ if (mCpuHotPlugData->ApicId[ProcessorNum] == RemoveApicId) {
+ break;
+ }
+ }
+
+ //
+ // Ignore the unplug if APIC ID not found
+ //
+ if (ProcessorNum == mCpuHotPlugData->ArrayLength) {
+ DEBUG ((DEBUG_VERBOSE, "%a: did not find APIC ID " FMT_APIC_ID
+ " to unplug\n", __FUNCTION__, RemoveApicId));
+ ToUnplugIdx++;
+ continue;
+ }
+
+ //
+ // Mark ProcessorNum for removal from SMM data structures
+ //
+ Status = mMmCpuService->RemoveProcessor (mMmCpuService, ProcessorNum);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: RemoveProcessor(" FMT_APIC_ID "): %r\n",
+ __FUNCTION__, RemoveApicId, Status));
+ return Status;
+ }
+
+ ToUnplugIdx++;
+ }
+
+ //
+ // We've removed this set of APIC IDs from SMM data structures.
+ //
+ return EFI_SUCCESS;
+}
+
+/**
CPU Hotplug MMI handler function.
This is a root MMI handler.
@@ -309,6 +386,13 @@ CpuHotplugMmi (
}
}
+ if (ToUnplugCount > 0) {
+ Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+ if (EFI_ERROR (Status)) {
+ goto Fatal;
+ }
+ }
+
//
// We've handled this MMI.
//
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (3 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus() Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state Ankur Arora
` (5 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Define CPU_HOT_EJECT_DATA and add PCD PcdCpuHotEjectDataAddress, which
will be used to share CPU ejection state between OvmfPkg/CpuHotPlugSmm
and PiSmmCpuDxeSmm.
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>
---
Notes:
Addresses the following review comments in v6:
(1) Dropped modifications to LibraryClasses in OvmfPkg.dec
(2,3) Cleanup comments around PCD PcdCpuHotEjectDataAddress.
(4) Move PCD PcdCpuHotEjectDataAddress declaration in CpuHotplugSmm.inf
to a patch-7 where it actually gets used.
(5a,5b) Change the comment in the top block to use Laszlo's language.
Also detail when the PCD would contain a valid value.
(6) Move Library/CpuHotEjectData.h to Pcd/CpuHotEjectData.h
(7,15,16) Fixup guard macro to be C namespace compliant. Also fixup the
comment style near the endif guard.
(8-10) Rename CPU_HOT_EJECT_FN to a more EDK2 compliant style. Also add
a comment block and fix spacing.
() Rename ApicIdMap -> QemuSelectorMap while keeping the type as UINT64.
Related to a comment in patch-8 ("... add worker to do CPU ejection".)
(11a,11b) Rename CPU_EJECT_INVALID to CPU_EJECT_QEMU_SELECTOR_INVALID
and add a comment about it.
() Remove CPU_EJECT_WORKER based on review comment on a patch 8.
(12,14) Remove CPU_HOT_EJECT_DATA fields Revision and Reserved.
Reorder CPU_HOT_EJECT_DATA to minimize internal padding
and ensure elements are properly aligned.
(13a,13b) Change CpuIndex->ApicId map to ProcessorNum -> QemuSelector
() Make CPU_HOT_EJECT_HANDLER->Handler,
CPU_HOT_EJECT_HANDLER->QemuSelectorMap volatile.
OvmfPkg/OvmfPkg.dec | 4 +++
OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 +++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c64a..9629707020ba 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -352,6 +352,10 @@ [PcdsDynamic, PcdsDynamicEx]
# This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below).
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE|BOOLEAN|0x34
+ ## This PCD adds a communication channel between OVMF's SmmCpuFeaturesLib
+ # instance in PiSmmCpuDxeSmm, and CpuHotplugSmm.
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0|UINT64|0x46
+
[PcdsFeatureFlag]
gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
diff --git a/OvmfPkg/Include/Pcd/CpuHotEjectData.h b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
new file mode 100644
index 000000000000..024a92726869
--- /dev/null
+++ b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
@@ -0,0 +1,52 @@
+/** @file
+ Definition for the CPU_HOT_EJECT_DATA structure, which shares
+ CPU hot-eject state between OVMF's SmmCpuFeaturesLib instance in
+ PiSmmCpuDxeSmm, and CpuHotplugSmm.
+
+ CPU_HOT_EJECT_DATA is allocated in SMRAM, and pointed-to by
+ PcdCpuHotEjectDataAddress.
+
+ PcdCpuHotEjectDataAddress is valid when SMM_REQUIRE is TRUE
+ and MaxNumberOfCpus > 1.
+
+ Copyright (C) 2021, Oracle Corporation.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef CPU_HOT_EJECT_DATA_H_
+#define CPU_HOT_EJECT_DATA_H_
+
+/**
+ CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+ on each CPU at exit from SMM.
+
+ @param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
+ and will be used as an index into
+ CPU_HOT_EJECT_DATA->QemuSelectorMap. It is
+ identical to the processor handle in
+ EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+typedef
+VOID
+(EFIAPI *CPU_HOT_EJECT_HANDLER) (
+ IN UINTN ProcessorNum
+ );
+
+//
+// CPU_EJECT_QEMU_SELECTOR_INVALID marks CPUs not being ejected in
+// CPU_HOT_EJECT_DATA->QemuSelectorMap.
+//
+// QEMU CPU Selector is UINT32, so we choose an invalid value larger
+// than that type.
+//
+#define CPU_EJECT_QEMU_SELECTOR_INVALID (MAX_UINT64)
+
+typedef struct {
+ volatile UINT64 *QemuSelectorMap; // Maps ProcessorNum -> QemuSelector
+ // for pending hot-ejects
+ volatile CPU_HOT_EJECT_HANDLER Handler; // Handler to do the CPU ejection
+ UINT32 ArrayLength; // Entries in the QemuSelectorMap
+} CPU_HOT_EJECT_DATA;
+
+#endif // CPU_HOT_EJECT_DATA_H_
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (4 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler Ankur Arora
` (4 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Init CPU_HOT_EJECT_DATA, which will be used to share CPU ejection
state between SmmCpuFeaturesLib (via PiSmmCpuDxeSmm) and CpuHotPlugSmm.
The init happens via SmmCpuFeaturesSmmRelocationComplete(), and so it
will run as part of the PiSmmCpuDxeSmm entry point function,
PiCpuSmmEntry(). Once inited, CPU_HOT_EJECT_DATA is exposed via
PcdCpuHotEjectDataAddress.
The CPU hot-eject handler (CPU_HOT_EJECT_DATA->Handler) is setup when
there is an ejection request via CpuHotplugSmm.
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>
---
Notes:
Addresses the following review comments:
(1) Detail in commit message about context in which CPU_HOT_EJECT_DATA
is inited.
(2) Add in sorted order MemoryAllocationLib in LibraryClasses
(3) Sort added includes in SmmCpuFeaturesLib.c
(4a-4b) Fixup linkage directives for mCpuHotEjectData.
(5) s/CpuHotEjectData/mCpuHotEjectData/
(6,10a,10b) Remove dependence on PcdCpuHotPlugSupport
(7) Make the tense structure consistent in block comment for
InitCpuHotEject().
(8) s/SmmCpuFeaturesSmmInitHotEject/InitCpuHotEject/
(9) s/mMaxNumberOfCpus/MaxNumberOfCpus/
(11) Remove a bunch of obvious comments.
(14a,14b,14c) Use SafeUint functions and rework the allocation logic
so we can just use a single allocation.
(12) Remove the AllocatePool() cast.
(13) Use a CpuDeadLoop() in case of failure; albeit via a goto, not
inline.
(15) Initialize the mCpuHotEjectData->QemuSelectorMap locally.
(16) Fix indentation in PcdSet64S.
(17) Change the cast in PcdSet64S() to UINTN.
(18) Use RETURN_STATUS instead of EFI_STATUS.
(19,20) Move the Handler logic in SmmCpuFeaturesRendezvousExit() into
into a separate patch.
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +
.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 92 ++++++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 97a10afb6e27..8a426a4c10fb 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -30,9 +30,13 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
MemEncryptSevLib
+ MemoryAllocationLib
PcdLib
+ SafeIntLib
SmmServicesTableLib
UefiBootServicesTableLib
[Pcd]
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 7ef7ed98342e..adbfc90ad46e 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -11,10 +11,13 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemEncryptSevLib.h>
+#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
+#include <Library/SafeIntLib.h>
#include <Library/SmmCpuFeaturesLib.h>
#include <Library/SmmServicesTableLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Pcd/CpuHotEjectData.h>
#include <PiSmm.h>
#include <Register/Intel/SmramSaveStateMap.h>
#include <Register/QemuSmramSaveStateMap.h>
@@ -171,6 +174,92 @@ SmmCpuFeaturesHookReturnFromSmm (
return OriginalInstructionPointer;
}
+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData = NULL;
+
+/**
+ Initialize mCpuHotEjectData if PcdCpuMaxLogicalProcessorNumber > 1.
+
+ Also setup the corresponding PcdCpuHotEjectDataAddress.
+**/
+STATIC
+VOID
+InitCpuHotEjectData (
+ VOID
+ )
+{
+ UINTN ArrayLen;
+ UINTN BaseLen;
+ UINTN TotalLen;
+ UINT32 Idx;
+ UINT32 MaxNumberOfCpus;
+ RETURN_STATUS PcdStatus;
+
+ MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+
+ if (MaxNumberOfCpus == 1) {
+ return;
+ }
+
+ //
+ // We want the following lay out for CPU_HOT_EJECT_DATA:
+ // UINTN alignment: CPU_HOT_EJECT_DATA
+ // --- padding if needed ---
+ // UINT64 alignment: CPU_HOT_EJECT_DATA->QemuSelectorMap[]
+ //
+ // Accordingly, we allocate:
+ // sizeof(*mCpuHotEjectData) + (MaxNumberOfCpus *
+ // sizeof(mCpuHotEjectData->QemuSelectorMap[0])).
+ // Add sizeof(UINT64) to use as padding if needed.
+ //
+
+ if (RETURN_ERROR (SafeUintnMult (sizeof (*mCpuHotEjectData), 1, &BaseLen)) ||
+ RETURN_ERROR (SafeUintnMult (
+ sizeof (mCpuHotEjectData->QemuSelectorMap[0]),
+ MaxNumberOfCpus, &ArrayLen)) ||
+ RETURN_ERROR (SafeUintnAdd (BaseLen, ArrayLen, &TotalLen))||
+ RETURN_ERROR (SafeUintnAdd (TotalLen, sizeof (UINT64), &TotalLen))) {
+ DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_EJECT_DATA\n", __FUNCTION__));
+ goto Fatal;
+ }
+
+ mCpuHotEjectData = AllocatePool (TotalLen);
+ if (mCpuHotEjectData == NULL) {
+ ASSERT (mCpuHotEjectData != NULL);
+ goto Fatal;
+ }
+
+ mCpuHotEjectData->Handler = NULL;
+ mCpuHotEjectData->ArrayLength = MaxNumberOfCpus;
+
+ mCpuHotEjectData->QemuSelectorMap = (void *)mCpuHotEjectData +
+ sizeof (*mCpuHotEjectData);
+ mCpuHotEjectData->QemuSelectorMap =
+ (void *)ALIGN_VALUE ((UINTN)mCpuHotEjectData->QemuSelectorMap,
+ sizeof (UINT64));
+ //
+ // We use mCpuHotEjectData->QemuSelectorMap to map
+ // ProcessorNum -> QemuSelector. Initialize to invalid values.
+ //
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
+ mCpuHotEjectData->QemuSelectorMap[Idx] = CPU_EJECT_QEMU_SELECTOR_INVALID;
+ }
+
+ //
+ // Expose address of CPU Hot eject Data structure
+ //
+ PcdStatus = PcdSet64S (PcdCpuHotEjectDataAddress,
+ (UINTN)(VOID *)mCpuHotEjectData);
+ if (RETURN_ERROR (PcdStatus)) {
+ ASSERT_EFI_ERROR (PcdStatus);
+ goto Fatal;
+ }
+
+ return;
+
+Fatal:
+ CpuDeadLoop ();
+}
+
/**
Hook point in normal execution mode that allows the one CPU that was elected
as monarch during System Management Mode initialization to perform additional
@@ -188,6 +277,9 @@ SmmCpuFeaturesSmmRelocationComplete (
UINTN MapPagesBase;
UINTN MapPagesCount;
+
+ InitCpuHotEjectData ();
+
if (!MemEncryptSevIsEnabled ()) {
return;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (5 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu() Ankur Arora
` (3 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Call the CPU hot-eject handler if one is installed. The condition for
installation is (PcdCpuMaxLogicalProcessorNumber > 1), and there's
a hot-unplug request.
The handler executes in context of SmmCpuFeaturesRendezvousExit(),
which is called at the tail end of SmiRendezvous() after the BSP has
given the signal to exit via the "AllCpusInSync" loop.
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>
---
Notes:
Address the following review comments from v6, patch-6:
(19a) Move the call to the ejection handler to a separate patch.
(19b) Describe the calling context of SmmCpuFeaturesRendezvousExit().
(20) Add comment describing the state when the Handler is not armed.
OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index adbfc90ad46e..c5cfe2610c23 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -467,6 +467,19 @@ SmmCpuFeaturesRendezvousExit (
IN UINTN CpuIndex
)
{
+ //
+ // We only call the Handler if CPU hot-eject is enabled
+ // (PcdCpuMaxLogicalProcessorNumber > 1), and hot-eject is needed
+ // in this SMI exit (otherwise mCpuHotEjectData->Handler is not armed.)
+ //
+
+ if (mCpuHotEjectData != NULL) {
+ CPU_HOT_EJECT_HANDLER Handler = mCpuHotEjectData->Handler;
+
+ if (Handler != NULL) {
+ Handler (CpuIndex);
+ }
+ }
}
/**
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu()
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (6 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject Ankur Arora
` (2 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Add EjectCpu(), which handles the CPU ejection, and provides a holding
area for said CPUs. It is called via SmmCpuFeaturesRendezvousExit(),
at the tail end of the SMI handling.
Also UnplugCpus() now stashes QEMU Selectors of CPUs which need to be
ejected in CPU_HOT_EJECT_DATA.QemuSelectorMap. This is used by
EjectCpu() to identify CPUs marked for ejection.
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>
---
Notes:
Address these review comments from v6:
(1) s/CpuEject/EjectCpu/g
(2) Ensure that the added include is in sorted order.
(3) Switch to a cheaper CpuSleep() based loop instead of
CpuDeadLoop(). Also add the CpuLib LibraryClass.
(4) Remove the nested else clause
(5) Use Laszlo's much clearer comment when we try to map multiple
QemuSelector to the same ProcessorNum.
(6a) Fix indentation of the debug print in the block in (5).
(6b,6c,6d) Fix printf types for ProcessorNum, use FMT_APIC_ID for
APIC_ID and 0x%Lx for QemuSelector[].
() As discussed elsewhere add an DEBUG_INFO print logging the
correspondence between ProcessorNum, APIC_ID, QemuSelector.
(7a,7b) Use EFI_ALREADY_STARTED instead of EFI_INVALID_PARAMETER and
document it in the UnplugCpus() comment block.
() As discussed elsewhere, add the import statement for
PcdCpuHotEjectDataAddress.
(9) Use Laszlo's comment in the PcdGet64(PcdCpuHotEjectDataAddress)
description block.
(10) Change mCpuHotEjectData init state checks from ASSERT to ones
consistent with similar checks for mCpuHotPlugData.
(11-14) Get rid of mCpuHotEjectData init loop: moved to a prior
patch so it can be done at allocation time.
(15) s/SmmCpuFeaturesSmiRendezvousExit/SmmCpuFeaturesRendezvousExit/
(16,17) Document the ordering requirements of
mCpuHotEjectData->Handler, and mCpuHotEjectData->QemuSelectorMap.
Not addressed:
(8) Not removing the EjectCount variable as I'd like to minimize
stores/loads to CPU_HOT_EJECT_DATA->Handler and so would like to do this
a single time at the end of the iteration. (It is safe to write multiple
times to the handler in UnplugCpus() but given the ordering concerns
around it, it seems cleaner to not access it unnecessarily.)
OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 157 ++++++++++++++++++++++++++++++--
2 files changed, 151 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
index 04322b0d7855..ebcc7e2ac63a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
@@ -40,6 +40,7 @@ [Packages]
[LibraryClasses]
BaseLib
BaseMemoryLib
+ CpuLib
DebugLib
LocalApicLib
MmServicesTableLib
@@ -54,6 +55,7 @@ [Protocols]
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress ## CONSUMES
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress ## CONSUMES
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ## CONSUMES
[FeaturePcd]
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index f07b5072749a..5dcaca53d42f 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -10,10 +10,12 @@
#include <IndustryStandard/Q35MchIch9.h> // ICH9_APM_CNT
#include <IndustryStandard/QemuCpuHotplug.h> // QEMU_CPUHP_CMD_GET_PENDING
#include <Library/BaseLib.h> // CpuDeadLoop()
+#include <Library/CpuLib.h> // CpuSleep()
#include <Library/DebugLib.h> // ASSERT()
#include <Library/MmServicesTableLib.h> // gMmst
#include <Library/PcdLib.h> // PcdGetBool()
#include <Library/SafeIntLib.h> // SafeUintnSub()
+#include <Pcd/CpuHotEjectData.h> // CPU_HOT_EJECT_DATA
#include <Protocol/MmCpuIo.h> // EFI_MM_CPU_IO_PROTOCOL
#include <Protocol/SmmCpuService.h> // EFI_SMM_CPU_SERVICE_PROTOCOL
#include <Uefi/UefiBaseType.h> // EFI_STATUS
@@ -32,11 +34,12 @@ STATIC EFI_MM_CPU_IO_PROTOCOL *mMmCpuIo;
//
STATIC EFI_SMM_CPU_SERVICE_PROTOCOL *mMmCpuService;
//
-// This structure is a communication side-channel between the
+// These structures serve as communication side-channels between the
// EFI_SMM_CPU_SERVICE_PROTOCOL consumer (i.e., this driver) and provider
// (i.e., PiSmmCpuDxeSmm).
//
STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData;
//
// SMRAM arrays for fetching the APIC IDs of processors with pending events (of
// known event types), for the time of just one MMI.
@@ -188,18 +191,72 @@ RevokeNewSlot:
}
/**
+ CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+ on each CPU at exit from SMM.
+
+ If, the executing CPU is not being ejected, nothing to be done.
+ If, the executing CPU is being ejected, wait in a halted loop
+ until ejected.
+
+ @param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
+ and will be used as an index into
+ CPU_HOT_EJECT_DATA->QemuSelectorMap. It is
+ identical to the processor handle number in
+ EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+VOID
+EFIAPI
+EjectCpu (
+ IN UINTN ProcessorNum
+ )
+{
+ UINT64 QemuSelector;
+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum];
+ if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ return;
+ }
+
+ //
+ // CPU(s) being unplugged get here from SmmCpuFeaturesRendezvousExit()
+ // after having been cleared to exit the SMI by the BSP and thus have
+ // no SMM processing remaining.
+ //
+ // Given that we cannot allow them to escape to the guest, we pen them
+ // here until the BSP tells QEMU to unplug them.
+ //
+ for (;;) {
+ DisableInterrupts ();
+ CpuSleep ();
+ }
+}
+
+/**
Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
For each such CPU, report the CPU to PiSmmCpuDxeSmm via
- EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
- unknown, skip it silently.
+ EFI_SMM_CPU_SERVICE_PROTOCOL and stash the APIC ID for later ejection.
+ If the to be hot-unplugged CPU is unknown, skip it silently.
+
+ Additonally, if we do stash any APIC IDs, also install a CPU eject handler
+ which would handle the ejection.
@param[in] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
hot-unplugged.
+ @param[in] ToUnplugSelector The QEMU Selectors of the CPUs that are about to
+ be hot-unplugged.
+
@param[in] ToUnplugCount The number of filled-in APIC IDs in
ToUnplugApicIds.
+ @retval EFI_ALREADY_STARTED For the ProcessorNum that
+ EFI_SMM_CPU_SERVICE_PROTOCOL had assigned to
+ one of the APIC ID in ToUnplugApicIds,
+ mCpuHotEjectData->QemuSelectorMap already has
+ the QemuSelector value stashed. (This should
+ never happen.)
+
@retval EFI_SUCCESS Known APIC IDs have been removed from SMM data
structures.
@@ -210,23 +267,36 @@ STATIC
EFI_STATUS
UnplugCpus (
IN APIC_ID *ToUnplugApicIds,
+ IN UINT32 *ToUnplugSelector,
IN UINT32 ToUnplugCount
)
{
EFI_STATUS Status;
UINT32 ToUnplugIdx;
+ UINT32 EjectCount;
UINTN ProcessorNum;
ToUnplugIdx = 0;
+ EjectCount = 0;
while (ToUnplugIdx < ToUnplugCount) {
APIC_ID RemoveApicId;
+ UINT32 QemuSelector;
RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+ QemuSelector = ToUnplugSelector[ToUnplugIdx];
//
- // mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
- // the ProcessorNum for the APIC ID to be removed.
+ // mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use RemoveApicId
+ // to find the corresponding ProcessorNum for the CPU to be removed.
//
+ // With this we can establish a 3 way mapping:
+ // APIC_ID -- ProcessorNum -- QemuSelector
+ //
+ // We stash the ProcessorNum -> QemuSelector mapping so it can later be
+ // used for CPU hot-eject in SmmCpuFeaturesRendezvousExit() context (where
+ // we only have ProcessorNum available.)
+ //
+
for (ProcessorNum = 0;
ProcessorNum < mCpuHotPlugData->ArrayLength;
ProcessorNum++) {
@@ -255,11 +325,64 @@ UnplugCpus (
return Status;
}
+ if (mCpuHotEjectData->QemuSelectorMap[ProcessorNum] !=
+ CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ //
+ // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is set to
+ // CPU_EJECT_QEMU_SELECTOR_INVALID when mCpuHotEjectData->QemuSelectorMap
+ // is allocated, and once the subject processsor is ejected.
+ //
+ // Additionally, mMmCpuService->RemoveProcessor(ProcessorNum) invalidates
+ // mCpuHotPlugData->ApicId[ProcessorNum], so a given ProcessorNum can
+ // never match more than one APIC ID and by transitivity, more than one
+ // QemuSelector in a single invocation of UnplugCpus().
+ //
+ DEBUG ((DEBUG_ERROR, "%a: ProcessorNum %Lu maps to QemuSelector 0x%Lx, "
+ "cannot also map to 0x%Lx\n", __FUNCTION__, (UINT64)ProcessorNum,
+ (UINT64)mCpuHotEjectData->QemuSelectorMap[ProcessorNum], QemuSelector));
+
+ Status = EFI_ALREADY_STARTED;
+ return Status;
+ }
+
+ //
+ // Stash the QemuSelector so we can do the actual ejection later.
+ //
+ mCpuHotEjectData->QemuSelectorMap[ProcessorNum] = (UINT64)QemuSelector;
+
+ DEBUG ((DEBUG_INFO, "%a: Started hot-unplug on ProcessorNum %Lu, APIC ID "
+ FMT_APIC_ID ", QemuSelector 0x%Lx\n", __FUNCTION__, (UINT64)ProcessorNum,
+ RemoveApicId, mCpuHotEjectData->QemuSelectorMap[ProcessorNum]));
+
+ EjectCount++;
ToUnplugIdx++;
}
+ if (EjectCount != 0) {
+ //
+ // We have processors to be ejected; install the handler.
+ //
+ mCpuHotEjectData->Handler = EjectCpu;
+
+ //
+ // The BSP, CPUs to be ejected dereference mCpuHotEjectData->Handler, and
+ // mCpuHotEjectData->QemuSelectorMap[] in SmmCpuFeaturesRendezvousExit().
+ //
+ // Assignments to both of these are ordered-before the BSP's SMI exit signal
+ // which happens via a write to SMM_DISPATCHER_MP_SYNC_DATA->AllCpusInSync.
+ // Dereferences of both are ordered-after the synchronization via
+ // "AllCpusInSync".
+ //
+ // So we are guaranteed that the Handler would see the assignments above.
+ // However, add a MemoryFence() here in-lieu of a compiler barrier to
+ // ensure that the compiler doesn't monkey around with the stores.
+ //
+ MemoryFence ();
+ }
+
//
- // We've removed this set of APIC IDs from SMM data structures.
+ // We've removed this set of APIC IDs from SMM data structures and
+ // have installed an ejection handler if needed.
//
return EFI_SUCCESS;
}
@@ -387,7 +510,7 @@ CpuHotplugMmi (
}
if (ToUnplugCount > 0) {
- Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+ Status = UnplugCpus (mToUnplugApicIds, mToUnplugSelector, ToUnplugCount);
if (EFI_ERROR (Status)) {
goto Fatal;
}
@@ -458,9 +581,14 @@ CpuHotplugEntry (
//
// Our DEPEX on EFI_SMM_CPU_SERVICE_PROTOCOL guarantees that PiSmmCpuDxeSmm
- // has pointed PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM.
+ // has pointed:
+ // - PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM,
+ // - PcdCpuHotEjectDataAddress to CPU_HOT_EJECT_DATA in SMRAM, if the
+ // possible CPU count is greater than 1.
//
mCpuHotPlugData = (VOID *)(UINTN)PcdGet64 (PcdCpuHotPlugDataAddress);
+ mCpuHotEjectData = (VOID *)(UINTN)PcdGet64 (PcdCpuHotEjectDataAddress);
+
if (mCpuHotPlugData == NULL) {
Status = EFI_NOT_FOUND;
DEBUG ((DEBUG_ERROR, "%a: CPU_HOT_PLUG_DATA: %r\n", __FUNCTION__, Status));
@@ -472,6 +600,19 @@ CpuHotplugEntry (
if (mCpuHotPlugData->ArrayLength == 1) {
return EFI_UNSUPPORTED;
}
+
+ if (mCpuHotEjectData == NULL) {
+ Status = EFI_NOT_FOUND;
+ } else if (mCpuHotPlugData->ArrayLength != mCpuHotEjectData->ArrayLength) {
+ Status = EFI_INVALID_PARAMETER;
+ } else {
+ Status = EFI_SUCCESS;
+ }
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: CPU_HOT_EJECT_DATA: %r\n", __FUNCTION__, Status));
+ goto Fatal;
+ }
+
//
// Allocate the data structures that depend on the possible CPU count.
//
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (7 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu() Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 9:04 ` [PATCH v7 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug Ankur Arora
2021-02-19 22:12 ` [edk2-devel] [PATCH v7 00/10] support " Laszlo Ersek
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Add logic in EjectCpu() to do the actual the CPU ejection.
On the BSP, ejection happens by first selecting the CPU via
its QemuSelector and then sending the QEMU "eject" command.
QEMU in-turn signals the remote VCPU thread which context-switches
the CPU out of the SMI handler.
Meanwhile the CPU being ejected, waits around in its holding
area until it is context-switched out. Note that it is possible
that a slow CPU gets ejected before it reaches the wait loop.
However, this would never happen before it has executed the
"AllCpusInSync" loop in SmiRendezvous().
It can mean that an ejected CPU does not execute code after
that point but given that the CPU state will be destroyed by
QEMU, the missed cleanup is no great loss.
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>
---
Notes:
Addresses the following reviewing comments from v6:
(1) s/CpuEject/EjectCpu/g
(2,2a,2c) Get rid of eject-worker and related.
(2b,2d) Use the PlatformSmmBspElection() logic to find out IsBSP.
(3,3b) Use CPU_HOT_EJECT_DATA->QemuSelector instead of ApicIdMap to
do the actual ejection.
(4,5a,5b) Fix the format etc in the final unplugged log message
() Also as discussed elsewhere document the ordering requirements for
mCpuHotEjectData->QemuSelector[] and mCpuHotEjectData->Handler.
() [from patch 2] Move definition of QEMU_CPUHP_STAT_EJECTED to this
patch.
() s/QEMU_CPUHP_STAT_EJECTED/QEMU_CPUHP_STAT_EJECT/
OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 122 +++++++++++++++++++--
.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 31 ++++++
3 files changed, 147 insertions(+), 7 deletions(-)
diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index 2ec7a107a64d..d0e83102c13f 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
#define QEMU_CPUHP_STAT_ENABLED BIT0
#define QEMU_CPUHP_STAT_INSERT BIT1
#define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_EJECT BIT3
#define QEMU_CPUHP_STAT_FW_REMOVE BIT4
#define QEMU_CPUHP_RW_CMD_DATA 0x8
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 5dcaca53d42f..b80d4a2283a4 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -18,6 +18,7 @@
#include <Pcd/CpuHotEjectData.h> // CPU_HOT_EJECT_DATA
#include <Protocol/MmCpuIo.h> // EFI_MM_CPU_IO_PROTOCOL
#include <Protocol/SmmCpuService.h> // EFI_SMM_CPU_SERVICE_PROTOCOL
+#include <Register/Intel/ArchitecturalMsr.h> // MSR_IA32_APIC_BASE_REGISTER
#include <Uefi/UefiBaseType.h> // EFI_STATUS
#include "ApicId.h" // APIC_ID
@@ -191,12 +192,36 @@ RevokeNewSlot:
}
/**
+ EjectCpu needs to know the BSP at SMI exit at a point when
+ some of the EFI_SMM_CPU_SERVICE_PROTOCOL state has been torn
+ down.
+ Reuse the logic from OvmfPkg::PlatformSmmBspElection() to
+ do that.
+**/
+STATIC
+BOOLEAN
+CheckIfBsp (
+ IN UINTN ProcessorNum
+ )
+{
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
+ BOOLEAN IsBsp;
+
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ IsBsp = (BOOLEAN)(ApicBaseMsr.Bits.BSP == 1);
+ return IsBsp;
+}
+
+/**
CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
on each CPU at exit from SMM.
- If, the executing CPU is not being ejected, nothing to be done.
+ If, the executing CPU is neither the BSP, nor being ejected, nothing
+ to be done.
If, the executing CPU is being ejected, wait in a halted loop
until ejected.
+ If, the executing CPU is the BSP, set QEMU CPU status to eject
+ for CPUs being ejected.
@param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
and will be used as an index into
@@ -211,9 +236,97 @@ EjectCpu (
)
{
UINT64 QemuSelector;
+ BOOLEAN IsBsp = CheckIfBsp (ProcessorNum);
+ //
+ // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is updated
+ // on the BSP in the ongoing SMI iteration at two places:
+ //
+ // - UnplugCpus() where the BSP determines if a CPU is under ejection
+ // or not. As the comment where mCpuHotEjectData->Handler is set-up
+ // describes any such updates are guaranteed to be ordered-before the
+ // dereference below.
+ //
+ // - EjectCpu() on the BSP updates QemuSelectorMap[ProcessorNum] for
+ // CPUs after they have been hot-ejected.
+ //
+ // The CPU under ejection: might be executing anywhere between the
+ // "AllCpusInSync" exit loop in SmiRendezvous() to about to
+ // dereference QemuSelectorMap[ProcessorNum].
+ // Given that the BSP ensures that this store only happens after the
+ // CPU has been ejected, this CPU would never see the after value.
+ // (Note that any CPU that is already executing the CpuSleep() loop
+ // below never raced any updates and always saw the before value.)
+ //
+ // CPUs not-under ejection: never see any changes so they are fine.
+ //
+ // Lastly, note that we are also guaranteed that any dereferencing
+ // CPU only sees the before or after value and not an intermediate
+ // value. This is because QemuSelectorMap[ProcessorNum] is aligned at
+ // a natural boundary.
+ //
QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum];
- if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID && !IsBsp) {
+ return;
+ }
+
+ if (IsBsp) {
+ UINT32 Idx;
+
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
+ UINT64 QemuSelector;
+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[Idx];
+
+ if (QemuSelector != CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ //
+ // This to-be-ejected-CPU has already received the BSP's SMI exit
+ // signal and, will execute SmmCpuFeaturesRendezvousExit()
+ // followed by this callback or is already waiting in the
+ // CpuSleep() loop below.
+ //
+ // Tell QEMU to context-switch it out.
+ //
+ QemuCpuhpWriteCpuSelector (mMmCpuIo, (UINT32) QemuSelector);
+ QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECT);
+
+ //
+ // We need a compiler barrier here to ensure that the compiler
+ // does not reorder the CpuStatus and QemuSelectorMap[Idx] stores.
+ //
+ // A store fence is not strictly necessary on x86 which has
+ // TSO; however, both of these stores are in different address spaces
+ // so also add a Store Fence here.
+ //
+ MemoryFence ();
+
+ //
+ // Clear the eject status for this CPU Idx to ensure that an invalid
+ // SMI later does not end up trying to eject it or a newly
+ // hotplugged CPU Idx does not go into the dead loop.
+ //
+ mCpuHotEjectData->QemuSelectorMap[Idx] =
+ CPU_EJECT_QEMU_SELECTOR_INVALID;
+
+ DEBUG ((DEBUG_INFO, "%a: Unplugged ProcessorNum %u, "
+ "QemuSelector 0x%Lx\n", __FUNCTION__, Idx, QemuSelector));
+ }
+ }
+
+ //
+ // We are done until the next hot-unplug; clear the handler.
+ //
+ // By virtue of the MemoryFence() in the ejection loop above, the
+ // following store is ordered-after all the ejections are done.
+ // (We know that there is at least one CPU hot-eject handler if this
+ // handler was installed.)
+ //
+ // As described in OvmfPkg::SmmCpuFeaturesRendezvousExit() this
+ // means that the only CPUs which might dereference
+ // mCpuHotEjectData->Handler are not under ejection, so we can
+ // safely reset.
+ //
+ mCpuHotEjectData->Handler = NULL;
return;
}
@@ -496,11 +609,6 @@ CpuHotplugMmi (
if (EFI_ERROR (Status)) {
goto Fatal;
}
- if (ToUnplugCount > 0) {
- DEBUG ((DEBUG_ERROR, "%a: hot-unplug is not supported yet\n",
- __FUNCTION__));
- goto Fatal;
- }
if (PluggedCount > 0) {
Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount);
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index c5cfe2610c23..0345466ac5a5 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -472,6 +472,37 @@ SmmCpuFeaturesRendezvousExit (
// (PcdCpuMaxLogicalProcessorNumber > 1), and hot-eject is needed
// in this SMI exit (otherwise mCpuHotEjectData->Handler is not armed.)
//
+ // mCpuHotEjectData itself is stable once setup so it can be
+ // dereferenced without needing any synchronization,
+ // but, mCpuHotEjectData->Handler is updated on the BSP in the
+ // ongoing SMI iteration at two places:
+ //
+ // - UnplugCpus() where the BSP determines if a CPU is under ejection
+ // or not. As the comment where mCpuHotEjectData->Handler is set-up
+ // describes any such updates are guaranteed to be ordered-before the
+ // dereference below.
+ //
+ // - EjectCpu() (which is called via the Handler below), on the BSP
+ // updates mCpuHotEjectData->Handler once it is done with all ejections.
+ //
+ // The CPU under ejection: might be executing anywhere between the
+ // "AllCpusInSync" exit loop in SmiRendezvous() to about to
+ // dereference the Handler field.
+ // Given that the BSP ensures that this store only happens after all
+ // CPUs under ejection have been ejected, this CPU would never see
+ // the after value.
+ // (Note that any CPU that is already executing the CpuSleep() loop
+ // below never raced any updates and always saw the before value.)
+ //
+ // CPUs not-under ejection: might see either value of the Handler
+ // which is fine, because the Handler is a NOP for CPUs not-under
+ // ejection.
+ //
+ // Lastly, note that we are also guaranteed that any dereferencing
+ // CPU only sees the before or after value and not an intermediate
+ // value. This is because mCpuHotEjectData->Handler is aligned at a
+ // natural boundary.
+ //
if (mCpuHotEjectData != NULL) {
CPU_HOT_EJECT_HANDLER Handler = mCpuHotEjectData->Handler;
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (8 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject Ankur Arora
@ 2021-02-19 9:04 ` Ankur Arora
2021-02-19 22:12 ` [edk2-devel] [PATCH v7 00/10] support " Laszlo Ersek
10 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 9:04 UTC (permalink / raw)
To: devel
Cc: lersek, imammedo, boris.ostrovsky, Ankur Arora, Jordan Justen,
Ard Biesheuvel, Aaron Young
Advertise OVMF support for CPU hot-unplug and negotiate it
if QEMU requests the feature.
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>
---
Notes:
Addresses the following review comments:
(1,3) s/hot unplug/hot-unplug/
(2) Get rid of the reference to the made up ICH9_APM_CNT_CPU_HOT_UNPLUG
(4,6) Remove the artificial tie in between
ICH9_LPC_SMI_F_CPU_HOTPLUG, ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
(5) Fully spell out "SMI on CPU hot-unplug".
(7) Emit separate messages on negotiation (or not) of
ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index c9d875543205..b1d59a559dae 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -29,6 +29,12 @@
//
#define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
+// The following bit value stands for "enable CPU hot-unplug, and inject an SMI
+// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
+// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
+//
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
+
//
// Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
// for the S3 boot script fragment to write to and read from.
@@ -112,7 +118,8 @@ NegotiateSmiFeatures (
QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);
//
- // We want broadcast SMI, SMI on CPU hotplug, and nothing else.
+ // We want broadcast SMI, SMI on CPU hotplug, SMI on CPU hot-unplug
+ // and nothing else.
//
RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST;
if (!MemEncryptSevIsEnabled ()) {
@@ -120,8 +127,10 @@ NegotiateSmiFeatures (
// For now, we only support hotplug with SEV disabled.
//
RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG;
+ RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG;
}
mSmiFeatures &= RequestedFeaturesMask;
+
QemuFwCfgSelectItem (mRequestedFeaturesItem);
QemuFwCfgWriteBytes (sizeof mSmiFeatures, &mSmiFeatures);
@@ -166,6 +175,13 @@ NegotiateSmiFeatures (
__FUNCTION__));
}
+ if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) == 0) {
+ DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug not negotiated\n", __FUNCTION__));
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug with SMI negotiated\n",
+ __FUNCTION__));
+ }
+
//
// Negotiation successful (although we may not have gotten the optimal
// feature set).
--
2.9.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v7 00/10] support CPU hot-unplug
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
` (9 preceding siblings ...)
2021-02-19 9:04 ` [PATCH v7 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug Ankur Arora
@ 2021-02-19 22:12 ` Laszlo Ersek
2021-02-19 22:19 ` Ankur Arora
10 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2021-02-19 22:12 UTC (permalink / raw)
To: devel, ankur.a.arora; +Cc: imammedo, boris.ostrovsky
Hi Ankur,
On 02/19/21 10:04, Ankur Arora wrote:
> Hi,
>
> This series adds OVMF support for CPU hot-unplug.
>
> QEMU secureboot hot-unplug logic corresponding to this is in upstream.
> Also posted here:
> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/
>
> Testing (with QEMU 5.2.50):
> - Stable with randomized CPU plug/unplug (guest maxcpus=33,128)
> - Synthetic tests with simultaneous multi CPU hot-unplug
>
> Also at:
> github.com/terminus/edk2/ hot-unplug-v7
>
> Changelog:
>
> v7:
> - Address review comments from v6.
> - Fix ejection bug where we were using APIC ID to do the ejection
> rather than the Qemu Selector.
> - Describes safety properties and ordering needed for concurrent
> accesses to CPU_HOT_EJECT_DATA->QemuSelectorMap, and
> CPU_HOT_EJECT_DATA->Handler,
I plan to review this series early next week.
This work is now going to miss the upcoming stable tag (soft feature
freeze is 2021-02-22 00:00:00 UTC-8), but we can hopefully merge it
early in the next development cycle.
Meanwhile, based on my github notifications folder, I believe you may
not have put this version of the series through CI -- if that's correct,
then please consider doing it.
Thanks!
Laszlo
>
> v6:
> - addresses v5 review comments.
> URL: https://patchew.org/EDK2/20210129005950.467638-1-ankur.a.arora@oracle.com/
>
> v5:
> - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
> add Qemu Cpu Status helper").
> URL: https://patchew.org/EDK2/20210126064440.299596-1-ankur.a.arora@oracle.com/
>
> v4:
> - Gets rid of unnecessary UefiCpuPkg changes
> URL: https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.arora@oracle.com/
>
> v3:
> - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
> and OvmfPkg/CpuHotplugSmm
> - Cleaner split of the hot-unplug code
> URL: https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.arora@oracle.com/
>
> v2:
> - Do the ejection via SmmCpuFeaturesRendezvousExit()
> URL: https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.arora@oracle.com/
>
> RFC:
> URL: https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.arora@oracle.com/
>
> Please review.
>
> Thanks
> Ankur
>
> Ankur Arora (10):
> OvmfPkg/CpuHotplugSmm: refactor hotplug logic
> OvmfPkg/CpuHotplugSmm: collect hot-unplug events
> OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
> OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
> OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
> OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
> OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
> OvmfPkg/CpuHotplugSmm: add EjectCpu()
> OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
> OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
>
> OvmfPkg/OvmfPkg.dec | 4 +
> OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +
> OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 7 +
> OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 2 +
> OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 ++
> OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 578 +++++++++++++++++----
> OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 106 +++-
> .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 136 +++++
> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
> 10 files changed, 789 insertions(+), 120 deletions(-)
> create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v7 00/10] support CPU hot-unplug
2021-02-19 22:12 ` [edk2-devel] [PATCH v7 00/10] support " Laszlo Ersek
@ 2021-02-19 22:19 ` Ankur Arora
0 siblings, 0 replies; 14+ messages in thread
From: Ankur Arora @ 2021-02-19 22:19 UTC (permalink / raw)
To: Laszlo Ersek, devel; +Cc: imammedo, boris.ostrovsky
On 2021-02-19 2:12 p.m., Laszlo Ersek wrote:
> Hi Ankur,
>
> On 02/19/21 10:04, Ankur Arora wrote:
>> Hi,
>>
>> This series adds OVMF support for CPU hot-unplug.
>>
>> QEMU secureboot hot-unplug logic corresponding to this is in upstream.
>> Also posted here:
>> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/
>>
>> Testing (with QEMU 5.2.50):
>> - Stable with randomized CPU plug/unplug (guest maxcpus=33,128)
>> - Synthetic tests with simultaneous multi CPU hot-unplug
>>
>> Also at:
>> github.com/terminus/edk2/ hot-unplug-v7
>>
>> Changelog:
>>
>> v7:
>> - Address review comments from v6.
>> - Fix ejection bug where we were using APIC ID to do the ejection
>> rather than the Qemu Selector.
>> - Describes safety properties and ordering needed for concurrent
>> accesses to CPU_HOT_EJECT_DATA->QemuSelectorMap, and
>> CPU_HOT_EJECT_DATA->Handler,
>
> I plan to review this series early next week.
Thanks.
> This work is now going to miss the upcoming stable tag (soft feature
> freeze is 2021-02-22 00:00:00 UTC-8), but we can hopefully merge it
> early in the next development cycle.
Yeah, I did get a little late with v7.
>
> Meanwhile, based on my github notifications folder, I believe you may
> not have put this version of the series through CI -- if that's correct,
> then please consider doing it.
I hadn't. Will do.
Thanks
Ankur
>
> Thanks!
> Laszlo
>
>>
>> v6:
>> - addresses v5 review comments.
>> URL: https://patchew.org/EDK2/20210129005950.467638-1-ankur.a.arora@oracle.com/
>>
>> v5:
>> - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
>> add Qemu Cpu Status helper").
>> URL: https://patchew.org/EDK2/20210126064440.299596-1-ankur.a.arora@oracle.com/
>>
>> v4:
>> - Gets rid of unnecessary UefiCpuPkg changes
>> URL: https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.arora@oracle.com/
>>
>> v3:
>> - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
>> and OvmfPkg/CpuHotplugSmm
>> - Cleaner split of the hot-unplug code
>> URL: https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.arora@oracle.com/
>>
>> v2:
>> - Do the ejection via SmmCpuFeaturesRendezvousExit()
>> URL: https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.arora@oracle.com/
>>
>> RFC:
>> URL: https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.arora@oracle.com/
>>
>> Please review.
>>
>> Thanks
>> Ankur
>>
>> Ankur Arora (10):
>> OvmfPkg/CpuHotplugSmm: refactor hotplug logic
>> OvmfPkg/CpuHotplugSmm: collect hot-unplug events
>> OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
>> OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
>> OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
>> OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
>> OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
>> OvmfPkg/CpuHotplugSmm: add EjectCpu()
>> OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
>> OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
>>
>> OvmfPkg/OvmfPkg.dec | 4 +
>> OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
>> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +
>> OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 7 +
>> OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 2 +
>> OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 ++
>> OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 578 +++++++++++++++++----
>> OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 106 +++-
>> .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 136 +++++
>> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
>> 10 files changed, 789 insertions(+), 120 deletions(-)
>> create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic
2021-02-19 9:04 ` [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic Ankur Arora
@ 2021-02-22 10:00 ` Laszlo Ersek
0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-02-22 10:00 UTC (permalink / raw)
To: devel, ankur.a.arora
Cc: imammedo, boris.ostrovsky, Jordan Justen, Ard Biesheuvel,
Aaron Young
On 02/19/21 10:04, Ankur Arora wrote:
> Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into
> ProcessHotAddedCpus(). This is in preparation for supporting CPU
> hot-unplug.
>
> 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>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Addresses these review comments from v6:
> (1) s/EFI_ERROR(/EFI_ERROR (/
> (2) Remove the empty line in the comment block above
> ProcessHotAddedCpus().
> () Nest the EFI_ERROR handling inside the (PluggedCount > 0) clause.
>
> OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 210 ++++++++++++++++++++++---------------
> 1 file changed, 126 insertions(+), 84 deletions(-)
LGTM, thanks
Laszlo
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-22 10:01 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-19 9:04 [PATCH v7 00/10] support CPU hot-unplug Ankur Arora
2021-02-19 9:04 ` [PATCH v7 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic Ankur Arora
2021-02-22 10:00 ` [edk2-devel] " Laszlo Ersek
2021-02-19 9:04 ` [PATCH v7 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events Ankur Arora
2021-02-19 9:04 ` [PATCH v7 03/10] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper Ankur Arora
2021-02-19 9:04 ` [PATCH v7 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus() Ankur Arora
2021-02-19 9:04 ` [PATCH v7 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA Ankur Arora
2021-02-19 9:04 ` [PATCH v7 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state Ankur Arora
2021-02-19 9:04 ` [PATCH v7 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler Ankur Arora
2021-02-19 9:04 ` [PATCH v7 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu() Ankur Arora
2021-02-19 9:04 ` [PATCH v7 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject Ankur Arora
2021-02-19 9:04 ` [PATCH v7 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug Ankur Arora
2021-02-19 22:12 ` [edk2-devel] [PATCH v7 00/10] support " Laszlo Ersek
2021-02-19 22:19 ` Ankur Arora
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox