On Wed, 26 Feb 2020 at 23:12, Laszlo Ersek <ler...@redhat.com> wrote:
>
> Add a function that collects the APIC IDs of CPUs that have just been
> hot-plugged, or are about to be hot-unplugged.
>
> Pending events are only located and never cleared; QEMU's AML needs the
> firmware to leave the status bits intact in the hotplug register block.
>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: Igor Mammedov <imamm...@redhat.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Michael Kinney <michael.d.kin...@intel.com>
> Cc: Philippe Mathieu-Daudé <phi...@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>
> Notes:
> v2:
>
> - Pick up Ard's Acked-by, which is conditional on approval from Intel
> reviewers on Cc. (I'd like to save Ard the churn of re-acking
> unmodified patches.)
>
> OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 2 +
> OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 1 +
> OvmfPkg/CpuHotplugSmm/ApicId.h | 23 +++
> OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 20 ++-
> OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 171 +++++++++++++++++++-
> 5 files changed, 211 insertions(+), 6 deletions(-)
>
> diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
> b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
> index 3d013633501b..a34a6d3fae61 100644
> --- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
> +++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
> @@ -13,32 +13,34 @@
> The new ("modern") hotplug interface appeared in QEMU v2.7.0.
>
> The macros in this header file map to the minimal subset of the modern
> interface that OVMF needs.
> **/
>
> #ifndef QEMU_CPU_HOTPLUG_H_
> #define QEMU_CPU_HOTPLUG_H_
>
> #include <Base.h>
>
> //
> // Each register offset is:
> // - relative to the board-dependent IO base address of the register block,
> // - named QEMU_CPUHP_(R|W|RW)_*, according to the possible access modes of
> the
> // register,
> // - followed by distinguished bitmasks or values in the register.
> //
> #define QEMU_CPUHP_R_CMD_DATA2 0x0
>
> #define QEMU_CPUHP_R_CPU_STAT 0x4
> #define QEMU_CPUHP_STAT_ENABLED BIT0
> +#define QEMU_CPUHP_STAT_INSERT BIT1
> +#define QEMU_CPUHP_STAT_REMOVE BIT2
>
> #define QEMU_CPUHP_RW_CMD_DATA 0x8
>
> #define QEMU_CPUHP_W_CPU_SEL 0x0
>
> #define QEMU_CPUHP_W_CMD 0x5
> #define QEMU_CPUHP_CMD_GET_PENDING 0x0
> #define QEMU_CPUHP_CMD_GET_ARCH_ID 0x3
>
> #endif // QEMU_CPU_HOTPLUG_H_
> diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
> b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
> index ac4ca4c1f4f2..ab690a9e5e20 100644
> --- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
> +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
> @@ -3,44 +3,45 @@
> #
> # Copyright (c) 2020, Red Hat, Inc.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> ##
>
> [Defines]
> INF_VERSION = 1.29
> PI_SPECIFICATION_VERSION = 0x00010046 #
> PI-1.7.0
> BASE_NAME = CpuHotplugSmm
> FILE_GUID = 84EEA114-C6BE-4445-8F90-51D97863E363
> MODULE_TYPE = DXE_SMM_DRIVER
> ENTRY_POINT = CpuHotplugEntry
>
> #
> # The following information is for reference only and not required by the
> build
> # tools.
> #
> # VALID_ARCHITECTURES = IA32 X64
> #
>
> [Sources]
> + ApicId.h
> CpuHotplug.c
> QemuCpuhp.c
> QemuCpuhp.h
>
> [Packages]
> MdePkg/MdePkg.dec
> OvmfPkg/OvmfPkg.dec
>
> [LibraryClasses]
> BaseLib
> DebugLib
> MmServicesTableLib
> PcdLib
> UefiDriverEntryPoint
>
> [Protocols]
> gEfiMmCpuIoProtocolGuid ##
> CONSUMES
>
> [Pcd]
> gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ##
> CONSUMES
>
> [FeaturePcd]
> diff --git a/OvmfPkg/CpuHotplugSmm/ApicId.h b/OvmfPkg/CpuHotplugSmm/ApicId.h
> new file mode 100644
> index 000000000000..3c365148ed02
> --- /dev/null
> +++ b/OvmfPkg/CpuHotplugSmm/ApicId.h
> @@ -0,0 +1,23 @@
> +/** @file
> + Type and macro definitions for representing and printing APIC IDs,
> compatibly
> + with the LocalApicLib and PrintLib classes, respectively.
> +
> + Copyright (c) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef APIC_ID_H_
> +#define APIC_ID_H_
> +
> +//
> +// The type that LocalApicLib represents an APIC ID with.
> +//
> +typedef UINT32 APIC_ID;
> +
> +//
> +// The PrintLib conversion specification for formatting an APIC_ID.
> +//
> +#define FMT_APIC_ID "0x%08x"
> +
> +#endif // APIC_ID_H_
> diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
> b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
> index 82f88f0b73bb..8adaa0ad91f0 100644
> --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
> +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
> @@ -1,47 +1,61 @@
> /** @file
> - Simple wrapper functions that access QEMU's modern CPU hotplug register
> - block.
> + Simple wrapper functions and utility functions that access QEMU's modern
> CPU
> + hotplug register block.
>
> - These functions thinly wrap some of the registers described in
> + These functions manipulate some of the registers described in
> "docs/specs/acpi_cpu_hotplug.txt" in the QEMU source. IO Ports are accessed
> via EFI_MM_CPU_IO_PROTOCOL. If a protocol call fails, these functions don't
> return.
>
> Copyright (c) 2020, Red Hat, Inc.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #ifndef QEMU_CPUHP_H_
> #define QEMU_CPUHP_H_
>
> #include <Protocol/MmCpuIo.h> // EFI_MM_CPU_IO_PROTOCOL
> +#include <Uefi/UefiBaseType.h> // EFI_STATUS
> +
> +#include "ApicId.h" // APIC_ID
>
> UINT32
> QemuCpuhpReadCommandData2 (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo
> );
>
> UINT8
> QemuCpuhpReadCpuStatus (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo
> );
>
> UINT32
> QemuCpuhpReadCommandData (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo
> );
>
> VOID
> QemuCpuhpWriteCpuSelector (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
> IN UINT32 Selector
> );
>
> VOID
> QemuCpuhpWriteCommand (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
> IN UINT8 Command
> );
>
> +EFI_STATUS
> +QemuCpuhpCollectApicIds (
> + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
> + IN UINT32 PossibleCpuCount,
> + IN UINT32 ApicIdCount,
> + OUT APIC_ID *PluggedApicIds,
> + OUT UINT32 *PluggedCount,
> + OUT APIC_ID *ToUnplugApicIds,
> + OUT UINT32 *ToUnplugCount
> + );
> +
> #endif // QEMU_CPUHP_H_
> diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> index 31e46f51934a..8d4a6693c8d6 100644
> --- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> +++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
> @@ -1,27 +1,27 @@
> /** @file
> - Simple wrapper functions that access QEMU's modern CPU hotplug register
> - block.
> + Simple wrapper functions and utility functions that access QEMU's modern
> CPU
> + hotplug register block.
>
> - These functions thinly wrap some of the registers described in
> + These functions manipulate some of the registers described in
> "docs/specs/acpi_cpu_hotplug.txt" in the QEMU source. IO Ports are accessed
> via EFI_MM_CPU_IO_PROTOCOL. If a protocol call fails, these functions don't
> return.
>
> Copyright (c) 2020, Red Hat, Inc.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #include <IndustryStandard/Q35MchIch9.h> // ICH9_CPU_HOTPLUG_BASE
> #include <IndustryStandard/QemuCpuHotplug.h> // QEMU_CPUHP_R_CMD_DATA2
> #include <Library/BaseLib.h> // CpuDeadLoop()
> #include <Library/DebugLib.h> // DEBUG()
>
> #include "QemuCpuhp.h"
>
> UINT32
> QemuCpuhpReadCommandData2 (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo
> )
> {
> UINT32 CommandData2;
> @@ -115,22 +115,187 @@ QemuCpuhpWriteCpuSelector (
>
> VOID
> QemuCpuhpWriteCommand (
> IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
> IN UINT8 Command
> )
> {
> EFI_STATUS Status;
>
> Status = MmCpuIo->Io.Write (
> MmCpuIo,
> MM_IO_UINT8,
> ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_W_CMD,
> 1,
> &Command
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, Status));
> ASSERT (FALSE);
> CpuDeadLoop ();
> }
> }
> +
> +/**
> + Collect the APIC IDs of
> + - the CPUs that have been hot-plugged,
> + - the CPUs that are about to be hot-unplugged.
> +
> + This function only scans for events -- it does not modify them -- in the
> + hotplug registers.
> +
> + 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] 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[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] 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.
> +
> + @retval EFI_INVALID_PARAMETER PossibleCpuCount is zero, or ApicIdCount is
> + zero.
> +
> + @retval EFI_PROTOCOL_ERROR Invalid bitmap detected in the
> + QEMU_CPUHP_R_CPU_STAT register.
> +
> + @retval EFI_BUFFER_TOO_SMALL There was an attempt to place more than
> + ApicIdCount APIC IDs into one of the
> + PluggedApicIds and ToUnplugApicIds arrays.
> +
> + @retval EFI_SUCCESS Output parameters have been set
> successfully.
> +**/
> +EFI_STATUS
> +QemuCpuhpCollectApicIds (
> + IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
> + IN UINT32 PossibleCpuCount,
> + IN UINT32 ApicIdCount,
> + OUT APIC_ID *PluggedApicIds,
> + OUT UINT32 *PluggedCount,
> + OUT APIC_ID *ToUnplugApicIds,
> + OUT UINT32 *ToUnplugCount
> + )
> +{
> + UINT32 CurrentSelector;
> +
> + if (PossibleCpuCount == 0 || ApicIdCount == 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *PluggedCount = 0;
> + *ToUnplugCount = 0;
> +
> + CurrentSelector = 0;
> + do {
> + UINT32 PendingSelector;
> + UINT8 CpuStatus;
> + APIC_ID *ExtendIds;
> + UINT32 *ExtendCount;
> + APIC_ID NewApicId;
> +
> + //
> + // Write CurrentSelector (which is valid) to the CPU selector register.
> + // Consequences:
> + //
> + // - Other register accesses will be permitted.
> + //
> + // - The QEMU_CPUHP_CMD_GET_PENDING command will start scanning for a CPU
> + // with pending events at CurrentSelector (inclusive).
> + //
> + QemuCpuhpWriteCpuSelector (MmCpuIo, CurrentSelector);
> + //
> + // Write the QEMU_CPUHP_CMD_GET_PENDING command. Consequences
> + // (independently of each other):
> + //
> + // - If there is a CPU with pending events, starting at CurrentSelector
> + // (inclusive), the CPU selector will be updated to that CPU. Note that
> + // the scanning in QEMU may wrap around, because we must never clear
> the
> + // event bits.
> + //
> + // - The QEMU_CPUHP_RW_CMD_DATA register will return the (possibly
> updated)
> + // CPU selector value.
> + //
> + QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_PENDING);
> + PendingSelector = QemuCpuhpReadCommandData (MmCpuIo);
> + if (PendingSelector < CurrentSelector) {
> + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u PendingSelector=%u: "
> + "wrap-around\n", __FUNCTION__, CurrentSelector, PendingSelector));
> + break;
> + }
> + CurrentSelector = PendingSelector;
> +
> + //
> + // Check the known status / event bits for the currently selected CPU.
> + //
> + CpuStatus = QemuCpuhpReadCpuStatus (MmCpuIo);
> + if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) {
> + //
> + // The "insert" event guarantees the "enabled" status; plus it excludes
> + // the "remove" event.
> + //
> + if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 ||
> + (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 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: insert\n",
> __FUNCTION__,
> + CurrentSelector));
> +
> + ExtendIds = PluggedApicIds;
> + ExtendCount = PluggedCount;
> + } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
> + DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n",
> __FUNCTION__,
> + CurrentSelector));
> +
> + ExtendIds = ToUnplugApicIds;
> + ExtendCount = ToUnplugCount;
> + } 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;
> +
> + //
> + // 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",
> + __FUNCTION__, *PluggedCount, *ToUnplugCount));
> + return EFI_SUCCESS;
> +}
> --
> 2.19.1.3.g30247aa5d201
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55202): https://edk2.groups.io/g/devel/message/55202
Mute This Topic: https://groups.io/mt/71575183/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
