Process fw_remove events in QemuCpuhpCollectApicIds(), and collect APIC IDs
and QEMU CPU Selectors for CPUs being hot-unplugged.
In addition, we now ignore CPUs which only have remove set. These
CPUs haven't been processed by OSPM yet.
This is based on the QEMU hot-unplug protocol documented here:
https://lore.kernel.org/qemu-devel/20201204170939.1815522-3-imammedo@redhat.com/
Cc: Laszlo Ersek <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 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_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..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__, Status));
goto ReleasePluggedApicIds;
}
+ Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,
+ (VOID **)&mToUnplugSelectors);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
+ goto ReleaseToUnplugApicIds;
+ }
//
// Allocate the Post-SMM Pen for hot-added CPUs.
@@ -412,7 +425,7 @@ CpuHotplugEntry (
Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress,
SystemTable->BootServices);
if (EFI_ERROR (Status)) {
- goto ReleaseToUnplugApicIds;
+ goto ReleaseToUnplugSelectors;
}
//
@@ -472,6 +485,10 @@ ReleasePostSmmPen:
SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);
mPostSmmPenAddress = 0;
+ReleaseToUnplugSelectors:
+ gMmst->MmFreePool (mToUnplugSelectors);
+ mToUnplugSelectors = NULL;
+
ReleaseToUnplugApicIds:
gMmst->MmFreePool (mToUnplugApicIds);
mToUnplugApicIds = NULL;
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 8d4a6693c8d6..8434dd446b96 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -145,27 +145,30 @@ QemuCpuhpWriteCommand (
On error, the contents of the output parameters are undefined.
- @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
- accessing IO Ports.
+ @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
+ accessing IO Ports.
- @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
- be positive.
+ @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
+ be positive.
- @param[in] ApicIdCount The number of elements each one of the
- PluggedApicIds and ToUnplugApicIds arrays can
- accommodate. Must be positive.
+ @param[in] ApicIdCount The number of elements each one of the
+ PluggedApicIds and ToUnplugApicIds arrays can
+ accommodate. Must be positive.
- @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
- hot-plugged.
+ @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
+ hot-plugged.
- @param[out] PluggedCount The number of filled-in APIC IDs in
- PluggedApicIds.
+ @param[out] PluggedCount The number of filled-in APIC IDs in
+ PluggedApicIds.
- @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
- hot-unplugged.
+ @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
+ hot-unplugged.
- @param[out] ToUnplugCount The number of filled-in APIC IDs in
- ToUnplugApicIds.
+ @param[out] ToUnplugSelectors The QEMU Selectors of the CPUs that are about
+ to be hot-unplugged.
+
+ @param[out] ToUnplugCount The number of filled-in APIC IDs in
+ ToUnplugApicIds.
@retval EFI_INVALID_PARAMETER PossibleCpuCount is zero, or ApicIdCount is
zero.
@@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds (
OUT APIC_ID *PluggedApicIds,
OUT UINT32 *PluggedCount,
OUT APIC_ID *ToUnplugApicIds,
+ OUT UINT32 *ToUnplugSelectors,
OUT UINT32 *ToUnplugCount
)
{
@@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds (
UINT32 PendingSelector;
UINT8 CpuStatus;
APIC_ID *ExtendIds;
+ UINT32 *ExtendSels;
UINT32 *ExtendCount;
APIC_ID NewApicId;
@@ -245,10 +250,10 @@ QemuCpuhpCollectApicIds (
if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) {
//
// The "insert" event guarantees the "enabled" status; plus it excludes
- // the "remove" event.
+ // the "fw_remove" event.
//
if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 ||
- (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
+ (CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
"inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
CpuStatus));
@@ -259,33 +264,63 @@ QemuCpuhpCollectApicIds (
CurrentSelector));
ExtendIds = PluggedApicIds;
+ ExtendSels = NULL;
ExtendCount = PluggedCount;
- } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
- DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n", __FUNCTION__,
- CurrentSelector));
+ } else if ((CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
+ //
+ // "fw_remove" event guarantees "enabled".
+ //
+ if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0) {
+ DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
+ "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
+ CpuStatus));
+ return EFI_PROTOCOL_ERROR;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: fw_remove\n",
+ __FUNCTION__, CurrentSelector));
ExtendIds = ToUnplugApicIds;
+ ExtendSels = ToUnplugSelectors;
ExtendCount = ToUnplugCount;
+ } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
+ //
+ // Let the OSPM deal with the "remove" event.
+ //
+ DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove (ignored)\n",
+ __FUNCTION__, CurrentSelector));
+
+ ExtendIds = NULL;
+ ExtendSels = NULL;
+ ExtendCount = NULL;
} else {
DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: no event\n",
__FUNCTION__, CurrentSelector));
break;
}
- //
- // Save the APIC ID of the CPU with the pending event, to the corresponding
- // APIC ID array.
- //
- if (*ExtendCount == ApicIdCount) {
- DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
- return EFI_BUFFER_TOO_SMALL;
- }
- QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
- NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
- DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
- NewApicId));
- ExtendIds[(*ExtendCount)++] = NewApicId;
+ ASSERT ((ExtendIds == NULL) == (ExtendCount == NULL));
+ ASSERT ((ExtendSels == NULL) || (ExtendIds != NULL));
+ if (ExtendIds != NULL) {
+ //
+ // Save the APIC ID of the CPU with the pending event, to the
+ // corresponding APIC ID array.
+ // For unplug events, also save the CurrentSelector.
+ //
+ if (*ExtendCount == ApicIdCount) {
+ DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
+ return EFI_BUFFER_TOO_SMALL;
+ }
+ QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
+ NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
+ DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
+ NewApicId));
+ if (ExtendSels != NULL) {
+ ExtendSels[(*ExtendCount)] = CurrentSelector;
+ }
+ ExtendIds[(*ExtendCount)++] = NewApicId;
+ }
//
// We've processed the CPU with (known) pending events, but we must never
// clear events. Therefore we need to advance past this CPU manually;
--
2.9.3
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72704): https://edk2.groups.io/g/devel/message/72704
Mute This Topic: https://groups.io/mt/81273603/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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-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 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 <lersek@redhat.com>
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_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..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__, Status));
> goto ReleasePluggedApicIds;
> }
> + Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,
> + (VOID **)&mToUnplugSelectors);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
> + goto ReleaseToUnplugApicIds;
> + }
>
> //
> // Allocate the Post-SMM Pen for hot-added CPUs.
> @@ -412,7 +425,7 @@ CpuHotplugEntry (
> Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress,
> SystemTable->BootServices);
> if (EFI_ERROR (Status)) {
> - goto ReleaseToUnplugApicIds;
> + goto ReleaseToUnplugSelectors;
> }
>
> //
> @@ -472,6 +485,10 @@ ReleasePostSmmPen:
> SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);
> mPostSmmPenAddress = 0;
>
> +ReleaseToUnplugSelectors:
> + gMmst->MmFreePool (mToUnplugSelectors);
> + mToUnplugSelectors = NULL;
> +
> ReleaseToUnplugApicIds:
> gMmst->MmFreePool (mToUnplugApicIds);
> mToUnplugApicIds = NULL;
> diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> index 8d4a6693c8d6..8434dd446b96 100644
> --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> @@ -145,27 +145,30 @@ QemuCpuhpWriteCommand (
>
> On error, the contents of the output parameters are undefined.
>
> - @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
> - accessing IO Ports.
> + @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
> + accessing IO Ports.
>
> - @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
> - be positive.
> + @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
> + be positive.
>
> - @param[in] ApicIdCount The number of elements each one of the
> - PluggedApicIds and ToUnplugApicIds arrays can
> - accommodate. Must be positive.
> + @param[in] ApicIdCount The number of elements each one of the
> + PluggedApicIds and ToUnplugApicIds arrays can
> + accommodate. Must be positive.
>
> - @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
> - hot-plugged.
> + @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
> + hot-plugged.
>
> - @param[out] PluggedCount The number of filled-in APIC IDs in
> - PluggedApicIds.
> + @param[out] PluggedCount The number of filled-in APIC IDs in
> + PluggedApicIds.
>
> - @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
> - hot-unplugged.
> + @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
> + hot-unplugged.
>
> - @param[out] ToUnplugCount The number of filled-in APIC IDs in
> - ToUnplugApicIds.
> + @param[out] ToUnplugSelectors The QEMU Selectors of the CPUs that are about
> + to be hot-unplugged.
> +
> + @param[out] ToUnplugCount The number of filled-in APIC IDs in
> + ToUnplugApicIds.
>
> @retval EFI_INVALID_PARAMETER PossibleCpuCount is zero, or ApicIdCount is
> zero.
> @@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds (
> OUT APIC_ID *PluggedApicIds,
> OUT UINT32 *PluggedCount,
> OUT APIC_ID *ToUnplugApicIds,
> + OUT UINT32 *ToUnplugSelectors,
> OUT UINT32 *ToUnplugCount
> )
> {
> @@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds (
> UINT32 PendingSelector;
> UINT8 CpuStatus;
> APIC_ID *ExtendIds;
> + UINT32 *ExtendSels;
> UINT32 *ExtendCount;
> APIC_ID NewApicId;
>
> @@ -245,10 +250,10 @@ QemuCpuhpCollectApicIds (
> if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) {
> //
> // The "insert" event guarantees the "enabled" status; plus it excludes
> - // the "remove" event.
> + // the "fw_remove" event.
> //
> if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 ||
> - (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
> + (CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
> DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
> "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
> CpuStatus));
> @@ -259,33 +264,63 @@ QemuCpuhpCollectApicIds (
> CurrentSelector));
>
> ExtendIds = PluggedApicIds;
> + ExtendSels = NULL;
> ExtendCount = PluggedCount;
> - } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
> - DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n", __FUNCTION__,
> - CurrentSelector));
> + } else if ((CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
> + //
> + // "fw_remove" event guarantees "enabled".
> + //
> + if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0) {
> + DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
> + "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
> + CpuStatus));
> + return EFI_PROTOCOL_ERROR;
> + }
> +
> + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: fw_remove\n",
> + __FUNCTION__, CurrentSelector));
>
> ExtendIds = ToUnplugApicIds;
> + ExtendSels = ToUnplugSelectors;
> ExtendCount = ToUnplugCount;
> + } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
> + //
> + // Let the OSPM deal with the "remove" event.
> + //
> + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove (ignored)\n",
> + __FUNCTION__, CurrentSelector));
> +
> + ExtendIds = NULL;
> + ExtendSels = NULL;
> + ExtendCount = NULL;
> } else {
> DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: no event\n",
> __FUNCTION__, CurrentSelector));
> break;
> }
>
> - //
> - // Save the APIC ID of the CPU with the pending event, to the corresponding
> - // APIC ID array.
> - //
> - if (*ExtendCount == ApicIdCount) {
> - DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
> - return EFI_BUFFER_TOO_SMALL;
> - }
> - QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
> - NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
> - DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
> - NewApicId));
> - ExtendIds[(*ExtendCount)++] = NewApicId;
> + ASSERT ((ExtendIds == NULL) == (ExtendCount == NULL));
> + ASSERT ((ExtendSels == NULL) || (ExtendIds != NULL));
>
> + if (ExtendIds != NULL) {
> + //
> + // Save the APIC ID of the CPU with the pending event, to the
> + // corresponding APIC ID array.
> + // For unplug events, also save the CurrentSelector.
> + //
> + if (*ExtendCount == ApicIdCount) {
> + DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
> + return EFI_BUFFER_TOO_SMALL;
> + }
> + QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
> + NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
> + DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
> + NewApicId));
> + if (ExtendSels != NULL) {
> + ExtendSels[(*ExtendCount)] = CurrentSelector;
> + }
> + ExtendIds[(*ExtendCount)++] = NewApicId;
> + }
> //
> // We've processed the CPU with (known) pending events, but we must never
> // clear events. Therefore we need to advance past this CPU manually;
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72863): https://edk2.groups.io/g/devel/message/72863
Mute This Topic: https://groups.io/mt/81273603/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.