| Message ID | c2ab409710f5e0f0346727b47aaabd14537d45b8.1695697701.git.lixianglai@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | *** Adds CPU hot-plug support to Loongarch *** | expand |
On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: > Hi Xianglai, > FYI. RFC V2 is out and you can now drop the arch agnostic patches from > your patch-set. Please check the details in the cover letter which one > you need to pick and rebase from: > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#t > > I am planning to float the architecture agnostic patch-set within this > week which will have same patches and in same order as mentioned in > the cover letter. This will untie the development across different > architectures. > > Many thanks > Salil. However, please get authorship info right. This claims patch has been codeveloped by Bernhard Beschow, xianglai li and yourself. Your patch claims a completely different list of authors with yourself being the only common author. Not nice. > > From: xianglai li <lixianglai@loongson.cn> > > Sent: Tuesday, September 26, 2023 10:54 AM > > To: qemu-devel@nongnu.org > > Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta > > <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; Xiaojuan > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S. > > Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha > > <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard > > Henderson <richard.henderson@linaro.org>; Eduardo Habkost > > <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; > > Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) > > <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter > > Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo Mao > > <maobibo@loongson.cn> > > Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > 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 Loongarch 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 interface of the build cpus > > AML > > function to accept both IO/MEMORY type regions and update the _CRS object > > correspondingly. > > > > Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> > > Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> > > Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> > > Cc: "Bernhard Beschow" <shentey@gmail.com> > > Cc: "Salil Mehta" <salil.mehta@huawei.com> > > Cc: "Salil Mehta" <salil.mehta@opnsrc.net> > > Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> > > Cc: Song Gao <gaosong@loongson.cn> > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Igor Mammedov <imammedo@redhat.com> > > Cc: Ani Sinha <anisinha@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Richard Henderson <richard.henderson@linaro.org> > > Cc: Eduardo Habkost <eduardo@habkost.net> > > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> > > Cc: Yanan Wang <wangyanan55@huawei.com> > > Cc: "Daniel P. Berrangé" <berrange@redhat.com> > > Cc: Peter Xu <peterx@redhat.com> > > Cc: David Hildenbrand <david@redhat.com> > > Cc: Bibo Mao <maobibo@loongson.cn> > > Signed-off-by: xianglai li <lixianglai@loongson.cn> > > --- > > hw/acpi/cpu.c | 20 +++++++++++++++----- > > hw/i386/acpi-build.c | 3 ++- > > include/hw/acpi/cpu.h | 5 +++-- > > 3 files changed, 20 insertions(+), 8 deletions(-) > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > index 5bad983928..0afa04832e 100644 > > --- a/hw/acpi/cpu.c > > +++ b/hw/acpi/cpu.c > > @@ -6,6 +6,7 @@ > > #include "qapi/qapi-events-acpi.h" > > #include "trace.h" > > #include "sysemu/numa.h" > > +#include "hw/acpi/cpu_hotplug.h" > > > > #define OVMF_CPUHP_SMI_CMD 4 > > > > @@ -332,9 +333,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 mmap_io_base, > > const char *res_root, > > - const char *event_handler_method) > > + const char *event_handler_method, > > + AmlRegionSpace rs) > > { > > Aml *ifctx; > > Aml *field; > > @@ -359,14 +361,22 @@ 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, mmap_io_base, > > mmap_io_base, 1, > > ACPI_CPU_HOTPLUG_REG_LEN)); > > + } else { > > + aml_append(crs, aml_memory32_fixed(mmap_io_base, > > + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); > > + } > > + > > aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); > > > > + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); > > /* 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), > > - ACPI_CPU_HOTPLUG_REG_LEN)); > > + aml_operation_region("PRST", rs, > > + aml_int(mmap_io_base), > > + ACPI_CPU_HOTPLUG_REG_LEN)); > > > > field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, > > AML_WRITE_AS_ZEROS); > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 863a939210..7016205d15 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..601f644e57 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 mmap_io_base, > > 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); > > > > -- > > 2.39.1 > > >
On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: > > > From: Michael S. Tsirkin <mst@redhat.com> > > Sent: Tuesday, September 26, 2023 12:12 PM > > To: Salil Mehta <salil.mehta@huawei.com> > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: > > > Hi Xianglai, > > > FYI. RFC V2 is out and you can now drop the arch agnostic patches from > > > your patch-set. Please check the details in the cover letter which one > > > you need to pick and rebase from: > > > > > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1- > > salil.mehta@huawei.com/T/#t > > > > > > I am planning to float the architecture agnostic patch-set within this > > > week which will have same patches and in same order as mentioned in > > > the cover letter. This will untie the development across different > > > architectures. > > > > > > Many thanks > > > Salil. > > > > However, please get authorship info right. This claims patch has been > > codeveloped by Bernhard Beschow, xianglai li and yourself. > > Your patch claims a completely different list of authors > > Yes, because those are the people who have developed the patches. > > > with yourself being the only common author. > > Not nice. > > I have already replied in the other thread. This patch has been > taken from the ARM patch-set sent in the year 2020. > > I am not sure who is the other author and how he has contributed. > > Co-developed-by usually points at main authors. > If you are not sure then find out please. And to help you stop guessing at the rules: Documentation/process/submitting-patches.rst Co-developed-by: states that the patch was co-created by multiple developers; it is used to give attribution to co-authors (in addition to the author attributed by the From: tag) when several people work on a single patch. Since Co-developed-by: denotes authorship, every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author. Standard sign-off procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the chronological history of the patch insofar as possible, regardless of whether the author is attributed via From: or Co-developed-by:. Notably, the last Signed-off-by: must always be that of the developer submitting the patch. > > > > > > > From: xianglai li <lixianglai@loongson.cn> > > > > Sent: Tuesday, September 26, 2023 10:54 AM > > > > To: qemu-devel@nongnu.org > > > > Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta > > > > <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; > > Xiaojuan > > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; > > Michael S. > > > > Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani > > Sinha > > > > <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard > > > > Henderson <richard.henderson@linaro.org>; Eduardo Habkost > > > > <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; > > > > Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) > > > > <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; > > Peter > > > > Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo Mao > > > > <maobibo@loongson.cn> > > > > Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > > > 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 Loongarch 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 interface of the build > > cpus > > > > AML > > > > function to accept both IO/MEMORY type regions and update the _CRS > > object > > > > correspondingly. > > > > > > > > Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> > > > > Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> > > > > Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> > > > > Cc: "Bernhard Beschow" <shentey@gmail.com> > > > > Cc: "Salil Mehta" <salil.mehta@huawei.com> > > > > Cc: "Salil Mehta" <salil.mehta@opnsrc.net> > > > > Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> > > > > Cc: Song Gao <gaosong@loongson.cn> > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > > > Cc: Igor Mammedov <imammedo@redhat.com> > > > > Cc: Ani Sinha <anisinha@redhat.com> > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > > Cc: Richard Henderson <richard.henderson@linaro.org> > > > > Cc: Eduardo Habkost <eduardo@habkost.net> > > > > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > > > Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> > > > > Cc: Yanan Wang <wangyanan55@huawei.com> > > > > Cc: "Daniel P. Berrangé" <berrange@redhat.com> > > > > Cc: Peter Xu <peterx@redhat.com> > > > > Cc: David Hildenbrand <david@redhat.com> > > > > Cc: Bibo Mao <maobibo@loongson.cn> > > > > Signed-off-by: xianglai li <lixianglai@loongson.cn> > > > > --- > > > > hw/acpi/cpu.c | 20 +++++++++++++++----- > > > > hw/i386/acpi-build.c | 3 ++- > > > > include/hw/acpi/cpu.h | 5 +++-- > > > > 3 files changed, 20 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > > > index 5bad983928..0afa04832e 100644 > > > > --- a/hw/acpi/cpu.c > > > > +++ b/hw/acpi/cpu.c > > > > @@ -6,6 +6,7 @@ > > > > #include "qapi/qapi-events-acpi.h" > > > > #include "trace.h" > > > > #include "sysemu/numa.h" > > > > +#include "hw/acpi/cpu_hotplug.h" > > > > > > > > #define OVMF_CPUHP_SMI_CMD 4 > > > > > > > > @@ -332,9 +333,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 > > mmap_io_base, > > > > const char *res_root, > > > > - const char *event_handler_method) > > > > + const char *event_handler_method, > > > > + AmlRegionSpace rs) > > > > { > > > > Aml *ifctx; > > > > Aml *field; > > > > @@ -359,14 +361,22 @@ 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, mmap_io_base, > > > > mmap_io_base, 1, > > > > ACPI_CPU_HOTPLUG_REG_LEN)); > > > > + } else { > > > > + aml_append(crs, aml_memory32_fixed(mmap_io_base, > > > > + ACPI_CPU_HOTPLUG_REG_LEN, > > AML_READ_WRITE)); > > > > + } > > > > + > > > > aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); > > > > > > > > + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); > > > > /* 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), > > > > - ACPI_CPU_HOTPLUG_REG_LEN)); > > > > + aml_operation_region("PRST", rs, > > > > + aml_int(mmap_io_base), > > > > + ACPI_CPU_HOTPLUG_REG_LEN)); > > > > > > > > field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, > > > > AML_WRITE_AS_ZEROS); > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > index 863a939210..7016205d15 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..601f644e57 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 > > mmap_io_base, > > > > 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); > > > > > > > > -- > > > > 2.39.1 > > > > > > >
On Tue, Sep 26, 2023 at 12:03:46PM +0000, Salil Mehta wrote: > Sure, ARM patch-set follows exactly above rules. > Almost. 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> You should drop your own Co-developed-by as well as multiple Signed-off-by.
Hi Salil Mehta via : > Hi Xianglai, > FYI. RFC V2 is out and you can now drop the arch agnostic patches from > your patch-set. Please check the details in the cover letter which one > you need to pick and rebase from: > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#t > > I am planning to float the architecture agnostic patch-set within this > week which will have same patches and in same order as mentioned in > the cover letter. This will untie the development across different > architectures. Very Good! Looking forward to the release of your architecture-independent patches. I will remove the first two patches in the next version. Thanks, Xianglai. > > Many thanks > Salil. > >> From: xianglai li <lixianglai@loongson.cn> >> Sent: Tuesday, September 26, 2023 10:54 AM >> To: qemu-devel@nongnu.org >> Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta >> <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; Xiaojuan >> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S. >> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha >> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard >> Henderson <richard.henderson@linaro.org>; Eduardo Habkost >> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; >> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) >> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter >> Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo Mao >> <maobibo@loongson.cn> >> Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >> >> 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 Loongarch 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 interface of the build cpus >> AML >> function to accept both IO/MEMORY type regions and update the _CRS object >> correspondingly. >> >> Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> >> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> >> Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> >> Cc: "Bernhard Beschow" <shentey@gmail.com> >> Cc: "Salil Mehta" <salil.mehta@huawei.com> >> Cc: "Salil Mehta" <salil.mehta@opnsrc.net> >> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> >> Cc: Song Gao <gaosong@loongson.cn> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Ani Sinha <anisinha@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Richard Henderson <richard.henderson@linaro.org> >> Cc: Eduardo Habkost <eduardo@habkost.net> >> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> >> Cc: Yanan Wang <wangyanan55@huawei.com> >> Cc: "Daniel P. Berrangé" <berrange@redhat.com> >> Cc: Peter Xu <peterx@redhat.com> >> Cc: David Hildenbrand <david@redhat.com> >> Cc: Bibo Mao <maobibo@loongson.cn> >> Signed-off-by: xianglai li <lixianglai@loongson.cn> >> --- >> hw/acpi/cpu.c | 20 +++++++++++++++----- >> hw/i386/acpi-build.c | 3 ++- >> include/hw/acpi/cpu.h | 5 +++-- >> 3 files changed, 20 insertions(+), 8 deletions(-) >> >> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c >> index 5bad983928..0afa04832e 100644 >> --- a/hw/acpi/cpu.c >> +++ b/hw/acpi/cpu.c >> @@ -6,6 +6,7 @@ >> #include "qapi/qapi-events-acpi.h" >> #include "trace.h" >> #include "sysemu/numa.h" >> +#include "hw/acpi/cpu_hotplug.h" >> >> #define OVMF_CPUHP_SMI_CMD 4 >> >> @@ -332,9 +333,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 mmap_io_base, >> const char *res_root, >> - const char *event_handler_method) >> + const char *event_handler_method, >> + AmlRegionSpace rs) >> { >> Aml *ifctx; >> Aml *field; >> @@ -359,14 +361,22 @@ 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, mmap_io_base, >> mmap_io_base, 1, >> ACPI_CPU_HOTPLUG_REG_LEN)); >> + } else { >> + aml_append(crs, aml_memory32_fixed(mmap_io_base, >> + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); >> + } >> + >> aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); >> >> + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); >> /* 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), >> - ACPI_CPU_HOTPLUG_REG_LEN)); >> + aml_operation_region("PRST", rs, >> + aml_int(mmap_io_base), >> + ACPI_CPU_HOTPLUG_REG_LEN)); >> >> field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, >> AML_WRITE_AS_ZEROS); >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 863a939210..7016205d15 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..601f644e57 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 mmap_io_base, >> 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); >> >> -- >> 2.39.1 >>
On Tue, Sep 26, 2023 at 07:54:04AM -0400, Michael S. Tsirkin wrote: > On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > Sent: Tuesday, September 26, 2023 12:12 PM > > > To: Salil Mehta <salil.mehta@huawei.com> > > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard > > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: > > > > Hi Xianglai, > > > > FYI. RFC V2 is out and you can now drop the arch agnostic patches from > > > > your patch-set. Please check the details in the cover letter which one > > > > you need to pick and rebase from: > > > > > > > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1- > > > salil.mehta@huawei.com/T/#t > > > > > > > > I am planning to float the architecture agnostic patch-set within this > > > > week which will have same patches and in same order as mentioned in > > > > the cover letter. This will untie the development across different > > > > architectures. > > > > > > > > Many thanks > > > > Salil. > > > > > > However, please get authorship info right. This claims patch has been > > > codeveloped by Bernhard Beschow, xianglai li and yourself. > > > Your patch claims a completely different list of authors > > > > Yes, because those are the people who have developed the patches. > > > > > with yourself being the only common author. > > > Not nice. > > > > I have already replied in the other thread. This patch has been > > taken from the ARM patch-set sent in the year 2020. > > > > I am not sure who is the other author and how he has contributed. > > > > Co-developed-by usually points at main authors. > > > > > If you are not sure then find out please. > And to help you stop guessing at the rules: > > Documentation/process/submitting-patches.rst > > Co-developed-by: states that the patch was co-created by multiple developers; > it is used to give attribution to co-authors (in addition to the author > attributed by the From: tag) when several people work on a single patch. Since > Co-developed-by: denotes authorship, every Co-developed-by: must be immediately > followed by a Signed-off-by: of the associated co-author. Standard sign-off > procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the > chronological history of the patch insofar as possible, regardless of whether > the author is attributed via From: or Co-developed-by:. Notably, the last > Signed-off-by: must always be that of the developer submitting the patch. Note, that's a linux.git docs requirement you're pointing to, not a QEMU one. I don't think QEMU has historically gone about this level of precise detail/strictness. Nothing in the DCO requires every co-developer to add a S-o-B. The person adding a S-o-B is attesting that they are confident they have the rights to submit this. One way they can attain this confidence is if the people they worked with add their own S-o-B but that's not a hard requirement. *If* some co-developers were working inside the same company and copyright is owned by the company, it is reasonable to only have one S-o-B for the person who finally submits it. That's a judgement call the person submitting can make. With regards, Daniel
Hi Salil Mehta via And Michael S. Tsirkin: >> From: Michael S. Tsirkin <mst@redhat.com> >> Sent: Tuesday, September 26, 2023 12:54 PM >> To: Salil Mehta <salil.mehta@huawei.com> >> Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard >> Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan >> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor >> Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo >> Bonzini <pbonzini@redhat.com>; Richard Henderson >> <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; >> Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé >> <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. >> Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David >> Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> >> Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >> >> On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: >>>> From: Michael S. Tsirkin <mst@redhat.com> >>>> Sent: Tuesday, September 26, 2023 12:12 PM >>>> To: Salil Mehta <salil.mehta@huawei.com> >>>> Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; >> Bernhard >>>> Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; >> Xiaojuan >>>> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor >>>> Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo >>>> Bonzini <pbonzini@redhat.com>; Richard Henderson >>>> <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; >>>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé >>>> <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. >>>> Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David >>>> Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> >>>> Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >>>> >>>> On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: >>>>> Hi Xianglai, >>>>> FYI. RFC V2 is out and you can now drop the arch agnostic patches >> from >>>>> your patch-set. Please check the details in the cover letter which >> one >>>>> you need to pick and rebase from: >>>>> >>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1- >>>> salil.mehta@huawei.com/T/#t >>>>> I am planning to float the architecture agnostic patch-set within >> this >>>>> week which will have same patches and in same order as mentioned in >>>>> the cover letter. This will untie the development across different >>>>> architectures. >>>>> >>>>> Many thanks >>>>> Salil. >>>> However, please get authorship info right. This claims patch has been >>>> codeveloped by Bernhard Beschow, xianglai li and yourself. >>>> Your patch claims a completely different list of authors >>> Yes, because those are the people who have developed the patches. >>> >>>> with yourself being the only common author. >>>> Not nice. >>> I have already replied in the other thread. This patch has been >>> taken from the ARM patch-set sent in the year 2020. >>> >>> I am not sure who is the other author and how he has contributed. >>> >>> Co-developed-by usually points at main authors. >>> >> >> If you are not sure then find out please. > > We really have not collaborated on anything as part of > this entire development of virtual CPU hotplug since the > year 2020? > > I would leave it to Xianglai to answer about the person. > I did not participate in the hot swap of arm virtualized cpu. I just referred to the patch sent by Salil Mehta to the community. Since his patch has not been integrated into qemu's code repository, I referred to Salil Mehta's patch to ensure that my code could run. I added Co-developed-by in order to show respect for the achievements of his labor, which is all my fault. I wrongly used Co-developed-by, and I apologize for that. I will delete the first two patches until the unrelated patches in Salil Mehta's architecture are combined, and then submit my own patch. Thanks, Xianglai. > >> And to help you stop guessing at the rules: >> >> Documentation/process/submitting-patches.rst >> >> Co-developed-by: states that the patch was co-created by multiple >> developers; >> it is used to give attribution to co-authors (in addition to the >> author >> attributed by the From: tag) when several people work on a single >> patch. Since >> Co-developed-by: denotes authorship, every Co-developed-by: must be >> immediately >> followed by a Signed-off-by: of the associated co-author. Standard >> sign-off >> procedure applies, i.e. the ordering of Signed-off-by: tags should >> reflect the >> chronological history of the patch insofar as possible, regardless of >> whether >> the author is attributed via From: or Co-developed-by:. Notably, the >> last >> Signed-off-by: must always be that of the developer submitting the >> patch. > > Sure, ARM patch-set follows exactly above rules. > > > >>>>>> From: xianglai li <lixianglai@loongson.cn> >>>>>> Sent: Tuesday, September 26, 2023 10:54 AM >>>>>> To: qemu-devel@nongnu.org >>>>>> Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta >>>>>> <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; >>>> Xiaojuan >>>>>> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; >>>> Michael S. >>>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani >>>> Sinha >>>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard >>>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost >>>>>> <eduardo@habkost.net>; Marcel Apfelbaum >> <marcel.apfelbaum@gmail.com>; >>>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) >>>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; >>>> Peter >>>>>> Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo >> Mao >>>>>> <maobibo@loongson.cn> >>>>>> Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >>>>>> >>>>>> 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 Loongarch 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 interface of the >> build >>>> cpus >>>>>> AML >>>>>> function to accept both IO/MEMORY type regions and update the _CRS >>>> object >>>>>> correspondingly. >>>>>> >>>>>> Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> >>>>>> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> >>>>>> Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> >>>>>> Cc: "Bernhard Beschow" <shentey@gmail.com> >>>>>> Cc: "Salil Mehta" <salil.mehta@huawei.com> >>>>>> Cc: "Salil Mehta" <salil.mehta@opnsrc.net> >>>>>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> >>>>>> Cc: Song Gao <gaosong@loongson.cn> >>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> >>>>>> Cc: Igor Mammedov <imammedo@redhat.com> >>>>>> Cc: Ani Sinha <anisinha@redhat.com> >>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>>>> Cc: Richard Henderson <richard.henderson@linaro.org> >>>>>> Cc: Eduardo Habkost <eduardo@habkost.net> >>>>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >>>>>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> >>>>>> Cc: Yanan Wang <wangyanan55@huawei.com> >>>>>> Cc: "Daniel P. Berrangé" <berrange@redhat.com> >>>>>> Cc: Peter Xu <peterx@redhat.com> >>>>>> Cc: David Hildenbrand <david@redhat.com> >>>>>> Cc: Bibo Mao <maobibo@loongson.cn> >>>>>> Signed-off-by: xianglai li <lixianglai@loongson.cn> >>>>>> --- >>>>>> hw/acpi/cpu.c | 20 +++++++++++++++----- >>>>>> hw/i386/acpi-build.c | 3 ++- >>>>>> include/hw/acpi/cpu.h | 5 +++-- >>>>>> 3 files changed, 20 insertions(+), 8 deletions(-) >>>>>> >>>>>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c >>>>>> index 5bad983928..0afa04832e 100644 >>>>>> --- a/hw/acpi/cpu.c >>>>>> +++ b/hw/acpi/cpu.c >>>>>> @@ -6,6 +6,7 @@ >>>>>> #include "qapi/qapi-events-acpi.h" >>>>>> #include "trace.h" >>>>>> #include "sysemu/numa.h" >>>>>> +#include "hw/acpi/cpu_hotplug.h" >>>>>> >>>>>> #define OVMF_CPUHP_SMI_CMD 4 >>>>>> >>>>>> @@ -332,9 +333,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 >>>> mmap_io_base, >>>>>> const char *res_root, >>>>>> - const char *event_handler_method) >>>>>> + const char *event_handler_method, >>>>>> + AmlRegionSpace rs) >>>>>> { >>>>>> Aml *ifctx; >>>>>> Aml *field; >>>>>> @@ -359,14 +361,22 @@ 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, mmap_io_base, >>>>>> mmap_io_base, 1, >>>>>> ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>> + } else { >>>>>> + aml_append(crs, aml_memory32_fixed(mmap_io_base, >>>>>> + ACPI_CPU_HOTPLUG_REG_LEN, >>>> AML_READ_WRITE)); >>>>>> + } >>>>>> + >>>>>> aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); >>>>>> >>>>>> + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); >>>>>> /* 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), >>>>>> - ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>> + aml_operation_region("PRST", rs, >>>>>> + aml_int(mmap_io_base), >>>>>> + >> ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>> field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, >>>>>> AML_WRITE_AS_ZEROS); >>>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>>>>> index 863a939210..7016205d15 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..601f644e57 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 >>>> mmap_io_base, >>>>>> 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); >>>>>> >>>>>> -- >>>>>> 2.39.1 >>>>>>
On Tue, Sep 26, 2023 at 03:52:48PM +0000, Salil Mehta wrote: > > From: Michael S. Tsirkin <mst@redhat.com> > > Sent: Tuesday, September 26, 2023 1:07 PM > > To: Salil Mehta <salil.mehta@huawei.com> > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > On Tue, Sep 26, 2023 at 12:03:46PM +0000, Salil Mehta wrote: > > > Sure, ARM patch-set follows exactly above rules. > > > > > > > > > Almost. > > > > 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> > > > > You should drop your own Co-developed-by as well as multiple Signed-off-by. > > > https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by > > Reference: Co-developed-by: Pasting excerpt from above link: > > "1. Standard sign-off procedure applies, i.e. the ordering of > Signed-off-by: tags should reflect the chronological history > of the patch insofar as possible, regardless of whether the > author is attributed via From: or Co-developed-by:. > 2.Notably, the last Signed-off-by: must always be that of the > developer submitting the patch." > > To be able to achieve 1. I have to put Co-developed-by: of > mine at the top as I am the main author of the patch-set > historically and have been continually driving the work. > (It is a common rule even within the kernel to keep first > SOB that of the main author) yes that is fine. > Reference: Signed-off-by: Excerpt from above link: > > " Any further SoBs (Signed-off-by:'s) following the author's > SoB are from people handling and transporting the patch, but > were not involved in its development. SoB chains should > reflect the real route a patch took as it was propagated to > the maintainers and ultimately to Linus, with the first SoB > entry signalling primary authorship of a single author." > > > And since I am the person who is submitting the patches > (which might or not be the same in future) I need to put > my SOB in any case to be able to achieve 2. > > This is to ensure primary author remains the first SOD/CDY. > > > Thanks > Salil. I think you misunderstand what it says - you don't need to repeat signatures many times. you took the patches that were signed off by people A,B,C and sent to me. Thus you do: S.o.b: A S.o.b: B S.o.b: C S.o.b: Salil Mehta and I add: Signed-off-by: Michael S. Tsirkin <mst@redhat.com> and send to Linus.
On Tue, Sep 26, 2023 at 01:30:30PM +0100, Daniel P. Berrangé wrote: > On Tue, Sep 26, 2023 at 07:54:04AM -0400, Michael S. Tsirkin wrote: > > On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: > > > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > > Sent: Tuesday, September 26, 2023 12:12 PM > > > > To: Salil Mehta <salil.mehta@huawei.com> > > > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard > > > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan > > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > > > On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: > > > > > Hi Xianglai, > > > > > FYI. RFC V2 is out and you can now drop the arch agnostic patches from > > > > > your patch-set. Please check the details in the cover letter which one > > > > > you need to pick and rebase from: > > > > > > > > > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1- > > > > salil.mehta@huawei.com/T/#t > > > > > > > > > > I am planning to float the architecture agnostic patch-set within this > > > > > week which will have same patches and in same order as mentioned in > > > > > the cover letter. This will untie the development across different > > > > > architectures. > > > > > > > > > > Many thanks > > > > > Salil. > > > > > > > > However, please get authorship info right. This claims patch has been > > > > codeveloped by Bernhard Beschow, xianglai li and yourself. > > > > Your patch claims a completely different list of authors > > > > > > Yes, because those are the people who have developed the patches. > > > > > > > with yourself being the only common author. > > > > Not nice. > > > > > > I have already replied in the other thread. This patch has been > > > taken from the ARM patch-set sent in the year 2020. > > > > > > I am not sure who is the other author and how he has contributed. > > > > > > Co-developed-by usually points at main authors. > > > > > > > > > If you are not sure then find out please. > > And to help you stop guessing at the rules: > > > > Documentation/process/submitting-patches.rst > > > > Co-developed-by: states that the patch was co-created by multiple developers; > > it is used to give attribution to co-authors (in addition to the author > > attributed by the From: tag) when several people work on a single patch. Since > > Co-developed-by: denotes authorship, every Co-developed-by: must be immediately > > followed by a Signed-off-by: of the associated co-author. Standard sign-off > > procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the > > chronological history of the patch insofar as possible, regardless of whether > > the author is attributed via From: or Co-developed-by:. Notably, the last > > Signed-off-by: must always be that of the developer submitting the patch. > > Note, that's a linux.git docs requirement you're pointing to, > not a QEMU one. > > I don't think QEMU has historically gone about this level > of precise detail/strictness. > > Nothing in the DCO requires every co-developer to add a S-o-B. > The person adding a S-o-B is attesting that they are confident > they have the rights to submit this. One way they can attain > this confidence is if the people they worked with add their own > S-o-B but that's not a hard requirement. *If* some co-developers > were working inside the same company and copyright is owned by > the company, it is reasonable to only have one S-o-B for the > person who finally submits it. That's a judgement call the person > submitting can make. > > With regards, > Daniel We really should write the rules up btw. And, I think it would be a really bad idea to use exactly the same tag as linux with a slightly different set of rules. > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Sep 26, 2023 at 08:49:27PM +0800, lixianglai wrote: > > Hi Salil Mehta via And Michael S. Tsirkin: > > > From: Michael S. Tsirkin <mst@redhat.com> > > > Sent: Tuesday, September 26, 2023 12:54 PM > > > To: Salil Mehta <salil.mehta@huawei.com> > > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard > > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > > > Sent: Tuesday, September 26, 2023 12:12 PM > > > > > To: Salil Mehta <salil.mehta@huawei.com> > > > > > Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; > > > Bernhard > > > > > Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; > > > Xiaojuan > > > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor > > > > > Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo > > > > > Bonzini <pbonzini@redhat.com>; Richard Henderson > > > > > <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; > > > > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé > > > > > <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. > > > > > Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David > > > > > Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> > > > > > Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > > > > > On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: > > > > > > Hi Xianglai, > > > > > > FYI. RFC V2 is out and you can now drop the arch agnostic patches > > > from > > > > > > your patch-set. Please check the details in the cover letter which > > > one > > > > > > you need to pick and rebase from: > > > > > > > > > > > > https://lore.kernel.org/qemu-devel/20230926100436.28284-1- > > > > > salil.mehta@huawei.com/T/#t > > > > > > I am planning to float the architecture agnostic patch-set within > > > this > > > > > > week which will have same patches and in same order as mentioned in > > > > > > the cover letter. This will untie the development across different > > > > > > architectures. > > > > > > > > > > > > Many thanks > > > > > > Salil. > > > > > However, please get authorship info right. This claims patch has been > > > > > codeveloped by Bernhard Beschow, xianglai li and yourself. > > > > > Your patch claims a completely different list of authors > > > > Yes, because those are the people who have developed the patches. > > > > > > > > > with yourself being the only common author. > > > > > Not nice. > > > > I have already replied in the other thread. This patch has been > > > > taken from the ARM patch-set sent in the year 2020. > > > > > > > > I am not sure who is the other author and how he has contributed. > > > > > > > > Co-developed-by usually points at main authors. > > > > > > > > > > If you are not sure then find out please. > > > > We really have not collaborated on anything as part of > > this entire development of virtual CPU hotplug since the > > year 2020? > > > > I would leave it to Xianglai to answer about the person. > > > > I did not participate in the hot swap of arm virtualized cpu. > > I just referred to the patch sent by Salil Mehta to the community. > > Since his patch has not been integrated into qemu's code repository, > > I referred to Salil Mehta's patch to ensure that my code could run. > > I added Co-developed-by in order to show respect for the achievements of his > labor, > > which is all my fault. I wrongly used Co-developed-by, and I apologize for > that. > > I will delete the first two patches until the unrelated patches in Salil > Mehta's architecture are combined, > > and then submit my own patch. > > > Thanks, > > Xianglai. I see. The correct way is to simply keep the original author. I don't know what you used to apply the patch but e.g. git am will do exactly that for you. Add your own S.O.B at the bottom and you are all set. > > > > > > And to help you stop guessing at the rules: > > > > > > Documentation/process/submitting-patches.rst > > > > > > Co-developed-by: states that the patch was co-created by multiple > > > developers; > > > it is used to give attribution to co-authors (in addition to the > > > author > > > attributed by the From: tag) when several people work on a single > > > patch. Since > > > Co-developed-by: denotes authorship, every Co-developed-by: must be > > > immediately > > > followed by a Signed-off-by: of the associated co-author. Standard > > > sign-off > > > procedure applies, i.e. the ordering of Signed-off-by: tags should > > > reflect the > > > chronological history of the patch insofar as possible, regardless of > > > whether > > > the author is attributed via From: or Co-developed-by:. Notably, the > > > last > > > Signed-off-by: must always be that of the developer submitting the > > > patch. > > > > Sure, ARM patch-set follows exactly above rules. > > > > > > > > > > > > > From: xianglai li <lixianglai@loongson.cn> > > > > > > > Sent: Tuesday, September 26, 2023 10:54 AM > > > > > > > To: qemu-devel@nongnu.org > > > > > > > Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta > > > > > > > <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; > > > > > Xiaojuan > > > > > > > Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; > > > > > Michael S. > > > > > > > Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani > > > > > Sinha > > > > > > > <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard > > > > > > > Henderson <richard.henderson@linaro.org>; Eduardo Habkost > > > > > > > <eduardo@habkost.net>; Marcel Apfelbaum > > > <marcel.apfelbaum@gmail.com>; > > > > > > > Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) > > > > > > > <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; > > > > > Peter > > > > > > > Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo > > > Mao > > > > > > > <maobibo@loongson.cn> > > > > > > > Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change > > > > > > > > > > > > > > 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 Loongarch 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 interface of the > > > build > > > > > cpus > > > > > > > AML > > > > > > > function to accept both IO/MEMORY type regions and update the _CRS > > > > > object > > > > > > > correspondingly. > > > > > > > > > > > > > > Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> > > > > > > > Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> > > > > > > > Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> > > > > > > > Cc: "Bernhard Beschow" <shentey@gmail.com> > > > > > > > Cc: "Salil Mehta" <salil.mehta@huawei.com> > > > > > > > Cc: "Salil Mehta" <salil.mehta@opnsrc.net> > > > > > > > Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> > > > > > > > Cc: Song Gao <gaosong@loongson.cn> > > > > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > > > > > > Cc: Igor Mammedov <imammedo@redhat.com> > > > > > > > Cc: Ani Sinha <anisinha@redhat.com> > > > > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > > > > > Cc: Richard Henderson <richard.henderson@linaro.org> > > > > > > > Cc: Eduardo Habkost <eduardo@habkost.net> > > > > > > > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > > > > > > Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> > > > > > > > Cc: Yanan Wang <wangyanan55@huawei.com> > > > > > > > Cc: "Daniel P. Berrangé" <berrange@redhat.com> > > > > > > > Cc: Peter Xu <peterx@redhat.com> > > > > > > > Cc: David Hildenbrand <david@redhat.com> > > > > > > > Cc: Bibo Mao <maobibo@loongson.cn> > > > > > > > Signed-off-by: xianglai li <lixianglai@loongson.cn> > > > > > > > --- > > > > > > > hw/acpi/cpu.c | 20 +++++++++++++++----- > > > > > > > hw/i386/acpi-build.c | 3 ++- > > > > > > > include/hw/acpi/cpu.h | 5 +++-- > > > > > > > 3 files changed, 20 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > > > > > > index 5bad983928..0afa04832e 100644 > > > > > > > --- a/hw/acpi/cpu.c > > > > > > > +++ b/hw/acpi/cpu.c > > > > > > > @@ -6,6 +6,7 @@ > > > > > > > #include "qapi/qapi-events-acpi.h" > > > > > > > #include "trace.h" > > > > > > > #include "sysemu/numa.h" > > > > > > > +#include "hw/acpi/cpu_hotplug.h" > > > > > > > > > > > > > > #define OVMF_CPUHP_SMI_CMD 4 > > > > > > > > > > > > > > @@ -332,9 +333,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 > > > > > mmap_io_base, > > > > > > > const char *res_root, > > > > > > > - const char *event_handler_method) > > > > > > > + const char *event_handler_method, > > > > > > > + AmlRegionSpace rs) > > > > > > > { > > > > > > > Aml *ifctx; > > > > > > > Aml *field; > > > > > > > @@ -359,14 +361,22 @@ 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, mmap_io_base, > > > > > > > mmap_io_base, 1, > > > > > > > ACPI_CPU_HOTPLUG_REG_LEN)); > > > > > > > + } else { > > > > > > > + aml_append(crs, aml_memory32_fixed(mmap_io_base, > > > > > > > + ACPI_CPU_HOTPLUG_REG_LEN, > > > > > AML_READ_WRITE)); > > > > > > > + } > > > > > > > + > > > > > > > aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); > > > > > > > > > > > > > > + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); > > > > > > > /* 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), > > > > > > > - ACPI_CPU_HOTPLUG_REG_LEN)); > > > > > > > + aml_operation_region("PRST", rs, > > > > > > > + aml_int(mmap_io_base), > > > > > > > + > > > ACPI_CPU_HOTPLUG_REG_LEN)); > > > > > > > field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, > > > > > > > AML_WRITE_AS_ZEROS); > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > > > index 863a939210..7016205d15 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..601f644e57 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 > > > > > mmap_io_base, > > > > > > > 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); > > > > > > > > > > > > > > -- > > > > > > > 2.39.1 > > > > > > >
Hi Michael S. Tsirkin: > On Tue, Sep 26, 2023 at 08:49:27PM +0800, lixianglai wrote: >> Hi Salil Mehta via And Michael S. Tsirkin: >>>> From: Michael S. Tsirkin <mst@redhat.com> >>>> Sent: Tuesday, September 26, 2023 12:54 PM >>>> To: Salil Mehta <salil.mehta@huawei.com> >>>> Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Bernhard >>>> Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan >>>> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor >>>> Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo >>>> Bonzini <pbonzini@redhat.com>; Richard Henderson >>>> <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; >>>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé >>>> <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. >>>> Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David >>>> Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> >>>> Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >>>> >>>> On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote: >>>>>> From: Michael S. Tsirkin <mst@redhat.com> >>>>>> Sent: Tuesday, September 26, 2023 12:12 PM >>>>>> To: Salil Mehta <salil.mehta@huawei.com> >>>>>> Cc: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org; >>>> Bernhard >>>>>> Beschow <shentey@gmail.com>; Salil Mehta <salil.mehta@opnsrc.net>; >>>> Xiaojuan >>>>>> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Igor >>>>>> Mammedov <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo >>>>>> Bonzini <pbonzini@redhat.com>; Richard Henderson >>>>>> <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>; >>>>>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé >>>>>> <philmd@linaro.org>; wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. >>>>>> Berrangé <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; David >>>>>> Hildenbrand <david@redhat.com>; Bibo Mao <maobibo@loongson.cn> >>>>>> Subject: Re: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >>>>>> >>>>>> On Tue, Sep 26, 2023 at 10:49:08AM +0000, Salil Mehta wrote: >>>>>>> Hi Xianglai, >>>>>>> FYI. RFC V2 is out and you can now drop the arch agnostic patches >>>> from >>>>>>> your patch-set. Please check the details in the cover letter which >>>> one >>>>>>> you need to pick and rebase from: >>>>>>> >>>>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1- >>>>>> salil.mehta@huawei.com/T/#t >>>>>>> I am planning to float the architecture agnostic patch-set within >>>> this >>>>>>> week which will have same patches and in same order as mentioned in >>>>>>> the cover letter. This will untie the development across different >>>>>>> architectures. >>>>>>> >>>>>>> Many thanks >>>>>>> Salil. >>>>>> However, please get authorship info right. This claims patch has been >>>>>> codeveloped by Bernhard Beschow, xianglai li and yourself. >>>>>> Your patch claims a completely different list of authors >>>>> Yes, because those are the people who have developed the patches. >>>>> >>>>>> with yourself being the only common author. >>>>>> Not nice. >>>>> I have already replied in the other thread. This patch has been >>>>> taken from the ARM patch-set sent in the year 2020. >>>>> >>>>> I am not sure who is the other author and how he has contributed. >>>>> >>>>> Co-developed-by usually points at main authors. >>>>> >>>> If you are not sure then find out please. >>> We really have not collaborated on anything as part of >>> this entire development of virtual CPU hotplug since the >>> year 2020? >>> >>> I would leave it to Xianglai to answer about the person. >>> >> I did not participate in the hot swap of arm virtualized cpu. >> >> I just referred to the patch sent by Salil Mehta to the community. >> >> Since his patch has not been integrated into qemu's code repository, >> >> I referred to Salil Mehta's patch to ensure that my code could run. >> >> I added Co-developed-by in order to show respect for the achievements of his >> labor, >> >> which is all my fault. I wrongly used Co-developed-by, and I apologize for >> that. >> >> I will delete the first two patches until the unrelated patches in Salil >> Mehta's architecture are combined, >> >> and then submit my own patch. >> >> >> Thanks, >> >> Xianglai. > I see. The correct way is to simply keep the original author. > I don't know what you used to apply the patch but e.g. git am > will do exactly that for you. > Add your own S.O.B at the bottom and you are all set. Oh, I see. Thank you very much, Xianglai. > >>>> And to help you stop guessing at the rules: >>>> >>>> Documentation/process/submitting-patches.rst >>>> >>>> Co-developed-by: states that the patch was co-created by multiple >>>> developers; >>>> it is used to give attribution to co-authors (in addition to the >>>> author >>>> attributed by the From: tag) when several people work on a single >>>> patch. Since >>>> Co-developed-by: denotes authorship, every Co-developed-by: must be >>>> immediately >>>> followed by a Signed-off-by: of the associated co-author. Standard >>>> sign-off >>>> procedure applies, i.e. the ordering of Signed-off-by: tags should >>>> reflect the >>>> chronological history of the patch insofar as possible, regardless of >>>> whether >>>> the author is attributed via From: or Co-developed-by:. Notably, the >>>> last >>>> Signed-off-by: must always be that of the developer submitting the >>>> patch. >>> Sure, ARM patch-set follows exactly above rules. >>> >>> >>> >>>>>>>> From: xianglai li <lixianglai@loongson.cn> >>>>>>>> Sent: Tuesday, September 26, 2023 10:54 AM >>>>>>>> To: qemu-devel@nongnu.org >>>>>>>> Cc: Bernhard Beschow <shentey@gmail.com>; Salil Mehta >>>>>>>> <salil.mehta@opnsrc.net>; Salil Mehta <salil.mehta@huawei.com>; >>>>>> Xiaojuan >>>>>>>> Yang <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; >>>>>> Michael S. >>>>>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani >>>>>> Sinha >>>>>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard >>>>>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost >>>>>>>> <eduardo@habkost.net>; Marcel Apfelbaum >>>> <marcel.apfelbaum@gmail.com>; >>>>>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y) >>>>>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; >>>>>> Peter >>>>>>>> Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo >>>> Mao >>>>>>>> <maobibo@loongson.cn> >>>>>>>> Subject: [PATCH v3 2/7] Update CPUs AML with cpu-(ctrl)dev change >>>>>>>> >>>>>>>> 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 Loongarch 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 interface of the >>>> build >>>>>> cpus >>>>>>>> AML >>>>>>>> function to accept both IO/MEMORY type regions and update the _CRS >>>>>> object >>>>>>>> correspondingly. >>>>>>>> >>>>>>>> Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> >>>>>>>> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> >>>>>>>> Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> >>>>>>>> Cc: "Bernhard Beschow" <shentey@gmail.com> >>>>>>>> Cc: "Salil Mehta" <salil.mehta@huawei.com> >>>>>>>> Cc: "Salil Mehta" <salil.mehta@opnsrc.net> >>>>>>>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> >>>>>>>> Cc: Song Gao <gaosong@loongson.cn> >>>>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> >>>>>>>> Cc: Igor Mammedov <imammedo@redhat.com> >>>>>>>> Cc: Ani Sinha <anisinha@redhat.com> >>>>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>>>>>> Cc: Richard Henderson <richard.henderson@linaro.org> >>>>>>>> Cc: Eduardo Habkost <eduardo@habkost.net> >>>>>>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >>>>>>>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> >>>>>>>> Cc: Yanan Wang <wangyanan55@huawei.com> >>>>>>>> Cc: "Daniel P. Berrangé" <berrange@redhat.com> >>>>>>>> Cc: Peter Xu <peterx@redhat.com> >>>>>>>> Cc: David Hildenbrand <david@redhat.com> >>>>>>>> Cc: Bibo Mao <maobibo@loongson.cn> >>>>>>>> Signed-off-by: xianglai li <lixianglai@loongson.cn> >>>>>>>> --- >>>>>>>> hw/acpi/cpu.c | 20 +++++++++++++++----- >>>>>>>> hw/i386/acpi-build.c | 3 ++- >>>>>>>> include/hw/acpi/cpu.h | 5 +++-- >>>>>>>> 3 files changed, 20 insertions(+), 8 deletions(-) >>>>>>>> >>>>>>>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c >>>>>>>> index 5bad983928..0afa04832e 100644 >>>>>>>> --- a/hw/acpi/cpu.c >>>>>>>> +++ b/hw/acpi/cpu.c >>>>>>>> @@ -6,6 +6,7 @@ >>>>>>>> #include "qapi/qapi-events-acpi.h" >>>>>>>> #include "trace.h" >>>>>>>> #include "sysemu/numa.h" >>>>>>>> +#include "hw/acpi/cpu_hotplug.h" >>>>>>>> >>>>>>>> #define OVMF_CPUHP_SMI_CMD 4 >>>>>>>> >>>>>>>> @@ -332,9 +333,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 >>>>>> mmap_io_base, >>>>>>>> const char *res_root, >>>>>>>> - const char *event_handler_method) >>>>>>>> + const char *event_handler_method, >>>>>>>> + AmlRegionSpace rs) >>>>>>>> { >>>>>>>> Aml *ifctx; >>>>>>>> Aml *field; >>>>>>>> @@ -359,14 +361,22 @@ 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, mmap_io_base, >>>>>>>> mmap_io_base, 1, >>>>>>>> ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>>>> + } else { >>>>>>>> + aml_append(crs, aml_memory32_fixed(mmap_io_base, >>>>>>>> + ACPI_CPU_HOTPLUG_REG_LEN, >>>>>> AML_READ_WRITE)); >>>>>>>> + } >>>>>>>> + >>>>>>>> aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); >>>>>>>> >>>>>>>> + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); >>>>>>>> /* 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), >>>>>>>> - ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>>>> + aml_operation_region("PRST", rs, >>>>>>>> + aml_int(mmap_io_base), >>>>>>>> + >>>> ACPI_CPU_HOTPLUG_REG_LEN)); >>>>>>>> field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, >>>>>>>> AML_WRITE_AS_ZEROS); >>>>>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>>>>>>> index 863a939210..7016205d15 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..601f644e57 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 >>>>>> mmap_io_base, >>>>>>>> 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); >>>>>>>> >>>>>>>> -- >>>>>>>> 2.39.1 >>>>>>>>
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 5bad983928..0afa04832e 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -6,6 +6,7 @@ #include "qapi/qapi-events-acpi.h" #include "trace.h" #include "sysemu/numa.h" +#include "hw/acpi/cpu_hotplug.h" #define OVMF_CPUHP_SMI_CMD 4 @@ -332,9 +333,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 mmap_io_base, const char *res_root, - const char *event_handler_method) + const char *event_handler_method, + AmlRegionSpace rs) { Aml *ifctx; Aml *field; @@ -359,14 +361,22 @@ 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, mmap_io_base, mmap_io_base, 1, ACPI_CPU_HOTPLUG_REG_LEN)); + } else { + aml_append(crs, aml_memory32_fixed(mmap_io_base, + ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE)); + } + aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs)); + g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY); /* 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), - ACPI_CPU_HOTPLUG_REG_LEN)); + aml_operation_region("PRST", rs, + aml_int(mmap_io_base), + ACPI_CPU_HOTPLUG_REG_LEN)); field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 863a939210..7016205d15 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..601f644e57 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 mmap_io_base, 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 Loongarch 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 interface of the build cpus AML function to accept both IO/MEMORY type regions and update the _CRS object correspondingly. Co-authored-by: "Bernhard Beschow" <shentey@gmail.com> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net> Co-authored-by: "Salil Mehta" <salil.mehta@huawei.com> Cc: "Bernhard Beschow" <shentey@gmail.com> Cc: "Salil Mehta" <salil.mehta@huawei.com> Cc: "Salil Mehta" <salil.mehta@opnsrc.net> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn> Cc: Song Gao <gaosong@loongson.cn> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Ani Sinha <anisinha@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Eduardo Habkost <eduardo@habkost.net> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> Cc: Yanan Wang <wangyanan55@huawei.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Peter Xu <peterx@redhat.com> Cc: David Hildenbrand <david@redhat.com> Cc: Bibo Mao <maobibo@loongson.cn> Signed-off-by: xianglai li <lixianglai@loongson.cn> --- hw/acpi/cpu.c | 20 +++++++++++++++----- hw/i386/acpi-build.c | 3 ++- include/hw/acpi/cpu.h | 5 +++-- 3 files changed, 20 insertions(+), 8 deletions(-)