|
| From: | Salil Mehta |
| Subject: | Re: [PATCH V13 5/8] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change |
| Date: | Mon, 8 Jul 2024 05:26:00 +0000 |
| User-agent: | Mozilla Thunderbird |
On 06/07/2024 14:35, Igor Mammedov wrote:
On Fri, 7 Jun 2024 12:56:46 +0100 Salil Mehta <salil.mehta@huawei.com> wrote:CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is IO port based and 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. Update build CPUs AML function to accept both IO/MEMORY region spaces and accordingly update the _CRS object.ack for above change
Thanks
but below part is one too many different changes withing 1 patch. anyways, GPE part probably won't be needed if you follow suggestion made on previous patch.
The change mentioned in the earlier patches might end up creating noise for this patch-set as one will have to touch the Memory Hotplug part as well. I'm willing to do that change but I think it is a noise for this patch-set, really.
On x86, 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 GPE based CPU Hotplug event handler. Hence, make AML for GPE.2 event handler conditional. 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> Tested-by: Miguel Luis <miguel.luis@oracle.com> Reviewed-by: Shaoqin Huang <shahuang@redhat.com> Tested-by: Zhao Liu <zhao1.liu@intel.com> --- 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 af2b6655d2..4c63514b16 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -343,9 +343,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; @@ -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 {else if (rs == yours type)+ aml_append(crs, aml_memory32_fixed(base_addr, + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); + }else assert on not supported input
Sure, no problem. I can incorporate the change. Thanks, Salil.
+ 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,@@ -700,9 +707,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 53f804ac16..b73b136605 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1537,7 +1537,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 e6e1a9ef59..48cded697c 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -61,9 +61,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);
| [Prev in Thread] | Current Thread | [Next in Thread] |