Re: [edk2-devel] [PATCH v9 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu()
On 03/12/21 07:26, Ankur Arora wrote: > 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 > 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 > --- > > Notes: > Addresses the following comments from v8: > > (1) Fixup the coment about UnplugCpus() to reference stashing QEMU > Cpu Selectors instead of APIC IDs. > (2) s/ToUnplugSelector/ToUnplugSelectors/ > (3) Use plural for APIC ID in comment describing retval > EFI_ALREADY_STARTED. > (4) Fixup indentation in check against CPU_EJECT_QEMU_SELECTOR_INVALID. > (5) Clarify comment: > - // never match more than one APIC ID and by transitivity, more than > one > - // QemuSelector in a single invocation of UnplugCpus(). > + // never match more than one APIC ID -- nor, by transitivity, > designate > + // more than one QemuSelector -- in a single invocation of > UnplugCpus(). > (6a) Remove unnecessary UINT64 cast for > mCpuHotEjectData->QemuSelectorMap[ProcessorNum]. > (6b) Switch from 0x%Lx -> %Lu for QemuSelectorMap[ProcessorNum]. > (6c) Switch from 0x%Lx -> %u for QemuSelector > (7) Switch to "return EFI_ALREADY_STARTED". > (8a) Replace "QemuSelector 0x%Lx" with "QemuSelector %u". > (8b) Replace the mCpuHotEjectData->QemuSelectorMap[ProcessorNum] argument > with just QemuSelector in DEBUG call. > (9) Clarify comment and make the language complementary to that in patch-7 > Explicitly mention release memory fence. > > OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 + > OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 154 > ++-- > 2 files changed, 148 insertions(+), 8 deletions(-) Reviewed-by: Laszlo Ersek Thanks Laszlo > 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 59f000eb7886..2eeb4567a262 100644 > --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c > +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c > @@ -10,10 +10,12 @@ > #include // ICH9_APM_CNT > #include // QEMU_CPUHP_CMD_GET_PENDING > #include // CpuDeadLoop() > +#include // CpuSleep() > #include // ASSERT() > #include // gMmst > #include // PcdGetBool() > #include // SafeUintnSub() > +#include // CPU_HOT_EJECT_DATA > #include // EFI_MM_CPU_IO_PROTOCOL > #include // EFI_SMM_CPU_SERVICE_PROTOCOL > #include// 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. > @@ -190,18 +193,71 @@ 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. > +**/ >
[edk2-devel] [PATCH v9 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu()
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
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
---
Notes:
Addresses the following comments from v8:
(1) Fixup the coment about UnplugCpus() to reference stashing QEMU
Cpu Selectors instead of APIC IDs.
(2) s/ToUnplugSelector/ToUnplugSelectors/
(3) Use plural for APIC ID in comment describing retval EFI_ALREADY_STARTED.
(4) Fixup indentation in check against CPU_EJECT_QEMU_SELECTOR_INVALID.
(5) Clarify comment:
- // never match more than one APIC ID and by transitivity, more than one
- // QemuSelector in a single invocation of UnplugCpus().
+ // never match more than one APIC ID -- nor, by transitivity, designate
+ // more than one QemuSelector -- in a single invocation of UnplugCpus().
(6a) Remove unnecessary UINT64 cast for
mCpuHotEjectData->QemuSelectorMap[ProcessorNum].
(6b) Switch from 0x%Lx -> %Lu for QemuSelectorMap[ProcessorNum].
(6c) Switch from 0x%Lx -> %u for QemuSelector
(7) Switch to "return EFI_ALREADY_STARTED".
(8a) Replace "QemuSelector 0x%Lx" with "QemuSelector %u".
(8b) Replace the mCpuHotEjectData->QemuSelectorMap[ProcessorNum] argument
with just QemuSelector in DEBUG call.
(9) Clarify comment and make the language complementary to that in patch-7
Explicitly mention release memory fence.
OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 154 ++--
2 files changed, 148 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 59f000eb7886..2eeb4567a262 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -10,10 +10,12 @@
#include // ICH9_APM_CNT
#include // QEMU_CPUHP_CMD_GET_PENDING
#include // CpuDeadLoop()
+#include // CpuSleep()
#include // ASSERT()
#include // gMmst
#include // PcdGetBool()
#include // SafeUintnSub()
+#include // CPU_HOT_EJECT_DATA
#include // EFI_MM_CPU_IO_PROTOCOL
#include // EFI_SMM_CPU_SERVICE_PROTOCOL
#include// 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.
@@ -190,18 +193,71 @@ 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;
+ }
+
+ //
+ // APs being unplugged get here from SmmCpuFeaturesRendezvousExit()
+ // after having been cleared to
