On Wed, 3 Dec 2025 00:28:10 +0800 Zhao Liu <[email protected]> wrote:
> From: Igor Mammedov <[email protected]> ^^^ 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 <[email protected]> > Signed-off-by: Zhao Liu <[email protected]> tested ping pong cross version (master vs master+this patch) migration with 10.1 machine type, nothing is broken, hence Acked-by: Igor Mammedov <[email protected]> > --- > Changes since v4: > * New patch split off from Igor's v5 [*]. > > [*]: > https://lore.kernel.org/qemu-devel/[email protected]/ > --- > 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;
