| Message ID | 20230926100436.28284-17-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support of Virtual CPU Hotplug for ARMv8 Arch | expand |
On 9/26/23 20:04, Salil Mehta 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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ . The existing AML code assumes _CRS object > would evaluate to a system resource which describes IO Port address. But on ARM ^^^^^^^^^^^^^^^^^^^ is evaluated to a > arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence > _CRS object should evaluate to system resource which describes memory-mapped ^^^^^^ should be evaluated > 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. > > NOTE: Beside above CPU scan shall be triggered when OSPM evaluates _EVT method > part of the GED framework which is covered in subsequent patch. > > Co-developed-by: Salil Mehta <salil.mehta@huawei.com> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > 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(-) > I guess the commit log can be simplified to: The CPU hotplug register block is declared as a IO region on x86, or a memory region on arm64 in build_cpus_aml(), as part of the generic container device (\\_SB.PCI0 or \\_SB.PRES). Adapt build_cpus_aml() so that IO region and memory region can be handled in the mean while. Reviewed-by: Gavin Shan <gshan@redhat.com> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index d5ba37b209..232720992d 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -341,9 +341,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; > @@ -370,13 +371,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, > @@ -702,9 +709,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 bb12b0ad43..560f108d38 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); > Thanks, Gavin
> From: Gavin Shan <gshan@redhat.com> > Sent: Thursday, September 28, 2023 2:26 AM > To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org; qemu- > arm@nongnu.org > Cc: maz@kernel.org; jean-philippe@linaro.org; Jonathan Cameron > <jonathan.cameron@huawei.com>; 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; will@kernel.org; ardb@kernel.org; > oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com; > rafael@kernel.org; borntraeger@linux.ibm.com; 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 > Subject: Re: [PATCH RFC V2 16/37] hw/acpi: Update CPUs AML with cpu- > (ctrl)dev change > > On 9/26/23 20:04, Salil Mehta 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 > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > . The existing AML code assumes _CRS object > > would evaluate to a system resource which describes IO Port address. But > on ARM > ^^^^^^^^^^^^^^^^^^^ > is evaluated to a > > > arch CPUs control device(\\_SB.PRES) register interface is memory-mapped > hence > > _CRS object should evaluate to system resource which describes memory-mapped > ^^^^^^ > should be evaluated > > 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. > > > > NOTE: Beside above CPU scan shall be triggered when OSPM evaluates _EVT method > > part of the GED framework which is covered in subsequent patch. > > > > Co-developed-by: Salil Mehta <salil.mehta@huawei.com> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > 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(-) > > > > I guess the commit log can be simplified to: > > The CPU hotplug register block is declared as a IO region on x86, or a memory > region on arm64 in build_cpus_aml(), as part of the generic container device > (\\_SB.PCI0 or \\_SB.PRES). > > Adapt build_cpus_aml() so that IO region and memory region can be handled > in the mean while. > > Reviewed-by: Gavin Shan <gshan@redhat.com> Has been reviewed already part of architecture agnostic patch-set, https://lore.kernel.org/qemu-devel/4764CF47-47CA-4685-805C-BBE6310BE164@oracle.com/T/#md615c6d3464e7178214785501e7035bf977886f2 Thanks Salil.
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index d5ba37b209..232720992d 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -341,9 +341,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; @@ -370,13 +371,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, @@ -702,9 +709,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 bb12b0ad43..560f108d38 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);
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. NOTE: Beside above CPU scan shall be triggered when OSPM evaluates _EVT method part of the GED framework which is covered in subsequent patch. Co-developed-by: Salil Mehta <salil.mehta@huawei.com> Signed-off-by: Salil Mehta <salil.mehta@huawei.com> 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(-)