QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Salil Mehta via <qemu-devel@nongnu.org>
To: Igor Mammedov <imammedo@redhat.com>, "mst@redhat.com" <mst@redhat.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
	"maz@kernel.org" <maz@kernel.org>,
	"jean-philippe@linaro.org" <jean-philippe@linaro.org>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	"lpieralisi@kernel.org" <lpieralisi@kernel.org>,
	"peter.maydell@linaro.org" <peter.maydell@linaro.org>,
	"richard.henderson@linaro.org" <richard.henderson@linaro.org>,
	"andrew.jones@linux.dev" <andrew.jones@linux.dev>,
	"david@redhat.com" <david@redhat.com>,
	"philmd@linaro.org" <philmd@linaro.org>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"oliver.upton@linux.dev" <oliver.upton@linux.dev>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"will@kernel.org" <will@kernel.org>,
	"gshan@redhat.com" <gshan@redhat.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"alex.bennee@linaro.org" <alex.bennee@linaro.org>,
	"linux@armlinux.org.uk" <linux@armlinux.org.uk>,
	"darren@os.amperecomputing.com" <darren@os.amperecomputing.com>,
	"ilkka@os.amperecomputing.com" <ilkka@os.amperecomputing.com>,
	"vishnu@os.amperecomputing.com" <vishnu@os.amperecomputing.com>,
	"karl.heubaum@oracle.com" <karl.heubaum@oracle.com>,
	"miguel.luis@oracle.com" <miguel.luis@oracle.com>,
	"salil.mehta@opnsrc.net" <salil.mehta@opnsrc.net>,
	zhukeqian <zhukeqian1@huawei.com>,
	"wangxiongfeng (C)" <wangxiongfeng2@huawei.com>,
	"wangyanan (Y)" <wangyanan55@huawei.com>,
	"jiakernel2@gmail.com" <jiakernel2@gmail.com>,
	"maobibo@loongson.cn" <maobibo@loongson.cn>,
	"lixianglai@loongson.cn" <lixianglai@loongson.cn>,
	Linuxarm <linuxarm@huawei.com>
Subject: RE: [PATCH V6 5/9] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
Date: Mon, 13 Nov 2023 17:45:56 +0000	[thread overview]
Message-ID: <48236d35d7f7468285226a47b0c0a4fd@huawei.com> (raw)
In-Reply-To: <20231027154648.2ce47292@imammedo.users.ipa.redhat.com>

Hi Igor,
Sorry, I missed this as well.

> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Friday, October 27, 2023 2:47 PM
> To: Salil Mehta <salil.mehta@huawei.com>; mst@redhat.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)


I just meant that because it is IO port based therefore existing CPUs AML code
would evaluate to a system resource which describes IO Port address.



> _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.

Sure.

> > 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.


Ok got it. But I guess existing CPUs AML code for ctrl-dev did not have this
change. Or did I miss anything?

 
> 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.

Ok yes. I will rephrase part of it.


Thanks
Salil.

> > 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);
> >



  reply	other threads:[~2023-11-13 17:46 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
2023-11-13 17:45     ` Salil Mehta via [this message]
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=48236d35d7f7468285226a47b0c0a4fd@huawei.com \
    --to=qemu-devel@nongnu.org \
    --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=imammedo@redhat.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=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).