On Wed, 3 Dec 2025 00:28:10 +0800 Zhao Liu <zhao1.liu@intel.com> wrote:
From: Igor Mammedov <imammedo@redhat.com> ^^^ given you resplit original patch, it's better to replace this with you, keeping my SoB is sufficient
For compatibility reasons PC/Q35 will start with legacy CPU hotplug interface by default but with new CPU hotplug AML code since 2.7 machine type (in commit 679dd1a957df ("pc: use new CPU hotplug interface since 2.7 machine type")). In that way, legacy firmware that doesn't use QEMU generated ACPI tables was able to continue using legacy CPU hotplug interface.
While later machine types, with firmware supporting QEMU provided ACPI tables, generate new CPU hotplug AML, which will switch to new CPU hotplug interface when guest OS executes its _INI method on ACPI tables loading.
Since 2.6 machine type is now gone, and consider that the legacy BIOS (based on QEMU ACPI prior to v2.7) should be no longer in use, previous compatibility requirements are no longer necessary. So initialize 'modern' hotplug directly from the very beginning for PC/Q35 machines with cpu_hotplug_hw_init(), and drop _INIT method.
Additionally, remove the checks and settings around cpu_hotplug_legacy in cpuhp VMState (for piix4 & ich9), to eliminate the risk of segmentation faults, as gpe_cpu no longer has the opportunity to be initialized. This is safe because all hotplug now start with the modern way, and it's impossible to switch to legacy way at runtime (even the "cpu-hotplug-legacy" properties does not allow it either).
Signed-off-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
tested ping pong cross version (master vs master+this patch) migration with 10.1 machine type, nothing is broken, hence Acked-by: Igor Mammedov <imammedo@redhat.com>
--- Changes since v4: * New patch split off from Igor's v5 [*].
[*]: https://lore.kernel.org/qemu-devel/20251031142825.179239-1-imammedo@redhat.c... --- hw/acpi/cpu.c | 10 ---------- hw/acpi/ich9.c | 22 +++------------------- hw/acpi/piix4.c | 21 +++------------------ hw/i386/acpi-build.c | 2 +- hw/loongarch/virt-acpi-build.c | 1 - include/hw/acpi/cpu.h | 1 - 6 files changed, 7 insertions(+), 50 deletions(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 6f1ae79edbf3..d63ca83c1bcd 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -408,16 +408,6 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(field, aml_reserved_field(4 * 8)); aml_append(field, aml_named_field(CPU_DATA, 32)); aml_append(cpu_ctrl_dev, field); - - if (opts.has_legacy_cphp) { - method = aml_method("_INI", 0, AML_SERIALIZED); - /* switch off legacy CPU hotplug HW and use new one, - * on reboot system is in new mode and writing 0 - * in CPU_SELECTOR selects BSP, which is NOP at - * the time _INI is called */ - aml_append(method, aml_store(zero, aml_name(CPU_SELECTOR))); - aml_append(cpu_ctrl_dev, method); - } } aml_append(sb_scope, cpu_ctrl_dev);
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 2b3b493c014b..54590129c695 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -183,26 +183,10 @@ static const VMStateDescription vmstate_tco_io_state = { } };
-static bool vmstate_test_use_cpuhp(void *opaque) -{ - ICH9LPCPMRegs *s = opaque; - return !s->cpu_hotplug_legacy; -} - -static int vmstate_cpuhp_pre_load(void *opaque) -{ - ICH9LPCPMRegs *s = opaque; - Object *obj = OBJECT(s->gpe_cpu.device); - object_property_set_bool(obj, "cpu-hotplug-legacy", false, &error_abort); - return 0; -} - static const VMStateDescription vmstate_cpuhp_state = { .name = "ich9_pm/cpuhp", .version_id = 1, .minimum_version_id = 1, - .needed = vmstate_test_use_cpuhp, - .pre_load = vmstate_cpuhp_pre_load, .fields = (const VMStateField[]) { VMSTATE_CPU_HOTPLUG(cpuhp_state, ICH9LPCPMRegs), VMSTATE_END_OF_LIST() @@ -338,8 +322,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, qemu_irq sci_irq) pm->powerdown_notifier.notify = pm_powerdown_req; qemu_register_powerdown_notifier(&pm->powerdown_notifier);
- legacy_acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), - OBJECT(lpc_pci), &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); + cpu_hotplug_hw_init(pci_address_space_io(lpc_pci), + OBJECT(lpc_pci), &pm->cpuhp_state, ICH9_CPU_HOTPLUG_IO_BASE);
acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), &pm->acpi_memory_hotplug, @@ -419,7 +403,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) { static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; pm->acpi_memory_hotplug.is_enabled = true; - pm->cpu_hotplug_legacy = true; + pm->cpu_hotplug_legacy = false; pm->disable_s3 = 0; pm->disable_s4 = 0; pm->s4_val = 2; diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 7a18f18dda21..a7a29b0d09a9 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -195,25 +195,10 @@ static const VMStateDescription vmstate_memhp_state = { } };
-static bool vmstate_test_use_cpuhp(void *opaque) -{ - PIIX4PMState *s = opaque; - return !s->cpu_hotplug_legacy; -} - -static int vmstate_cpuhp_pre_load(void *opaque) -{ - Object *obj = OBJECT(opaque); - object_property_set_bool(obj, "cpu-hotplug-legacy", false, &error_abort); - return 0; -} - static const VMStateDescription vmstate_cpuhp_state = { .name = "piix4_pm/cpuhp", .version_id = 1, .minimum_version_id = 1, - .needed = vmstate_test_use_cpuhp, - .pre_load = vmstate_cpuhp_pre_load, .fields = (const VMStateField[]) { VMSTATE_CPU_HOTPLUG(cpuhp_state, PIIX4PMState), VMSTATE_END_OF_LIST() @@ -573,12 +558,12 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, qbus_set_hotplug_handler(BUS(pci_get_bus(PCI_DEVICE(s))), OBJECT(s)); }
- s->cpu_hotplug_legacy = true; + s->cpu_hotplug_legacy = false; object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy", piix4_get_cpu_hotplug_legacy, piix4_set_cpu_hotplug_legacy); - legacy_acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu, - PIIX4_CPU_HOTPLUG_IO_BASE); + cpu_hotplug_hw_init(parent, OBJECT(s), &s->cpuhp_state, + PIIX4_CPU_HOTPLUG_IO_BASE);
if (s->acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug, diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9446a9f862ca..23147ddc25e7 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -964,7 +964,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base); } else { CPUHotplugFeatures opts = { - .acpi_1_compatible = true, .has_legacy_cphp = true, + .acpi_1_compatible = true, .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, .fw_unplugs_cpu = pm->smi_on_cpu_unplug, }; diff --git a/hw/loongarch/virt-acpi-build.c b/hw/loongarch/virt-acpi-build.c index 3694c9827f04..8d01c8e3de87 100644 --- a/hw/loongarch/virt-acpi-build.c +++ b/hw/loongarch/virt-acpi-build.c @@ -369,7 +369,6 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
if (event & ACPI_GED_CPU_HOTPLUG_EVT) { opts.acpi_1_compatible = false; - opts.has_legacy_cphp = false; opts.fw_unplugs_cpu = false; opts.smi_path = NULL;
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index 32654dc274fd..2cb0ca4f3dce 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -54,7 +54,6 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
typedef struct CPUHotplugFeatures { bool acpi_1_compatible; - bool has_legacy_cphp; bool fw_unplugs_cpu; const char *smi_path; } CPUHotplugFeatures;