[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH V2 05/10] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
From: |
Salil Mehta |
Subject: |
RE: [PATCH V2 05/10] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change |
Date: |
Tue, 3 Oct 2023 11:31:41 +0000 |
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: Monday, October 2, 2023 5:10 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; maz@kernel.org; jean-
> philippe@linaro.org; lpieralisi@kernel.org; peter.maydell@linaro.org;
> richard.henderson@linaro.org; imammedo@redhat.com; andrew.jones@linux.dev;
> david@redhat.com; philmd@linaro.org; eric.auger@redhat.com;
> oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com;
> will@kernel.org; gshan@redhat.com; rafael@kernel.org;
> alex.bennee@linaro.org; linux@armlinux.org.uk;
> darren@os.amperecomputing.com; ilkka@os.amperecomputing.com;
> vishnu@os.amperecomputing.com; karl.heubaum@oracle.com;
> miguel.luis@oracle.com; salil.mehta@opnsrc.net; zhukeqian
> <zhukeqian1@huawei.com>; wangxiongfeng (C) <wangxiongfeng2@huawei.com>;
> wangyanan (Y) <wangyanan55@huawei.com>; jiakernel2@gmail.com;
> maobibo@loongson.cn; lixianglai@loongson.cn; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH V2 05/10] hw/acpi: Update CPUs AML with cpu-(ctrl)dev
> change
>
> On Sat, 30 Sep 2023 01:19:28 +0100
> Salil Mehta <salil.mehta@huawei.com> 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
> > ARM
> > 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.
>
> This also makes a change to make the event_handler_method optional.
> Would be good to explain why or leave that for a future patch.
Will add.
Thanks
Salil.
>
> Otherwise looks fine.
>
> >
> > Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> > Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > hw/acpi/cpu.c | 23 ++++++++++++++++-------
> > hw/i386/acpi-build.c | 2 +-
> > include/hw/acpi/cpu.h | 5 +++--
> > 3 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index 45defdc0e2..66a71660ec 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -338,9 +338,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 base_addr,
> > const char *res_root,
> > - const char *event_handler_method)
> > + const char *event_handler_method,
> > + AmlRegionSpace rs)
> > {
> > Aml *ifctx;
> > Aml *field;
> > @@ -367,13 +368,19 @@ 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, base_addr, base_addr,
> 1,
> > ACPI_CPU_HOTPLUG_REG_LEN));
> > + } else {
> > + aml_append(crs, aml_memory32_fixed(base_addr,
> > + 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),
> > + aml_operation_region("PRST", rs, aml_int(base_addr),
> > ACPI_CPU_HOTPLUG_REG_LEN));
> >
> > field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
> > @@ -699,9 +706,11 @@ void build_cpus_aml(Aml *table, MachineState
> *machine, CPUHotplugFeatures opts,
> > aml_append(sb_scope, cpus_dev);
> > aml_append(table, sb_scope);
> >
> > - method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> > - aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
> > - aml_append(table, method);
> > + if (event_handler_method) {
> > + method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> > + aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
> > + aml_append(table, method);
> > + }
> >
> > g_free(cphp_res_path);
> > }
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 4d2d40bab5..611d3d044d 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1550,7 +1550,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..b87ebfdf4b 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 base_addr,
> > 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);
> >