[edk2-devel] [PATCH v9 02/10] 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-imamm...@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 --- Notes: Addresses the following comments from v8: (1) Fix commit message to mention that we collect cpu-selectors as well. (2,3,6) s/UnplugSelector/UnplugSelectors/ in CpuHotplug.c, QemuCpuhp.c (4) Fix comment above the declaration of the now renamed mToUnplugSelector. (5) Fix spacing around "||". (7) Fix QemuCpuCollectApicIds() comments to line up descriptions for ToUnplugSelectors and other params. (8) s/ExtendSel/ExtendSels/. (9) Add the (ExtendSels => ExtendIds) assert. (10) Fix the missing CurrentSelector++ bug. OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 1 + OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 + OvmfPkg/CpuHotplugSmm/CpuHotplug.c| 29 +-- OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 101 +++--- 4 files changed, 93 insertions(+), 39 deletions(-) diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h index 8adaa0ad91f0..3e2c2192e1c0 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 ); 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_ENABLEDBIT0 #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..ee1497b93140 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__, Statu
Re: [edk2-devel] [PATCH v9 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events
On 03/12/21 07:26, Ankur Arora wrote: > 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-imamm...@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 > --- > > Notes: > Addresses the following comments from v8: > (1) Fix commit message to mention that we collect cpu-selectors as well. > (2,3,6) s/UnplugSelector/UnplugSelectors/ in CpuHotplug.c, QemuCpuhp.c > (4) Fix comment above the declaration of the now renamed > mToUnplugSelector. > (5) Fix spacing around "||". > (7) Fix QemuCpuCollectApicIds() comments to line up descriptions for > ToUnplugSelectors and other params. > (8) s/ExtendSel/ExtendSels/. > (9) Add the (ExtendSels => ExtendIds) assert. > (10) Fix the missing CurrentSelector++ bug. > > OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 1 + > OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 + > OvmfPkg/CpuHotplugSmm/CpuHotplug.c| 29 +-- > OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 101 > +++--- > 4 files changed, 93 insertions(+), 39 deletions(-) Reviewed-by: Laszlo Ersek Thanks Laszlo > > diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h > index 8adaa0ad91f0..3e2c2192e1c0 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 >); > > 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_ENABLEDBIT0 > #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..ee1497b93140 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