On Thu, 20 Jul 2023 15:15:07 +0800
xianglai li <lixiang...@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 <yangxiaoj...@loongson.cn>
> Cc: Song Gao <gaos...@loongson.cn>
> Cc: "Michael S. Tsirkin" <m...@redhat.com>
> Cc: Igor Mammedov <imamm...@redhat.com>
> Cc: Ani Sinha <anisi...@redhat.com>
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Cc: Richard Henderson <richard.hender...@linaro.org>
> Cc: Eduardo Habkost <edua...@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelb...@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <phi...@linaro.org>
> Cc: Yanan Wang <wangyana...@huawei.com>
> Cc: "Daniel P. Berrangé" <berra...@redhat.com>
> Cc: Peter Xu <pet...@redhat.com>
> Cc: David Hildenbrand <da...@redhat.com>
> Signed-off-by: xianglai li <lixiang...@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);
>
