| Message ID | 20250501183628.87479-3-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/i386/pc: Remove deprecated 2.6 and 2.7 PC machines | expand |
On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote: > The PCMachineClass::legacy_cpu_hotplug boolean was only used > by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got > removed. Remove it and simplify build_dsdt(), removing > build_legacy_cpu_hotplug_aml() altogether. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/acpi/cpu_hotplug.h | 3 - > include/hw/i386/pc.h | 3 - > hw/acpi/cpu_hotplug.c | 230 ---------------------------------- > hw/i386/acpi-build.c | 4 +- > 4 files changed, 1 insertion(+), 239 deletions(-) > > diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h > index 3b932abbbbe..aeee630cf05 100644 > --- a/include/hw/acpi/cpu_hotplug.h > +++ b/include/hw/acpi/cpu_hotplug.h > @@ -34,7 +34,4 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, > void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, > CPUHotplugState *cpuhp_state, > uint16_t io_port); > - > -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, > - uint16_t io_base); > #endif > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 79b72c54dd3..a3de3e9560d 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -110,9 +110,6 @@ struct PCMachineClass { > bool enforce_amd_1tb_hole; > bool isa_bios_alias; > > - /* generate legacy CPU hotplug AML */ > - bool legacy_cpu_hotplug; > - > /* use PVH to load kernels that support this feature */ > bool pvh_enabled; > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c > index aa0e1e3efa5..fe439705bda 100644 > --- a/hw/acpi/cpu_hotplug.c > +++ b/hw/acpi/cpu_hotplug.c > @@ -116,233 +116,3 @@ void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, > memory_region_del_subregion(parent, &gpe_cpu->io); > cpu_hotplug_hw_init(parent, gpe_cpu->device, cpuhp_state, io_port); > } > - > -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, > - uint16_t io_base) > -{ > - Aml *dev; > - Aml *crs; > - Aml *pkg; > - Aml *field; > - Aml *method; > - Aml *if_ctx; > - Aml *else_ctx; > - int i, apic_idx; > - Aml *sb_scope = aml_scope("_SB"); > - uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0}; > - Aml *cpu_id = aml_arg(1); > - Aml *apic_id = aml_arg(0); > - Aml *cpu_on = aml_local(0); > - Aml *madt = aml_local(1); > - Aml *cpus_map = aml_name(CPU_ON_BITMAP); > - Aml *zero = aml_int(0); > - Aml *one = aml_int(1); > - MachineClass *mc = MACHINE_GET_CLASS(machine); > - const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine); > - X86MachineState *x86ms = X86_MACHINE(machine); > - > - /* > - * _MAT method - creates an madt apic buffer > - * apic_id = Arg0 = Local APIC ID > - * cpu_id = Arg1 = Processor ID > - * cpu_on = Local0 = CPON flag for this cpu > - * madt = Local1 = Buffer (in madt apic form) to return > - */ > - method = aml_method(CPU_MAT_METHOD, 2, AML_NOTSERIALIZED); > - aml_append(method, > - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); > - aml_append(method, > - aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt)); > - /* Update the processor id, lapic id, and enable/disable status */ > - aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2)))); > - aml_append(method, aml_store(apic_id, aml_index(madt, aml_int(3)))); > - aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4)))); > - aml_append(method, aml_return(madt)); > - aml_append(sb_scope, method); > - > - /* > - * _STA method - return ON status of cpu > - * apic_id = Arg0 = Local APIC ID > - * cpu_on = Local0 = CPON flag for this cpu > - */ > - method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED); > - aml_append(method, > - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); > - if_ctx = aml_if(cpu_on); > - { > - aml_append(if_ctx, aml_return(aml_int(0xF))); > - } > - aml_append(method, if_ctx); > - else_ctx = aml_else(); > - { > - aml_append(else_ctx, aml_return(zero)); > - } > - aml_append(method, else_ctx); > - aml_append(sb_scope, method); > - > - method = aml_method(CPU_EJECT_METHOD, 2, AML_NOTSERIALIZED); > - aml_append(method, aml_sleep(200)); > - aml_append(sb_scope, method); > - > - method = aml_method(CPU_SCAN_METHOD, 0, AML_NOTSERIALIZED); > - { > - Aml *while_ctx, *if_ctx2, *else_ctx2; > - Aml *bus_check_evt = aml_int(1); > - Aml *remove_evt = aml_int(3); > - Aml *status_map = aml_local(5); /* Local5 = active cpu bitmap */ > - Aml *byte = aml_local(2); /* Local2 = last read byte from bitmap */ > - Aml *idx = aml_local(0); /* Processor ID / APIC ID iterator */ > - Aml *is_cpu_on = aml_local(1); /* Local1 = CPON flag for cpu */ > - Aml *status = aml_local(3); /* Local3 = active state for cpu */ > - > - aml_append(method, aml_store(aml_name(CPU_STATUS_MAP), status_map)); > - aml_append(method, aml_store(zero, byte)); > - aml_append(method, aml_store(zero, idx)); > - > - /* While (idx < SizeOf(CPON)) */ > - while_ctx = aml_while(aml_lless(idx, aml_sizeof(cpus_map))); > - aml_append(while_ctx, > - aml_store(aml_derefof(aml_index(cpus_map, idx)), is_cpu_on)); > - > - if_ctx = aml_if(aml_and(idx, aml_int(0x07), NULL)); > - { > - /* Shift down previously read bitmap byte */ > - aml_append(if_ctx, aml_shiftright(byte, one, byte)); > - } > - aml_append(while_ctx, if_ctx); > - > - else_ctx = aml_else(); > - { > - /* Read next byte from cpu bitmap */ > - aml_append(else_ctx, aml_store(aml_derefof(aml_index(status_map, > - aml_shiftright(idx, aml_int(3), NULL))), byte)); > - } > - aml_append(while_ctx, else_ctx); > - > - aml_append(while_ctx, aml_store(aml_and(byte, one, NULL), status)); > - if_ctx = aml_if(aml_lnot(aml_equal(is_cpu_on, status))); > - { > - /* State change - update CPON with new state */ > - aml_append(if_ctx, aml_store(status, aml_index(cpus_map, idx))); > - if_ctx2 = aml_if(aml_equal(status, one)); > - { > - aml_append(if_ctx2, > - aml_call2(AML_NOTIFY_METHOD, idx, bus_check_evt)); > - } > - aml_append(if_ctx, if_ctx2); > - else_ctx2 = aml_else(); > - { > - aml_append(else_ctx2, > - aml_call2(AML_NOTIFY_METHOD, idx, remove_evt)); > - } > - } > - aml_append(if_ctx, else_ctx2); > - aml_append(while_ctx, if_ctx); > - > - aml_append(while_ctx, aml_increment(idx)); /* go to next cpu */ > - aml_append(method, while_ctx); > - } > - aml_append(sb_scope, method); > - > - /* The current AML generator can cover the APIC ID range [0..255], > - * inclusive, for VCPU hotplug. */ > - QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); > - if (x86ms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > - error_report("max_cpus is too large. APIC ID of last CPU is %u", > - x86ms->apic_id_limit - 1); > - exit(1); > - } > - > - /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ > - dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); > - aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06"))); > - aml_append(dev, > - aml_name_decl("_UID", aml_string("CPU Hotplug resources")) > - ); > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - crs = aml_resource_template(); > - aml_append(crs, > - aml_io(AML_DECODE16, io_base, io_base, 1, ACPI_GPE_PROC_LEN) > - ); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(sb_scope, dev); > - /* declare CPU hotplug MMIO region and PRS field to access it */ > - aml_append(sb_scope, aml_operation_region( > - "PRST", AML_SYSTEM_IO, aml_int(io_base), ACPI_GPE_PROC_LEN)); > - field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE); > - aml_append(field, aml_named_field("PRS", 256)); > - aml_append(sb_scope, field); > - > - /* build Processor object for each processor */ > - for (i = 0; i < apic_ids->len; i++) { > - int cpu_apic_id = apic_ids->cpus[i].arch_id; > - > - assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT); > - > - dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id); > - > - method = aml_method("_MAT", 0, AML_NOTSERIALIZED); > - aml_append(method, > - aml_return(aml_call2(CPU_MAT_METHOD, > - aml_int(cpu_apic_id), aml_int(i)) > - )); > - aml_append(dev, method); > - > - method = aml_method("_STA", 0, AML_NOTSERIALIZED); > - aml_append(method, > - aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id)))); > - aml_append(dev, method); > - > - method = aml_method("_EJ0", 1, AML_NOTSERIALIZED); > - aml_append(method, > - aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id), > - aml_arg(0))) > - ); > - aml_append(dev, method); > - > - aml_append(sb_scope, dev); > - } > - > - /* build this code: > - * Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...} > - */ > - /* Arg0 = APIC ID */ > - method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED); > - for (i = 0; i < apic_ids->len; i++) { > - int cpu_apic_id = apic_ids->cpus[i].arch_id; > - > - if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id))); > - aml_append(if_ctx, > - aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1)) > - ); > - aml_append(method, if_ctx); > - } > - aml_append(sb_scope, method); > - > - /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" > - * > - * Note: The ability to create variable-sized packages was first > - * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages > - * ith up to 255 elements. Windows guests up to win2k8 fail when > - * VarPackageOp is used. > - */ > - pkg = x86ms->apic_id_limit <= 255 ? aml_package(x86ms->apic_id_limit) : > - aml_varpackage(x86ms->apic_id_limit); > - > - for (i = 0, apic_idx = 0; i < apic_ids->len; i++) { > - int cpu_apic_id = apic_ids->cpus[i].arch_id; > - > - for (; apic_idx < cpu_apic_id; apic_idx++) { > - aml_append(pkg, aml_int(0)); > - } > - aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0)); > - apic_idx = cpu_apic_id + 1; > - } > - aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg)); > - aml_append(ctx, sb_scope); > - > - method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED); > - aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD)); > - aml_append(ctx, method); > -} > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 3fffa4a3328..625889783ec 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1465,9 +1465,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > } > aml_append(dsdt, scope); > > - if (pcmc->legacy_cpu_hotplug) { > - build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base); > - } else { > + { > CPUHotplugFeatures opts = { > .acpi_1_compatible = true, .has_legacy_cphp = true, > .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, Do you happen to know exactly why the CPU hotplug ACPI entry was deprecated (it might be helpful to add this to the commit message). Anyhow: Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com> ATB, Mark.
On 2/5/25 10:57, Mark Cave-Ayland wrote: > On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote: > >> The PCMachineClass::legacy_cpu_hotplug boolean was only used >> by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got >> removed. Remove it and simplify build_dsdt(), removing >> build_legacy_cpu_hotplug_aml() altogether. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/acpi/cpu_hotplug.h | 3 - >> include/hw/i386/pc.h | 3 - >> hw/acpi/cpu_hotplug.c | 230 ---------------------------------- >> hw/i386/acpi-build.c | 4 +- >> 4 files changed, 1 insertion(+), 239 deletions(-) >> >> diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/ >> cpu_hotplug.h >> index 3b932abbbbe..aeee630cf05 100644 >> --- a/include/hw/acpi/cpu_hotplug.h >> +++ b/include/hw/acpi/cpu_hotplug.h >> @@ -34,7 +34,4 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion >> *parent, Object *owner, >> void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, >> CPUHotplugState *cpuhp_state, >> uint16_t io_port); >> - >> -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, >> - uint16_t io_base); >> #endif >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 79b72c54dd3..a3de3e9560d 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -110,9 +110,6 @@ struct PCMachineClass { >> bool enforce_amd_1tb_hole; >> bool isa_bios_alias; >> - /* generate legacy CPU hotplug AML */ >> - bool legacy_cpu_hotplug; >> - >> /* use PVH to load kernels that support this feature */ >> bool pvh_enabled; >> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c >> index aa0e1e3efa5..fe439705bda 100644 >> --- a/hw/acpi/cpu_hotplug.c >> +++ b/hw/acpi/cpu_hotplug.c >> @@ -116,233 +116,3 @@ void acpi_switch_to_modern_cphp(AcpiCpuHotplug >> *gpe_cpu, >> memory_region_del_subregion(parent, &gpe_cpu->io); >> cpu_hotplug_hw_init(parent, gpe_cpu->device, cpuhp_state, io_port); >> } >> - >> -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, >> - uint16_t io_base) >> -{ >> - Aml *dev; >> - Aml *crs; >> - Aml *pkg; >> - Aml *field; >> - Aml *method; >> - Aml *if_ctx; >> - Aml *else_ctx; >> - int i, apic_idx; >> - Aml *sb_scope = aml_scope("_SB"); >> - uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0}; >> - Aml *cpu_id = aml_arg(1); >> - Aml *apic_id = aml_arg(0); >> - Aml *cpu_on = aml_local(0); >> - Aml *madt = aml_local(1); >> - Aml *cpus_map = aml_name(CPU_ON_BITMAP); >> - Aml *zero = aml_int(0); >> - Aml *one = aml_int(1); >> - MachineClass *mc = MACHINE_GET_CLASS(machine); >> - const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine); >> - X86MachineState *x86ms = X86_MACHINE(machine); >> - >> - /* >> - * _MAT method - creates an madt apic buffer >> - * apic_id = Arg0 = Local APIC ID >> - * cpu_id = Arg1 = Processor ID >> - * cpu_on = Local0 = CPON flag for this cpu >> - * madt = Local1 = Buffer (in madt apic form) to return >> - */ >> - method = aml_method(CPU_MAT_METHOD, 2, AML_NOTSERIALIZED); >> - aml_append(method, >> - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); >> - aml_append(method, >> - aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt)); >> - /* Update the processor id, lapic id, and enable/disable status */ >> - aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2)))); >> - aml_append(method, aml_store(apic_id, aml_index(madt, aml_int(3)))); >> - aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4)))); >> - aml_append(method, aml_return(madt)); >> - aml_append(sb_scope, method); >> - >> - /* >> - * _STA method - return ON status of cpu >> - * apic_id = Arg0 = Local APIC ID >> - * cpu_on = Local0 = CPON flag for this cpu >> - */ >> - method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED); >> - aml_append(method, >> - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); >> - if_ctx = aml_if(cpu_on); >> - { >> - aml_append(if_ctx, aml_return(aml_int(0xF))); >> - } >> - aml_append(method, if_ctx); >> - else_ctx = aml_else(); >> - { >> - aml_append(else_ctx, aml_return(zero)); >> - } >> - aml_append(method, else_ctx); >> - aml_append(sb_scope, method); >> - >> - method = aml_method(CPU_EJECT_METHOD, 2, AML_NOTSERIALIZED); >> - aml_append(method, aml_sleep(200)); >> - aml_append(sb_scope, method); >> - >> - method = aml_method(CPU_SCAN_METHOD, 0, AML_NOTSERIALIZED); >> - { >> - Aml *while_ctx, *if_ctx2, *else_ctx2; >> - Aml *bus_check_evt = aml_int(1); >> - Aml *remove_evt = aml_int(3); >> - Aml *status_map = aml_local(5); /* Local5 = active cpu bitmap */ >> - Aml *byte = aml_local(2); /* Local2 = last read byte from >> bitmap */ >> - Aml *idx = aml_local(0); /* Processor ID / APIC ID iterator */ >> - Aml *is_cpu_on = aml_local(1); /* Local1 = CPON flag for cpu */ >> - Aml *status = aml_local(3); /* Local3 = active state for cpu */ >> - >> - aml_append(method, aml_store(aml_name(CPU_STATUS_MAP), >> status_map)); >> - aml_append(method, aml_store(zero, byte)); >> - aml_append(method, aml_store(zero, idx)); >> - >> - /* While (idx < SizeOf(CPON)) */ >> - while_ctx = aml_while(aml_lless(idx, aml_sizeof(cpus_map))); >> - aml_append(while_ctx, >> - aml_store(aml_derefof(aml_index(cpus_map, idx)), >> is_cpu_on)); >> - >> - if_ctx = aml_if(aml_and(idx, aml_int(0x07), NULL)); >> - { >> - /* Shift down previously read bitmap byte */ >> - aml_append(if_ctx, aml_shiftright(byte, one, byte)); >> - } >> - aml_append(while_ctx, if_ctx); >> - >> - else_ctx = aml_else(); >> - { >> - /* Read next byte from cpu bitmap */ >> - aml_append(else_ctx, >> aml_store(aml_derefof(aml_index(status_map, >> - aml_shiftright(idx, aml_int(3), NULL))), byte)); >> - } >> - aml_append(while_ctx, else_ctx); >> - >> - aml_append(while_ctx, aml_store(aml_and(byte, one, NULL), >> status)); >> - if_ctx = aml_if(aml_lnot(aml_equal(is_cpu_on, status))); >> - { >> - /* State change - update CPON with new state */ >> - aml_append(if_ctx, aml_store(status, aml_index(cpus_map, >> idx))); >> - if_ctx2 = aml_if(aml_equal(status, one)); >> - { >> - aml_append(if_ctx2, >> - aml_call2(AML_NOTIFY_METHOD, idx, bus_check_evt)); >> - } >> - aml_append(if_ctx, if_ctx2); >> - else_ctx2 = aml_else(); >> - { >> - aml_append(else_ctx2, >> - aml_call2(AML_NOTIFY_METHOD, idx, remove_evt)); >> - } >> - } >> - aml_append(if_ctx, else_ctx2); >> - aml_append(while_ctx, if_ctx); >> - >> - aml_append(while_ctx, aml_increment(idx)); /* go to next cpu */ >> - aml_append(method, while_ctx); >> - } >> - aml_append(sb_scope, method); >> - >> - /* The current AML generator can cover the APIC ID range [0..255], >> - * inclusive, for VCPU hotplug. */ >> - QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); >> - if (x86ms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { >> - error_report("max_cpus is too large. APIC ID of last CPU is %u", >> - x86ms->apic_id_limit - 1); >> - exit(1); >> - } >> - >> - /* create PCI0.PRES device and its _CRS to reserve CPU hotplug >> MMIO */ >> - dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); >> - aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06"))); >> - aml_append(dev, >> - aml_name_decl("_UID", aml_string("CPU Hotplug resources")) >> - ); >> - /* device present, functioning, decoding, not shown in UI */ >> - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); >> - crs = aml_resource_template(); >> - aml_append(crs, >> - aml_io(AML_DECODE16, io_base, io_base, 1, ACPI_GPE_PROC_LEN) >> - ); >> - aml_append(dev, aml_name_decl("_CRS", crs)); >> - aml_append(sb_scope, dev); >> - /* declare CPU hotplug MMIO region and PRS field to access it */ >> - aml_append(sb_scope, aml_operation_region( >> - "PRST", AML_SYSTEM_IO, aml_int(io_base), ACPI_GPE_PROC_LEN)); >> - field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE); >> - aml_append(field, aml_named_field("PRS", 256)); >> - aml_append(sb_scope, field); >> - >> - /* build Processor object for each processor */ >> - for (i = 0; i < apic_ids->len; i++) { >> - int cpu_apic_id = apic_ids->cpus[i].arch_id; >> - >> - assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT); >> - >> - dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id); >> - >> - method = aml_method("_MAT", 0, AML_NOTSERIALIZED); >> - aml_append(method, >> - aml_return(aml_call2(CPU_MAT_METHOD, >> - aml_int(cpu_apic_id), aml_int(i)) >> - )); >> - aml_append(dev, method); >> - >> - method = aml_method("_STA", 0, AML_NOTSERIALIZED); >> - aml_append(method, >> - aml_return(aml_call1(CPU_STATUS_METHOD, >> aml_int(cpu_apic_id)))); >> - aml_append(dev, method); >> - >> - method = aml_method("_EJ0", 1, AML_NOTSERIALIZED); >> - aml_append(method, >> - aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id), >> - aml_arg(0))) >> - ); >> - aml_append(dev, method); >> - >> - aml_append(sb_scope, dev); >> - } >> - >> - /* build this code: >> - * Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, >> Arg1)} ...} >> - */ >> - /* Arg0 = APIC ID */ >> - method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED); >> - for (i = 0; i < apic_ids->len; i++) { >> - int cpu_apic_id = apic_ids->cpus[i].arch_id; >> - >> - if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id))); >> - aml_append(if_ctx, >> - aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1)) >> - ); >> - aml_append(method, if_ctx); >> - } >> - aml_append(sb_scope, method); >> - >> - /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" >> - * >> - * Note: The ability to create variable-sized packages was first >> - * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages >> - * ith up to 255 elements. Windows guests up to win2k8 fail when >> - * VarPackageOp is used. >> - */ >> - pkg = x86ms->apic_id_limit <= 255 ? aml_package(x86ms- >> >apic_id_limit) : >> - aml_varpackage(x86ms- >> >apic_id_limit); >> - >> - for (i = 0, apic_idx = 0; i < apic_ids->len; i++) { >> - int cpu_apic_id = apic_ids->cpus[i].arch_id; >> - >> - for (; apic_idx < cpu_apic_id; apic_idx++) { >> - aml_append(pkg, aml_int(0)); >> - } >> - aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0)); >> - apic_idx = cpu_apic_id + 1; >> - } >> - aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg)); >> - aml_append(ctx, sb_scope); >> - >> - method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED); >> - aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD)); >> - aml_append(ctx, method); >> -} >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 3fffa4a3328..625889783ec 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1465,9 +1465,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> } >> aml_append(dsdt, scope); >> - if (pcmc->legacy_cpu_hotplug) { >> - build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base); >> - } else { >> + { >> CPUHotplugFeatures opts = { >> .acpi_1_compatible = true, .has_legacy_cphp = true, >> .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : >> NULL, > > Do you happen to know exactly why the CPU hotplug ACPI entry was > deprecated (it might be helpful to add this to the commit message). I'll mention: commit 679dd1a957df418453efdd3ed2914dba5cd73773 Author: Igor Mammedov <imammedo@redhat.com> Date: Wed Jun 15 11:25:23 2016 +0200 pc: use new CPU hotplug interface since 2.7 machine type 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. That way legacy firmware that doesn't use QEMU generated ACPI tables will be able to continue using legacy CPU hotplug interface. While new machine type, with firmware supporting QEMU provided ACPI tables, will generate new CPU hotplug AML, which will switch to new CPU hotplug interface when guest OS executes its _INI method on ACPI tables loading. > Anyhow: > > Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com> Thanks!
On 01/05/2025 20.36, Philippe Mathieu-Daudé wrote: > The PCMachineClass::legacy_cpu_hotplug boolean was only used > by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got > removed. Remove it and simplify build_dsdt(), removing > build_legacy_cpu_hotplug_aml() altogether. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/acpi/cpu_hotplug.h | 3 - > include/hw/i386/pc.h | 3 - > hw/acpi/cpu_hotplug.c | 230 ---------------------------------- > hw/i386/acpi-build.c | 4 +- > 4 files changed, 1 insertion(+), 239 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index 3b932abbbbe..aeee630cf05 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -34,7 +34,4 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, CPUHotplugState *cpuhp_state, uint16_t io_port); - -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, - uint16_t io_base); #endif diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 79b72c54dd3..a3de3e9560d 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -110,9 +110,6 @@ struct PCMachineClass { bool enforce_amd_1tb_hole; bool isa_bios_alias; - /* generate legacy CPU hotplug AML */ - bool legacy_cpu_hotplug; - /* use PVH to load kernels that support this feature */ bool pvh_enabled; diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index aa0e1e3efa5..fe439705bda 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -116,233 +116,3 @@ void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, memory_region_del_subregion(parent, &gpe_cpu->io); cpu_hotplug_hw_init(parent, gpe_cpu->device, cpuhp_state, io_port); } - -void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine, - uint16_t io_base) -{ - Aml *dev; - Aml *crs; - Aml *pkg; - Aml *field; - Aml *method; - Aml *if_ctx; - Aml *else_ctx; - int i, apic_idx; - Aml *sb_scope = aml_scope("_SB"); - uint8_t madt_tmpl[8] = {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0}; - Aml *cpu_id = aml_arg(1); - Aml *apic_id = aml_arg(0); - Aml *cpu_on = aml_local(0); - Aml *madt = aml_local(1); - Aml *cpus_map = aml_name(CPU_ON_BITMAP); - Aml *zero = aml_int(0); - Aml *one = aml_int(1); - MachineClass *mc = MACHINE_GET_CLASS(machine); - const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine); - X86MachineState *x86ms = X86_MACHINE(machine); - - /* - * _MAT method - creates an madt apic buffer - * apic_id = Arg0 = Local APIC ID - * cpu_id = Arg1 = Processor ID - * cpu_on = Local0 = CPON flag for this cpu - * madt = Local1 = Buffer (in madt apic form) to return - */ - method = aml_method(CPU_MAT_METHOD, 2, AML_NOTSERIALIZED); - aml_append(method, - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); - aml_append(method, - aml_store(aml_buffer(sizeof(madt_tmpl), madt_tmpl), madt)); - /* Update the processor id, lapic id, and enable/disable status */ - aml_append(method, aml_store(cpu_id, aml_index(madt, aml_int(2)))); - aml_append(method, aml_store(apic_id, aml_index(madt, aml_int(3)))); - aml_append(method, aml_store(cpu_on, aml_index(madt, aml_int(4)))); - aml_append(method, aml_return(madt)); - aml_append(sb_scope, method); - - /* - * _STA method - return ON status of cpu - * apic_id = Arg0 = Local APIC ID - * cpu_on = Local0 = CPON flag for this cpu - */ - method = aml_method(CPU_STATUS_METHOD, 1, AML_NOTSERIALIZED); - aml_append(method, - aml_store(aml_derefof(aml_index(cpus_map, apic_id)), cpu_on)); - if_ctx = aml_if(cpu_on); - { - aml_append(if_ctx, aml_return(aml_int(0xF))); - } - aml_append(method, if_ctx); - else_ctx = aml_else(); - { - aml_append(else_ctx, aml_return(zero)); - } - aml_append(method, else_ctx); - aml_append(sb_scope, method); - - method = aml_method(CPU_EJECT_METHOD, 2, AML_NOTSERIALIZED); - aml_append(method, aml_sleep(200)); - aml_append(sb_scope, method); - - method = aml_method(CPU_SCAN_METHOD, 0, AML_NOTSERIALIZED); - { - Aml *while_ctx, *if_ctx2, *else_ctx2; - Aml *bus_check_evt = aml_int(1); - Aml *remove_evt = aml_int(3); - Aml *status_map = aml_local(5); /* Local5 = active cpu bitmap */ - Aml *byte = aml_local(2); /* Local2 = last read byte from bitmap */ - Aml *idx = aml_local(0); /* Processor ID / APIC ID iterator */ - Aml *is_cpu_on = aml_local(1); /* Local1 = CPON flag for cpu */ - Aml *status = aml_local(3); /* Local3 = active state for cpu */ - - aml_append(method, aml_store(aml_name(CPU_STATUS_MAP), status_map)); - aml_append(method, aml_store(zero, byte)); - aml_append(method, aml_store(zero, idx)); - - /* While (idx < SizeOf(CPON)) */ - while_ctx = aml_while(aml_lless(idx, aml_sizeof(cpus_map))); - aml_append(while_ctx, - aml_store(aml_derefof(aml_index(cpus_map, idx)), is_cpu_on)); - - if_ctx = aml_if(aml_and(idx, aml_int(0x07), NULL)); - { - /* Shift down previously read bitmap byte */ - aml_append(if_ctx, aml_shiftright(byte, one, byte)); - } - aml_append(while_ctx, if_ctx); - - else_ctx = aml_else(); - { - /* Read next byte from cpu bitmap */ - aml_append(else_ctx, aml_store(aml_derefof(aml_index(status_map, - aml_shiftright(idx, aml_int(3), NULL))), byte)); - } - aml_append(while_ctx, else_ctx); - - aml_append(while_ctx, aml_store(aml_and(byte, one, NULL), status)); - if_ctx = aml_if(aml_lnot(aml_equal(is_cpu_on, status))); - { - /* State change - update CPON with new state */ - aml_append(if_ctx, aml_store(status, aml_index(cpus_map, idx))); - if_ctx2 = aml_if(aml_equal(status, one)); - { - aml_append(if_ctx2, - aml_call2(AML_NOTIFY_METHOD, idx, bus_check_evt)); - } - aml_append(if_ctx, if_ctx2); - else_ctx2 = aml_else(); - { - aml_append(else_ctx2, - aml_call2(AML_NOTIFY_METHOD, idx, remove_evt)); - } - } - aml_append(if_ctx, else_ctx2); - aml_append(while_ctx, if_ctx); - - aml_append(while_ctx, aml_increment(idx)); /* go to next cpu */ - aml_append(method, while_ctx); - } - aml_append(sb_scope, method); - - /* The current AML generator can cover the APIC ID range [0..255], - * inclusive, for VCPU hotplug. */ - QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); - if (x86ms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { - error_report("max_cpus is too large. APIC ID of last CPU is %u", - x86ms->apic_id_limit - 1); - exit(1); - } - - /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ - dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); - aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06"))); - aml_append(dev, - aml_name_decl("_UID", aml_string("CPU Hotplug resources")) - ); - /* device present, functioning, decoding, not shown in UI */ - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); - crs = aml_resource_template(); - aml_append(crs, - aml_io(AML_DECODE16, io_base, io_base, 1, ACPI_GPE_PROC_LEN) - ); - aml_append(dev, aml_name_decl("_CRS", crs)); - aml_append(sb_scope, dev); - /* declare CPU hotplug MMIO region and PRS field to access it */ - aml_append(sb_scope, aml_operation_region( - "PRST", AML_SYSTEM_IO, aml_int(io_base), ACPI_GPE_PROC_LEN)); - field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE); - aml_append(field, aml_named_field("PRS", 256)); - aml_append(sb_scope, field); - - /* build Processor object for each processor */ - for (i = 0; i < apic_ids->len; i++) { - int cpu_apic_id = apic_ids->cpus[i].arch_id; - - assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT); - - dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id); - - method = aml_method("_MAT", 0, AML_NOTSERIALIZED); - aml_append(method, - aml_return(aml_call2(CPU_MAT_METHOD, - aml_int(cpu_apic_id), aml_int(i)) - )); - aml_append(dev, method); - - method = aml_method("_STA", 0, AML_NOTSERIALIZED); - aml_append(method, - aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id)))); - aml_append(dev, method); - - method = aml_method("_EJ0", 1, AML_NOTSERIALIZED); - aml_append(method, - aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id), - aml_arg(0))) - ); - aml_append(dev, method); - - aml_append(sb_scope, dev); - } - - /* build this code: - * Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...} - */ - /* Arg0 = APIC ID */ - method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED); - for (i = 0; i < apic_ids->len; i++) { - int cpu_apic_id = apic_ids->cpus[i].arch_id; - - if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id))); - aml_append(if_ctx, - aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1)) - ); - aml_append(method, if_ctx); - } - aml_append(sb_scope, method); - - /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" - * - * Note: The ability to create variable-sized packages was first - * introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size packages - * ith up to 255 elements. Windows guests up to win2k8 fail when - * VarPackageOp is used. - */ - pkg = x86ms->apic_id_limit <= 255 ? aml_package(x86ms->apic_id_limit) : - aml_varpackage(x86ms->apic_id_limit); - - for (i = 0, apic_idx = 0; i < apic_ids->len; i++) { - int cpu_apic_id = apic_ids->cpus[i].arch_id; - - for (; apic_idx < cpu_apic_id; apic_idx++) { - aml_append(pkg, aml_int(0)); - } - aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0)); - apic_idx = cpu_apic_id + 1; - } - aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg)); - aml_append(ctx, sb_scope); - - method = aml_method("\\_GPE._E02", 0, AML_NOTSERIALIZED); - aml_append(method, aml_call0("\\_SB." CPU_SCAN_METHOD)); - aml_append(ctx, method); -} diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 3fffa4a3328..625889783ec 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1465,9 +1465,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, } aml_append(dsdt, scope); - if (pcmc->legacy_cpu_hotplug) { - build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base); - } else { + { CPUHotplugFeatures opts = { .acpi_1_compatible = true, .has_legacy_cphp = true, .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL,
The PCMachineClass::legacy_cpu_hotplug boolean was only used by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got removed. Remove it and simplify build_dsdt(), removing build_legacy_cpu_hotplug_aml() altogether. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/acpi/cpu_hotplug.h | 3 - include/hw/i386/pc.h | 3 - hw/acpi/cpu_hotplug.c | 230 ---------------------------------- hw/i386/acpi-build.c | 4 +- 4 files changed, 1 insertion(+), 239 deletions(-)