From: Igor Mammedov <imammedo@redhat.com>
To: Salil Mehta <salil.mehta@huawei.com>, <mst@redhat.com>
Cc: <qemu-devel@nongnu.org>, <qemu-arm@nongnu.org>, <maz@kernel.org>,
<jean-philippe@linaro.org>, <jonathan.cameron@huawei.com>,
<lpieralisi@kernel.org>, <peter.maydell@linaro.org>,
<richard.henderson@linaro.org>, <andrew.jones@linux.dev>,
<david@redhat.com>, <philmd@linaro.org>, <eric.auger@redhat.com>,
<oliver.upton@linux.dev>, <pbonzini@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>,
<zhukeqian1@huawei.com>, <wangxiongfeng2@huawei.com>,
<wangyanan55@huawei.com>, <jiakernel2@gmail.com>,
<maobibo@loongson.cn>, <lixianglai@loongson.cn>,
<linuxarm@huawei.com>
Subject: Re: [PATCH V6 5/9] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
Date: Fri, 27 Oct 2023 15:46:48 +0200 [thread overview]
Message-ID: <20231027154648.2ce47292@imammedo.users.ipa.redhat.com> (raw)
In-Reply-To: <20231013105129.25648-6-salil.mehta@huawei.com>
On Fri, 13 Oct 2023 11:51:25 +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
^^^^
being placed in PCI0 context is no the reason why resource was described as IO.
being IO is probably historical thing (as legacy hp was implemented as IO)
_CRS could have been at _SB level as motherboard resource but, in that case
we would need to carve out hole in PCI's _CRS explicitly to exclude it.
Hence it was placed in PCI0 context as a hack that helps to avoid us that.
Perhaps it also applies to other targets.
> 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. Update build CPUs AML function to accept both IO/MEMORY region
> spaces and accordingly update the _CRS object.
Also x86 should be able to switch to and work with MMIO region
(I think QEMU wise IO and MMIO are the same)
and we can just use MMIO, likely without any compat machinery.
aka. existing/running/migrated x86 guests will use IO instructions to access region
(since CRS they have read, says it is IO), while new VMs will access region
as MMIO.
I might be wrong though,
Michael?
> On x86, Legacy CPU Hotplug uses Generic ACPI GPE Block Bit 2 (GPE.2) event
> handler to notify OSPM about any CPU hot(un)plug events. Latest CPU Hotplug is
> based on ACPI Generic Event Device framework and uses ACPI GED device for the
> same. Not all architectures support Legacy CPU Hotplug. Hence, make AML for
> GPE.2 event handler conditional.
x86 has support for Legacy and Modern CPU hotplug (the later is enabled at runtime).
And both use GPE for event delivery, so above statement is not entirely
correct/confusing.
>
> 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>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Tested-by: Xianglai Li <lixianglai@loongson.cn>
> ---
> hw/acpi/cpu.c | 23 ++++++++++++++++-------
> hw/i386/acpi-build.c | 3 ++-
> include/hw/acpi/cpu.h | 5 +++--
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 4b24a25003..596b6d9d81 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -339,9 +339,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
> #define CPU_FW_EJECT_EVENT "CEJF"
>
> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> - build_madt_cpu_fn build_madt_cpu, hwaddr io_base,
> + build_madt_cpu_fn build_madt_cpu, hwaddr base_addr,
> const char *res_root,
> - const char *event_handler_method)
> + const char *event_handler_method,
> + AmlRegionSpace rs)
> {
> Aml *ifctx;
> Aml *field;
> @@ -366,13 +367,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,
> @@ -696,9 +703,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 3f2b27cf75..f9f31f9db5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1550,7 +1550,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
> };
> build_cpus_aml(dsdt, machine, opts, pc_madt_cpu_entry,
> - pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02");
> + pm->cpu_hp_io_base, "\\_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 bc901660fb..b521a4e0de 100644
> --- a/include/hw/acpi/cpu.h
> +++ b/include/hw/acpi/cpu.h
> @@ -60,9 +60,10 @@ typedef void (*build_madt_cpu_fn)(int uid, const CPUArchIdList *apic_ids,
> GArray *entry, bool force_enabled);
>
> void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> - build_madt_cpu_fn build_madt_cpu, hwaddr io_base,
> + build_madt_cpu_fn build_madt_cpu, 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);
>
next prev parent reply other threads:[~2023-10-27 13:47 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-13 10:51 [PATCH V6 0/9] Add architecture agnostic code to support vCPU Hotplug Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-13 10:51 ` [PATCH V6 1/9] accel/kvm: Extract common KVM vCPU {creation, parking} code Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-27 12:56 ` [PATCH V6 1/9] accel/kvm: Extract common KVM vCPU {creation,parking} code Igor Mammedov
2023-11-06 14:37 ` Salil Mehta via
2023-10-13 10:51 ` [PATCH V6 2/9] hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-13 10:51 ` [PATCH V6 3/9] hw/acpi: Add ACPI CPU hotplug init stub Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-27 13:05 ` Igor Mammedov
2023-11-06 14:40 ` Salil Mehta via
2023-10-13 10:51 ` [PATCH V6 4/9] hw/acpi: Init GED framework with CPU hotplug events Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-19 10:16 ` Alex Bennée
2023-10-19 10:27 ` Salil Mehta via
2023-10-19 10:27 ` Salil Mehta
2023-10-27 14:08 ` Igor Mammedov
2023-11-13 17:36 ` Salil Mehta via
2023-10-13 10:51 ` [PATCH V6 5/9] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-27 13:46 ` Igor Mammedov [this message]
2023-11-13 17:45 ` Salil Mehta via
2023-10-13 10:51 ` [PATCH V6 6/9] hw/acpi: Update GED _EVT method AML with CPU scan Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-13 10:51 ` [PATCH V6 7/9] hw/acpi: Update ACPI GED framework to support vCPU Hotplug Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-27 13:18 ` Igor Mammedov
2023-11-08 10:58 ` Salil Mehta via
2023-10-13 10:51 ` [PATCH V6 8/9] physmem: Add helper function to destroy CPU AddressSpace Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-13 10:51 ` [PATCH V6 9/9] gdbstub: Add helper function to unregister GDB register space Salil Mehta via
2023-10-13 10:51 ` Salil Mehta
2023-10-13 16:33 ` [PATCH V6 0/9] Add architecture agnostic code to support vCPU Hotplug Miguel Luis
2023-10-16 9:52 ` Salil Mehta via
2023-10-16 9:52 ` Salil Mehta
2023-10-16 10:01 ` Miguel Luis
2023-10-16 10:09 ` Salil Mehta via
2023-10-16 10:09 ` Salil Mehta
2023-10-19 9:50 ` Miguel Luis
2023-10-19 9:04 ` Shaoqin Huang
2023-10-19 9:34 ` Salil Mehta via
2023-10-19 9:34 ` Salil Mehta
2023-10-20 3:30 ` Shaoqin Huang
2023-10-26 12:41 ` Igor Mammedov
2023-10-27 10:23 ` Salil Mehta via
2023-10-27 10:23 ` Salil Mehta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231027154648.2ce47292@imammedo.users.ipa.redhat.com \
--to=imammedo@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=andrew.jones@linux.dev \
--cc=darren@os.amperecomputing.com \
--cc=david@redhat.com \
--cc=eric.auger@redhat.com \
--cc=gshan@redhat.com \
--cc=ilkka@os.amperecomputing.com \
--cc=jean-philippe@linaro.org \
--cc=jiakernel2@gmail.com \
--cc=jonathan.cameron@huawei.com \
--cc=karl.heubaum@oracle.com \
--cc=linux@armlinux.org.uk \
--cc=linuxarm@huawei.com \
--cc=lixianglai@loongson.cn \
--cc=lpieralisi@kernel.org \
--cc=maobibo@loongson.cn \
--cc=maz@kernel.org \
--cc=miguel.luis@oracle.com \
--cc=mst@redhat.com \
--cc=oliver.upton@linux.dev \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rafael@kernel.org \
--cc=richard.henderson@linaro.org \
--cc=salil.mehta@huawei.com \
--cc=salil.mehta@opnsrc.net \
--cc=vishnu@os.amperecomputing.com \
--cc=wangxiongfeng2@huawei.com \
--cc=wangyanan55@huawei.com \
--cc=will@kernel.org \
--cc=zhukeqian1@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).