| Message ID | d40ea40fe8290160f95a79515bebc20c4fbfe48f.1689837093.git.lixianglai@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Adds CPU hot-plug support to Loongarch | expand |
On Thu, 20 Jul 2023 15:15:07 +0800 xianglai li <lixianglai@loongson.cn> wrote: > CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on > PCI and is IO port based and hence existing cpus AML code assumes _CRS objects > would evaluate to a system resource which describes IO Port address. But on LOONGARCH > arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence > _CRS object should evaluate to system resource which describes memory-mapped > base address. > > This cpus AML code change updates the existing inerface of the build cpus AML ^^^ typo > function to accept both IO/MEMORY type regions and update the _CRS object > correspondingly. try to reformat commit message to less than 80 character per line > > Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> > Cc: Song Gao <gaosong@loongson.cn> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Ani Sinha <anisinha@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Eduardo Habkost <eduardo@habkost.net> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> > Cc: Yanan Wang <wangyanan55@huawei.com> > Cc: "Daniel P. Berrangé" <berrange@redhat.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: David Hildenbrand <david@redhat.com> > Signed-off-by: xianglai li <lixianglai@loongson.cn> > --- > hw/acpi/cpu.c | 30 +++++++++++++++++++++++------- > hw/i386/acpi-build.c | 2 +- > include/hw/acpi/cpu.h | 5 +++-- > 3 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index 6897c8789a..3b945a1a40 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -5,6 +5,7 @@ > #include "qapi/qapi-events-acpi.h" > #include "trace.h" > #include "sysemu/numa.h" > +#include "hw/acpi/cpu_hotplug.h" > > #define OVMF_CPUHP_SMI_CMD 4 > > @@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = { > #define CPU_FW_EJECT_EVENT "CEJF" > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > - hwaddr io_base, > + hwaddr mmap_io_base, > const char *res_root, > - const char *event_handler_method) > + const char *event_handler_method, > + AmlRegionSpace rs) > { > Aml *ifctx; > Aml *field; > @@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0)); > > crs = aml_resource_template(); > - aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1, > + if (rs == AML_SYSTEM_IO) { > + aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1, > ACPI_CPU_HOTPLUG_REG_LEN)); > + } else { > + aml_append(crs, aml_memory32_fixed(mmap_io_base, > + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); > + } > aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); > > - /* declare CPU hotplug MMIO region with related access fields */ > - aml_append(cpu_ctrl_dev, > - aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base), > - ACPI_CPU_HOTPLUG_REG_LEN)); > + if (rs == AML_SYSTEM_IO) { > + /* declare CPU hotplug MMIO region with related access fields */ > + aml_append(cpu_ctrl_dev, > + aml_operation_region("PRST", AML_SYSTEM_IO, > + aml_int(mmap_io_base), > + ACPI_CPU_HOTPLUG_REG_LEN)); > + } else { > + aml_append(cpu_ctrl_dev, > + aml_operation_region("PRST", AML_SYSTEM_MEMORY, > + aml_int(mmap_io_base), > + ACPI_CPU_HOTPLUG_REG_LEN)); > + } to reduce duplication, following could be better way to spell it: g_assert(rs == foo1 || rs == foo2) ... aml_operation_region("PRST", rs, aml_int(io_base), ... > > field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, > AML_WRITE_AS_ZEROS); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 9c74fa17ad..5d02690593 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > .fw_unplugs_cpu = pm->smi_on_cpu_unplug, > }; > build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, > - "\\_SB.PCI0", "\\_GPE._E02"); > + "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO); > } > > if (pcms->memhp_io_base && nr_mem) { > diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h > index 999caaf510..cddea78333 100644 > --- a/include/hw/acpi/cpu.h > +++ b/include/hw/acpi/cpu.h > @@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures { > } CPUHotplugFeatures; > > void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > - hwaddr io_base, > + hwaddr mmap_io_base, > const char *res_root, > - const char *event_handler_method); > + const char *event_handler_method, > + AmlRegionSpace rs); > > void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list); >
Hi, Igor Mammedov : On 7/28/23 7:55 PM, Igor Mammedov wrote: > On Thu, 20 Jul 2023 15:15:07 +0800 > xianglai li <lixianglai@loongson.cn> wrote: > >> CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on >> PCI and is IO port based and hence existing cpus AML code assumes _CRS objects >> would evaluate to a system resource which describes IO Port address. But on LOONGARCH >> arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence >> _CRS object should evaluate to system resource which describes memory-mapped >> base address. >> >> This cpus AML code change updates the existing inerface of the build cpus AML > ^^^ typo Oh, I'm sorry for the typo. >> function to accept both IO/MEMORY type regions and update the _CRS object >> correspondingly. > try to reformat commit message to less than 80 character per line Ok! I'll modify it as suggested in the next version of patch. >> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> >> Cc: Song Gao <gaosong@loongson.cn> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Ani Sinha <anisinha@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Richard Henderson <richard.henderson@linaro.org> >> Cc: Eduardo Habkost <eduardo@habkost.net> >> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> >> Cc: Yanan Wang <wangyanan55@huawei.com> >> Cc: "Daniel P. Berrangé" <berrange@redhat.com> >> Cc: Peter Xu <peterx@redhat.com> >> Cc: David Hildenbrand <david@redhat.com> >> Signed-off-by: xianglai li <lixianglai@loongson.cn> >> --- >> hw/acpi/cpu.c | 30 +++++++++++++++++++++++------- >> hw/i386/acpi-build.c | 2 +- >> include/hw/acpi/cpu.h | 5 +++-- >> 3 files changed, 27 insertions(+), 10 deletions(-) >> >> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c >> index 6897c8789a..3b945a1a40 100644 >> --- a/hw/acpi/cpu.c >> +++ b/hw/acpi/cpu.c >> @@ -5,6 +5,7 @@ >> #include "qapi/qapi-events-acpi.h" >> #include "trace.h" >> #include "sysemu/numa.h" >> +#include "hw/acpi/cpu_hotplug.h" >> >> #define OVMF_CPUHP_SMI_CMD 4 >> >> @@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = { >> #define CPU_FW_EJECT_EVENT "CEJF" >> >> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> - hwaddr io_base, >> + hwaddr mmap_io_base, >> const char *res_root, >> - const char *event_handler_method) >> + const char *event_handler_method, >> + AmlRegionSpace rs) >> { >> Aml *ifctx; >> Aml *field; >> @@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0)); >> >> crs = aml_resource_template(); >> - aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1, >> + if (rs == AML_SYSTEM_IO) { >> + aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1, >> ACPI_CPU_HOTPLUG_REG_LEN)); >> + } else { >> + aml_append(crs, aml_memory32_fixed(mmap_io_base, >> + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); >> + } >> aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); >> >> - /* declare CPU hotplug MMIO region with related access fields */ >> - aml_append(cpu_ctrl_dev, >> - aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base), >> - ACPI_CPU_HOTPLUG_REG_LEN)); >> + if (rs == AML_SYSTEM_IO) { >> + /* declare CPU hotplug MMIO region with related access fields */ >> + aml_append(cpu_ctrl_dev, >> + aml_operation_region("PRST", AML_SYSTEM_IO, >> + aml_int(mmap_io_base), >> + ACPI_CPU_HOTPLUG_REG_LEN)); >> + } else { >> + aml_append(cpu_ctrl_dev, >> + aml_operation_region("PRST", AML_SYSTEM_MEMORY, >> + aml_int(mmap_io_base), >> + ACPI_CPU_HOTPLUG_REG_LEN)); >> + } > > to reduce duplication, following could be better way to spell it: > g_assert(rs == foo1 || rs == foo2) > ... aml_operation_region("PRST", rs, aml_int(io_base), ... Well, this suggestion makes the code look good, I'll fix it in the next patch release. >> >> field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, >> AML_WRITE_AS_ZEROS); >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 9c74fa17ad..5d02690593 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> .fw_unplugs_cpu = pm->smi_on_cpu_unplug, >> }; >> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, >> - "\\_SB.PCI0", "\\_GPE._E02"); >> + "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO); >> } >> >> if (pcms->memhp_io_base && nr_mem) { >> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h >> index 999caaf510..cddea78333 100644 >> --- a/include/hw/acpi/cpu.h >> +++ b/include/hw/acpi/cpu.h >> @@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures { >> } CPUHotplugFeatures; >> >> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, >> - hwaddr io_base, >> + hwaddr mmap_io_base, >> const char *res_root, >> - const char *event_handler_method); >> + const char *event_handler_method, >> + AmlRegionSpace rs); >> >> void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list); >> Thanks! xianglai.
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 6897c8789a..3b945a1a40 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -5,6 +5,7 @@ #include "qapi/qapi-events-acpi.h" #include "trace.h" #include "sysemu/numa.h" +#include "hw/acpi/cpu_hotplug.h" #define OVMF_CPUHP_SMI_CMD 4 @@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = { #define CPU_FW_EJECT_EVENT "CEJF" void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, - hwaddr io_base, + hwaddr mmap_io_base, const char *res_root, - const char *event_handler_method) + const char *event_handler_method, + AmlRegionSpace rs) { Aml *ifctx; Aml *field; @@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0)); crs = aml_resource_template(); - aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1, + if (rs == AML_SYSTEM_IO) { + aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1, ACPI_CPU_HOTPLUG_REG_LEN)); + } else { + aml_append(crs, aml_memory32_fixed(mmap_io_base, + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); + } + aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); - /* declare CPU hotplug MMIO region with related access fields */ - aml_append(cpu_ctrl_dev, - aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base), - ACPI_CPU_HOTPLUG_REG_LEN)); + if (rs == AML_SYSTEM_IO) { + /* declare CPU hotplug MMIO region with related access fields */ + aml_append(cpu_ctrl_dev, + aml_operation_region("PRST", AML_SYSTEM_IO, + aml_int(mmap_io_base), + ACPI_CPU_HOTPLUG_REG_LEN)); + } else { + aml_append(cpu_ctrl_dev, + aml_operation_region("PRST", AML_SYSTEM_MEMORY, + aml_int(mmap_io_base), + ACPI_CPU_HOTPLUG_REG_LEN)); + } field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9c74fa17ad..5d02690593 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, .fw_unplugs_cpu = pm->smi_on_cpu_unplug, }; build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, - "\\_SB.PCI0", "\\_GPE._E02"); + "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO); } if (pcms->memhp_io_base && nr_mem) { diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index 999caaf510..cddea78333 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures { } CPUHotplugFeatures; void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, - hwaddr io_base, + hwaddr mmap_io_base, const char *res_root, - const char *event_handler_method); + const char *event_handler_method, + AmlRegionSpace rs); void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on PCI and is IO port based and hence existing cpus AML code assumes _CRS objects would evaluate to a system resource which describes IO Port address. But on LOONGARCH arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence _CRS object should evaluate to system resource which describes memory-mapped base address. This cpus AML code change updates the existing inerface of the build cpus AML function to accept both IO/MEMORY type regions and update the _CRS object correspondingly. Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> Cc: Song Gao <gaosong@loongson.cn> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Ani Sinha <anisinha@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Eduardo Habkost <eduardo@habkost.net> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> Cc: Yanan Wang <wangyanan55@huawei.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Peter Xu <peterx@redhat.com> Cc: David Hildenbrand <david@redhat.com> Signed-off-by: xianglai li <lixianglai@loongson.cn> --- hw/acpi/cpu.c | 30 +++++++++++++++++++++++------- hw/i386/acpi-build.c | 2 +- include/hw/acpi/cpu.h | 5 +++-- 3 files changed, 27 insertions(+), 10 deletions(-)