| Message ID | 20201204170939.1815522-6-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for cpu hot-unplug with SMI broadcast enabled | expand |
On 2020-12-04 9:09 a.m., Igor Mammedov wrote: > if firmware and QEMU negotiated CPU hotunplug support, generate > _EJ0 method so that it will mark CPU for removal by firmware and > pass control to it by triggering SMI. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > include/hw/acpi/cpu.h | 1 + > hw/acpi/cpu.c | 15 +++++++++++++-- > hw/i386/acpi-build.c | 1 + > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h > index d71edde456..999caaf510 100644 > --- a/include/hw/acpi/cpu.h > +++ b/include/hw/acpi/cpu.h > @@ -51,6 +51,7 @@ 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; > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index 811218f673..bded2a837f 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = { > #define CPU_INSERT_EVENT "CINS" > #define CPU_REMOVE_EVENT "CRMV" > #define CPU_EJECT_EVENT "CEJ0" > +#define CPU_FW_EJECT_EVENT "CEJF" > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > hwaddr io_base, > @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1)); Bit 2: Device remove event, used to distinguish device for which no device eject request to OSPM was issued. Firmware must ignore this bit. > /* initiates device eject, write only */ > aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1)); Bit 3: reserved and should be ignored by OSPM > - aml_append(field, aml_reserved_field(4)); > + aml_append(field, aml_reserved_field(1)); > + /* tell firmware to do device eject, write only */ > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); > + aml_append(field, aml_reserved_field(2)); Shouldn't this be instead: > - aml_append(field, aml_reserved_field(4)); > + /* tell firmware to do device eject, write only */ > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); > + aml_append(field, aml_reserved_field(3)); Bit 4: if set to 1, OSPM requests firmware to perform device eject. Bit 5-7: reserved and should be ignored by OSPM Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5. Ankur > aml_append(field, aml_named_field(CPU_COMMAND, 8)); > aml_append(cpu_ctrl_dev, field); > > @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); > Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT); > Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT); > + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT); > > aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010"))); > aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05"))); > @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); > aml_append(method, aml_store(idx, cpu_selector)); > - aml_append(method, aml_store(one, ej_evt)); > + if (opts.fw_unplugs_cpu) { > + aml_append(method, aml_store(one, fw_ej_evt)); > + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD), > + aml_name("%s", opts.smi_path))); > + } else { > + aml_append(method, aml_store(one, ej_evt)); > + } > aml_append(method, aml_release(ctrl_lock)); > } > aml_append(cpus_dev, method); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 9036e5594c..475e76f514 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > CPUHotplugFeatures opts = { > .acpi_1_compatible = true, .has_legacy_cphp = true, > .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, > + .fw_unplugs_cpu = pm->smi_on_cpu_unplug, > }; > build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, > "\\_SB.PCI0", "\\_GPE._E02"); >
On 2020-12-06 10:20 p.m., Ankur Arora wrote: > On 2020-12-04 9:09 a.m., Igor Mammedov wrote: >> if firmware and QEMU negotiated CPU hotunplug support, generate >> _EJ0 method so that it will mark CPU for removal by firmware and >> pass control to it by triggering SMI. >> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> --- >> include/hw/acpi/cpu.h | 1 + >> hw/acpi/cpu.c | 15 +++++++++++++-- >> hw/i386/acpi-build.c | 1 + >> 3 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h >> index d71edde456..999caaf510 100644 >> --- a/include/hw/acpi/cpu.h >> +++ b/include/hw/acpi/cpu.h >> @@ -51,6 +51,7 @@ 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; >> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c >> index 811218f673..bded2a837f 100644 >> --- a/hw/acpi/cpu.c >> +++ b/hw/acpi/cpu.c >> @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = { >> #define CPU_INSERT_EVENT "CINS" >> #define CPU_REMOVE_EVENT "CRMV" >> #define CPU_EJECT_EVENT "CEJ0" >> +#define CPU_FW_EJECT_EVENT "CEJF" >> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> hwaddr io_base, >> @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1)); > > Bit 2: Device remove event, used to distinguish device for which > no device eject request to OSPM was issued. Firmware must > ignore this bit. > >> /* initiates device eject, write only */ >> aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1)); > > Bit 3: reserved and should be ignored by OSPM In write-access mode: Bit 3: if set to 1 initiates device eject, set by OSPM when it triggers CPU device removal and calls _EJ0 method or by firmware when bit #4 is set. In case bit #4 were set, it's cleared as part of device eject. (and so should not be a reserved_field() below) > >> - aml_append(field, aml_reserved_field(4)); >> + aml_append(field, aml_reserved_field(1)); >> + /* tell firmware to do device eject, write only */ >> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); >> + aml_append(field, aml_reserved_field(2)); > > Shouldn't this be instead: > >> - aml_append(field, aml_reserved_field(4)); >> + /* tell firmware to do device eject, write only */ >> + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); >> + aml_append(field, aml_reserved_field(3)); > > Bit 4: if set to 1, OSPM requests firmware to perform device eject. > Bit 5-7: reserved and should be ignored by OSPM > > Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5. > > > Ankur > >> aml_append(field, aml_named_field(CPU_COMMAND, 8)); >> aml_append(cpu_ctrl_dev, field); >> @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); >> Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT); >> Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT); >> + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT); >> aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010"))); >> aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05"))); >> @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); >> aml_append(method, aml_store(idx, cpu_selector)); >> - aml_append(method, aml_store(one, ej_evt)); >> + if (opts.fw_unplugs_cpu) { >> + aml_append(method, aml_store(one, fw_ej_evt)); >> + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD), >> + aml_name("%s", opts.smi_path))); >> + } else { >> + aml_append(method, aml_store(one, ej_evt)); >> + } >> aml_append(method, aml_release(ctrl_lock)); >> } >> aml_append(cpus_dev, method); >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 9036e5594c..475e76f514 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> CPUHotplugFeatures opts = { >> .acpi_1_compatible = true, .has_legacy_cphp = true, >> .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, >> + .fw_unplugs_cpu = pm->smi_on_cpu_unplug, >> }; >> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, >> "\\_SB.PCI0", "\\_GPE._E02"); >>
On Sun, 6 Dec 2020 22:20:27 -0800 Ankur Arora <ankur.a.arora@oracle.com> wrote: > On 2020-12-04 9:09 a.m., Igor Mammedov wrote: > > if firmware and QEMU negotiated CPU hotunplug support, generate > > _EJ0 method so that it will mark CPU for removal by firmware and > > pass control to it by triggering SMI. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > include/hw/acpi/cpu.h | 1 + > > hw/acpi/cpu.c | 15 +++++++++++++-- > > hw/i386/acpi-build.c | 1 + > > 3 files changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h > > index d71edde456..999caaf510 100644 > > --- a/include/hw/acpi/cpu.h > > +++ b/include/hw/acpi/cpu.h > > @@ -51,6 +51,7 @@ 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; > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > index 811218f673..bded2a837f 100644 > > --- a/hw/acpi/cpu.c > > +++ b/hw/acpi/cpu.c > > @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = { > > #define CPU_INSERT_EVENT "CINS" > > #define CPU_REMOVE_EVENT "CRMV" > > #define CPU_EJECT_EVENT "CEJ0" > > +#define CPU_FW_EJECT_EVENT "CEJF" > > > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > hwaddr io_base, > > @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1)); > > Bit 2: Device remove event, used to distinguish device for which > no device eject request to OSPM was issued. Firmware must > ignore this bit. > > > /* initiates device eject, write only */ > > aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1)); > > Bit 3: reserved and should be ignored by OSPM > > > - aml_append(field, aml_reserved_field(4)); > > + aml_append(field, aml_reserved_field(1)); > > + /* tell firmware to do device eject, write only */ > > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); > > + aml_append(field, aml_reserved_field(2)); > > Shouldn't this be instead: > > > - aml_append(field, aml_reserved_field(4)); > > + /* tell firmware to do device eject, write only */ > > + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); > > + aml_append(field, aml_reserved_field(3)); > yes, it should be this way, I'll fix in v2 > Bit 4: if set to 1, OSPM requests firmware to perform device eject. > Bit 5-7: reserved and should be ignored by OSPM > > Otherwise AFAICS CPU_FW_EJECT_EVENT would correspond to bit 5. > > > Ankur > > > aml_append(field, aml_named_field(CPU_COMMAND, 8)); > > aml_append(cpu_ctrl_dev, field); > > > > @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); > > Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT); > > Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT); > > + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT); > > > > aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010"))); > > aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05"))); > > @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > > > aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); > > aml_append(method, aml_store(idx, cpu_selector)); > > - aml_append(method, aml_store(one, ej_evt)); > > + if (opts.fw_unplugs_cpu) { > > + aml_append(method, aml_store(one, fw_ej_evt)); > > + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD), > > + aml_name("%s", opts.smi_path))); > > + } else { > > + aml_append(method, aml_store(one, ej_evt)); > > + } > > aml_append(method, aml_release(ctrl_lock)); > > } > > aml_append(cpus_dev, method); > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 9036e5594c..475e76f514 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > CPUHotplugFeatures opts = { > > .acpi_1_compatible = true, .has_legacy_cphp = true, > > .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, > > + .fw_unplugs_cpu = pm->smi_on_cpu_unplug, > > }; > > build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, > > "\\_SB.PCI0", "\\_GPE._E02"); > > >
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index d71edde456..999caaf510 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -51,6 +51,7 @@ 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; diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 811218f673..bded2a837f 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -341,6 +341,7 @@ const VMStateDescription vmstate_cpu_hotplug = { #define CPU_INSERT_EVENT "CINS" #define CPU_REMOVE_EVENT "CRMV" #define CPU_EJECT_EVENT "CEJ0" +#define CPU_FW_EJECT_EVENT "CEJF" void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, hwaddr io_base, @@ -393,7 +394,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1)); /* initiates device eject, write only */ aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1)); - aml_append(field, aml_reserved_field(4)); + aml_append(field, aml_reserved_field(1)); + /* tell firmware to do device eject, write only */ + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); + aml_append(field, aml_reserved_field(2)); aml_append(field, aml_named_field(CPU_COMMAND, 8)); aml_append(cpu_ctrl_dev, field); @@ -428,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT); Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT); + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT); aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010"))); aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05"))); @@ -470,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); aml_append(method, aml_store(idx, cpu_selector)); - aml_append(method, aml_store(one, ej_evt)); + if (opts.fw_unplugs_cpu) { + aml_append(method, aml_store(one, fw_ej_evt)); + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD), + aml_name("%s", opts.smi_path))); + } else { + aml_append(method, aml_store(one, ej_evt)); + } aml_append(method, aml_release(ctrl_lock)); } aml_append(cpus_dev, method); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9036e5594c..475e76f514 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1586,6 +1586,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, CPUHotplugFeatures opts = { .acpi_1_compatible = true, .has_legacy_cphp = true, .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, + .fw_unplugs_cpu = pm->smi_on_cpu_unplug, }; build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02");
if firmware and QEMU negotiated CPU hotunplug support, generate _EJ0 method so that it will mark CPU for removal by firmware and pass control to it by triggering SMI. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- include/hw/acpi/cpu.h | 1 + hw/acpi/cpu.c | 15 +++++++++++++-- hw/i386/acpi-build.c | 1 + 3 files changed, 15 insertions(+), 2 deletions(-)