[v3,2/7] Update CPUs AML with cpu-(ctrl)dev change

Commit Message

Xianglai Li Sept. 26, 2023, 9:54 a.m. UTC

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" <[e-mail]>
Co-authored-by: "Salil Mehta" <[e-mail]>
Co-authored-by: "Salil Mehta" <[e-mail]>
Cc: "Bernhard Beschow" <[e-mail]>
Cc: "Salil Mehta" <[e-mail]>
Cc: "Salil Mehta" <[e-mail]>
Cc: Xiaojuan Yang <[e-mail]>
Cc: Song Gao <[e-mail]>
Cc: "Michael S. Tsirkin" <[e-mail]>
Cc: Igor Mammedov <[e-mail]>
Cc: Ani Sinha <[e-mail]>
Cc: Paolo Bonzini <[e-mail]>
Cc: Richard Henderson <[e-mail]>
Cc: Eduardo Habkost <[e-mail]>
Cc: Marcel Apfelbaum <[e-mail]>
Cc: "Philippe Mathieu-Daudé" <[e-mail]>
Cc: Yanan Wang <[e-mail]>
Cc: "Daniel P. Berrangé" <[e-mail]>
Cc: Peter Xu <[e-mail]>
Cc: David Hildenbrand <[e-mail]>
Cc: Bibo Mao <[e-mail]>
Signed-off-by: xianglai li <[e-mail]>
---
 hw/acpi/cpu.c         | 20 +++++++++++++++-----
 hw/i386/acpi-build.c  |  3 ++-
 include/hw/acpi/cpu.h |  5 +++--
 3 files changed, 20 insertions(+), 8 deletions(-)

Comments

Salil Mehta Sept. 26, 2023, 10:49 a.m. UTC | #1

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/[e-mail]/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.

> From: xianglai li <[e-mail]>
> Sent: Tuesday, September 26, 2023 10:54 AM
> To: [e-mail]
> Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> Yang <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo Mao
> <[e-mail]>
> 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" <[e-mail]>
> Co-authored-by: "Salil Mehta" <[e-mail]>
> Co-authored-by: "Salil Mehta" <[e-mail]>
> Cc: "Bernhard Beschow" <[e-mail]>
> Cc: "Salil Mehta" <[e-mail]>
> Cc: "Salil Mehta" <[e-mail]>
> Cc: Xiaojuan Yang <[e-mail]>
> Cc: Song Gao <[e-mail]>
> Cc: "Michael S. Tsirkin" <[e-mail]>
> Cc: Igor Mammedov <[e-mail]>
> Cc: Ani Sinha <[e-mail]>
> Cc: Paolo Bonzini <[e-mail]>
> Cc: Richard Henderson <[e-mail]>
> Cc: Eduardo Habkost <[e-mail]>
> Cc: Marcel Apfelbaum <[e-mail]>
> Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> Cc: Yanan Wang <[e-mail]>
> Cc: "Daniel P. Berrangé" <[e-mail]>
> Cc: Peter Xu <[e-mail]>
> Cc: David Hildenbrand <[e-mail]>
> Cc: Bibo Mao <[e-mail]>
> Signed-off-by: xianglai li <[e-mail]>
> ---
>  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
>

Michael S. Tsirkin Sept. 26, 2023, 11:12 a.m. UTC | #2

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/[e-mail]/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 <[e-mail]>
> > Sent: Tuesday, September 26, 2023 10:54 AM
> > To: [e-mail]
> > Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> > <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > Yang <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > Henderson <[e-mail]>; Eduardo Habkost
> > <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> > Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo Mao
> > <[e-mail]>
> > 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" <[e-mail]>
> > Co-authored-by: "Salil Mehta" <[e-mail]>
> > Co-authored-by: "Salil Mehta" <[e-mail]>
> > Cc: "Bernhard Beschow" <[e-mail]>
> > Cc: "Salil Mehta" <[e-mail]>
> > Cc: "Salil Mehta" <[e-mail]>
> > Cc: Xiaojuan Yang <[e-mail]>
> > Cc: Song Gao <[e-mail]>
> > Cc: "Michael S. Tsirkin" <[e-mail]>
> > Cc: Igor Mammedov <[e-mail]>
> > Cc: Ani Sinha <[e-mail]>
> > Cc: Paolo Bonzini <[e-mail]>
> > Cc: Richard Henderson <[e-mail]>
> > Cc: Eduardo Habkost <[e-mail]>
> > Cc: Marcel Apfelbaum <[e-mail]>
> > Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> > Cc: Yanan Wang <[e-mail]>
> > Cc: "Daniel P. Berrangé" <[e-mail]>
> > Cc: Peter Xu <[e-mail]>
> > Cc: David Hildenbrand <[e-mail]>
> > Cc: Bibo Mao <[e-mail]>
> > Signed-off-by: xianglai li <[e-mail]>
> > ---
> >  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
> > 
>

Salil Mehta Sept. 26, 2023, 11:45 a.m. UTC | #3

> From: Michael S. Tsirkin <[e-mail]>
> Sent: Tuesday, September 26, 2023 12:12 PM
> To: Salil Mehta <[e-mail]>
> Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> Bonzini <[e-mail]>; Richard Henderson
> <[e-mail]>; Eduardo Habkost <[e-mail]>;
> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> 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-
> [e-mail]/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.





> > > From: xianglai li <[e-mail]>
> > > Sent: Tuesday, September 26, 2023 10:54 AM
> > > To: [e-mail]
> > > Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> > > <[e-mail]>; Salil Mehta <[e-mail]>;
> Xiaojuan
> > > Yang <[e-mail]>; Song Gao <[e-mail]>;
> Michael S.
> > > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> Sinha
> > > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > > Henderson <[e-mail]>; Eduardo Habkost
> > > <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> > > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > > <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> Peter
> > > Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo Mao
> > > <[e-mail]>
> > > 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" <[e-mail]>
> > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > Cc: "Bernhard Beschow" <[e-mail]>
> > > Cc: "Salil Mehta" <[e-mail]>
> > > Cc: "Salil Mehta" <[e-mail]>
> > > Cc: Xiaojuan Yang <[e-mail]>
> > > Cc: Song Gao <[e-mail]>
> > > Cc: "Michael S. Tsirkin" <[e-mail]>
> > > Cc: Igor Mammedov <[e-mail]>
> > > Cc: Ani Sinha <[e-mail]>
> > > Cc: Paolo Bonzini <[e-mail]>
> > > Cc: Richard Henderson <[e-mail]>
> > > Cc: Eduardo Habkost <[e-mail]>
> > > Cc: Marcel Apfelbaum <[e-mail]>
> > > Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> > > Cc: Yanan Wang <[e-mail]>
> > > Cc: "Daniel P. Berrangé" <[e-mail]>
> > > Cc: Peter Xu <[e-mail]>
> > > Cc: David Hildenbrand <[e-mail]>
> > > Cc: Bibo Mao <[e-mail]>
> > > Signed-off-by: xianglai li <[e-mail]>
> > > ---
> > >  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
> > >
> >

Michael S. Tsirkin Sept. 26, 2023, 11:54 a.m. UTC | #4

On Tue, Sep 26, 2023 at 11:45:19AM +0000, Salil Mehta wrote:
> 
> > From: Michael S. Tsirkin <[e-mail]>
> > Sent: Tuesday, September 26, 2023 12:12 PM
> > To: Salil Mehta <[e-mail]>
> > Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> > Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > Bonzini <[e-mail]>; Richard Henderson
> > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > 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-
> > [e-mail]/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 <[e-mail]>
> > > > Sent: Tuesday, September 26, 2023 10:54 AM
> > > > To: [e-mail]
> > > > Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> > > > <[e-mail]>; Salil Mehta <[e-mail]>;
> > Xiaojuan
> > > > Yang <[e-mail]>; Song Gao <[e-mail]>;
> > Michael S.
> > > > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> > Sinha
> > > > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > > > Henderson <[e-mail]>; Eduardo Habkost
> > > > <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> > > > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > > > <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> > Peter
> > > > Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo Mao
> > > > <[e-mail]>
> > > > 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" <[e-mail]>
> > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > Cc: "Bernhard Beschow" <[e-mail]>
> > > > Cc: "Salil Mehta" <[e-mail]>
> > > > Cc: "Salil Mehta" <[e-mail]>
> > > > Cc: Xiaojuan Yang <[e-mail]>
> > > > Cc: Song Gao <[e-mail]>
> > > > Cc: "Michael S. Tsirkin" <[e-mail]>
> > > > Cc: Igor Mammedov <[e-mail]>
> > > > Cc: Ani Sinha <[e-mail]>
> > > > Cc: Paolo Bonzini <[e-mail]>
> > > > Cc: Richard Henderson <[e-mail]>
> > > > Cc: Eduardo Habkost <[e-mail]>
> > > > Cc: Marcel Apfelbaum <[e-mail]>
> > > > Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> > > > Cc: Yanan Wang <[e-mail]>
> > > > Cc: "Daniel P. Berrangé" <[e-mail]>
> > > > Cc: Peter Xu <[e-mail]>
> > > > Cc: David Hildenbrand <[e-mail]>
> > > > Cc: Bibo Mao <[e-mail]>
> > > > Signed-off-by: xianglai li <[e-mail]>
> > > > ---
> > > >  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
> > > >
> > >

Salil Mehta Sept. 26, 2023, 12:03 p.m. UTC | #5

> From: Michael S. Tsirkin <[e-mail]>
> Sent: Tuesday, September 26, 2023 12:54 PM
> To: Salil Mehta <[e-mail]>
> Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> Bonzini <[e-mail]>; Richard Henderson
> <[e-mail]>; Eduardo Habkost <[e-mail]>;
> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> 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 <[e-mail]>
> > > Sent: Tuesday, September 26, 2023 12:12 PM
> > > To: Salil Mehta <[e-mail]>
> > > Cc: xianglai li <[e-mail]>; [e-mail];
> Bernhard
> > > Beschow <[e-mail]>; Salil Mehta <[e-mail]>;
> Xiaojuan
> > > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > > Bonzini <[e-mail]>; Richard Henderson
> > > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > > 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-
> > > [e-mail]/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.



> 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 <[e-mail]>
> > > > > Sent: Tuesday, September 26, 2023 10:54 AM
> > > > > To: [e-mail]
> > > > > Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> > > > > <[e-mail]>; Salil Mehta <[e-mail]>;
> > > Xiaojuan
> > > > > Yang <[e-mail]>; Song Gao <[e-mail]>;
> > > Michael S.
> > > > > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> > > Sinha
> > > > > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > > > > Henderson <[e-mail]>; Eduardo Habkost
> > > > > <[e-mail]>; Marcel Apfelbaum
> <[e-mail]>;
> > > > > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > > > > <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> > > Peter
> > > > > Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo
> Mao
> > > > > <[e-mail]>
> > > > > 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" <[e-mail]>
> > > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > > Cc: "Bernhard Beschow" <[e-mail]>
> > > > > Cc: "Salil Mehta" <[e-mail]>
> > > > > Cc: "Salil Mehta" <[e-mail]>
> > > > > Cc: Xiaojuan Yang <[e-mail]>
> > > > > Cc: Song Gao <[e-mail]>
> > > > > Cc: "Michael S. Tsirkin" <[e-mail]>
> > > > > Cc: Igor Mammedov <[e-mail]>
> > > > > Cc: Ani Sinha <[e-mail]>
> > > > > Cc: Paolo Bonzini <[e-mail]>
> > > > > Cc: Richard Henderson <[e-mail]>
> > > > > Cc: Eduardo Habkost <[e-mail]>
> > > > > Cc: Marcel Apfelbaum <[e-mail]>
> > > > > Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> > > > > Cc: Yanan Wang <[e-mail]>
> > > > > Cc: "Daniel P. Berrangé" <[e-mail]>
> > > > > Cc: Peter Xu <[e-mail]>
> > > > > Cc: David Hildenbrand <[e-mail]>
> > > > > Cc: Bibo Mao <[e-mail]>
> > > > > Signed-off-by: xianglai li <[e-mail]>
> > > > > ---
> > > > >  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
> > > > >
> > > >

Michael S. Tsirkin Sept. 26, 2023, 12:07 p.m. UTC | #6

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 <[e-mail]>
	Signed-off-by: Salil Mehta <[e-mail]>
	Co-developed-by: Keqian Zhu <[e-mail]>
	Signed-off-by: Keqian Zhu <[e-mail]>
	Signed-off-by: Salil Mehta <[e-mail]>

You should drop your own Co-developed-by as well as multiple Signed-off-by.

Xianglai Li Sept. 26, 2023, 12:17 p.m. UTC | #7

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/[e-mail]/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 <[e-mail]>
>> Sent: Tuesday, September 26, 2023 10:54 AM
>> To: [e-mail]
>> Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
>> <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
>> Yang <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>> Henderson <[e-mail]>; Eduardo Habkost
>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>> Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo Mao
>> <[e-mail]>
>> 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" <[e-mail]>
>> Co-authored-by: "Salil Mehta" <[e-mail]>
>> Co-authored-by: "Salil Mehta" <[e-mail]>
>> Cc: "Bernhard Beschow" <[e-mail]>
>> Cc: "Salil Mehta" <[e-mail]>
>> Cc: "Salil Mehta" <[e-mail]>
>> Cc: Xiaojuan Yang <[e-mail]>
>> Cc: Song Gao <[e-mail]>
>> Cc: "Michael S. Tsirkin" <[e-mail]>
>> Cc: Igor Mammedov <[e-mail]>
>> Cc: Ani Sinha <[e-mail]>
>> Cc: Paolo Bonzini <[e-mail]>
>> Cc: Richard Henderson <[e-mail]>
>> Cc: Eduardo Habkost <[e-mail]>
>> Cc: Marcel Apfelbaum <[e-mail]>
>> Cc: "Philippe Mathieu-Daudé" <[e-mail]>
>> Cc: Yanan Wang <[e-mail]>
>> Cc: "Daniel P. Berrangé" <[e-mail]>
>> Cc: Peter Xu <[e-mail]>
>> Cc: David Hildenbrand <[e-mail]>
>> Cc: Bibo Mao <[e-mail]>
>> Signed-off-by: xianglai li <[e-mail]>
>> ---
>>   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
>>

Daniel P. Berrangé Sept. 26, 2023, 12:30 p.m. UTC | #8

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 <[e-mail]>
> > > Sent: Tuesday, September 26, 2023 12:12 PM
> > > To: Salil Mehta <[e-mail]>
> > > Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> > > Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > > Bonzini <[e-mail]>; Richard Henderson
> > > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > > 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-
> > > [e-mail]/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

Xianglai Li Sept. 26, 2023, 12:49 p.m. UTC | #9

Hi Salil Mehta via  And Michael S. Tsirkin:
>> From: Michael S. Tsirkin <[e-mail]>
>> Sent: Tuesday, September 26, 2023 12:54 PM
>> To: Salil Mehta <[e-mail]>
>> Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
>> Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
>> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
>> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
>> Bonzini <[e-mail]>; Richard Henderson
>> <[e-mail]>; Eduardo Habkost <[e-mail]>;
>> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
>> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
>> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
>> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
>> 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 <[e-mail]>
>>>> Sent: Tuesday, September 26, 2023 12:12 PM
>>>> To: Salil Mehta <[e-mail]>
>>>> Cc: xianglai li <[e-mail]>; [e-mail];
>> Bernhard
>>>> Beschow <[e-mail]>; Salil Mehta <[e-mail]>;
>> Xiaojuan
>>>> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
>>>> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
>>>> Bonzini <[e-mail]>; Richard Henderson
>>>> <[e-mail]>; Eduardo Habkost <[e-mail]>;
>>>> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
>>>> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
>>>> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
>>>> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
>>>> 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-
>>>> [e-mail]/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 <[e-mail]>
>>>>>> Sent: Tuesday, September 26, 2023 10:54 AM
>>>>>> To: [e-mail]
>>>>>> Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
>>>>>> <[e-mail]>; Salil Mehta <[e-mail]>;
>>>> Xiaojuan
>>>>>> Yang <[e-mail]>; Song Gao <[e-mail]>;
>>>> Michael S.
>>>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
>>>> Sinha
>>>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>>>> <[e-mail]>; Marcel Apfelbaum
>> <[e-mail]>;
>>>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>>>> Peter
>>>>>> Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo
>> Mao
>>>>>> <[e-mail]>
>>>>>> 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" <[e-mail]>
>>>>>> Co-authored-by: "Salil Mehta" <[e-mail]>
>>>>>> Co-authored-by: "Salil Mehta" <[e-mail]>
>>>>>> Cc: "Bernhard Beschow" <[e-mail]>
>>>>>> Cc: "Salil Mehta" <[e-mail]>
>>>>>> Cc: "Salil Mehta" <[e-mail]>
>>>>>> Cc: Xiaojuan Yang <[e-mail]>
>>>>>> Cc: Song Gao <[e-mail]>
>>>>>> Cc: "Michael S. Tsirkin" <[e-mail]>
>>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>>> Cc: Ani Sinha <[e-mail]>
>>>>>> Cc: Paolo Bonzini <[e-mail]>
>>>>>> Cc: Richard Henderson <[e-mail]>
>>>>>> Cc: Eduardo Habkost <[e-mail]>
>>>>>> Cc: Marcel Apfelbaum <[e-mail]>
>>>>>> Cc: "Philippe Mathieu-Daudé" <[e-mail]>
>>>>>> Cc: Yanan Wang <[e-mail]>
>>>>>> Cc: "Daniel P. Berrangé" <[e-mail]>
>>>>>> Cc: Peter Xu <[e-mail]>
>>>>>> Cc: David Hildenbrand <[e-mail]>
>>>>>> Cc: Bibo Mao <[e-mail]>
>>>>>> Signed-off-by: xianglai li <[e-mail]>
>>>>>> ---
>>>>>>   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
>>>>>>

Salil Mehta Sept. 26, 2023, 3:52 p.m. UTC | #10

> From: Michael S. Tsirkin <[e-mail]>
> Sent: Tuesday, September 26, 2023 1:07 PM
> To: Salil Mehta <[e-mail]>
> Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> Bonzini <[e-mail]>; Richard Henderson
> <[e-mail]>; Eduardo Habkost <[e-mail]>;
> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> 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 <[e-mail]>
> 	Signed-off-by: Salil Mehta <[e-mail]>
> 	Co-developed-by: Keqian Zhu <[e-mail]>
> 	Signed-off-by: Keqian Zhu <[e-mail]>
> 	Signed-off-by: Salil Mehta <[e-mail]>
> 
> 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)

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.

Michael S. Tsirkin Sept. 26, 2023, 5:38 p.m. UTC | #11

On Tue, Sep 26, 2023 at 03:52:48PM +0000, Salil Mehta wrote:
> > From: Michael S. Tsirkin <[e-mail]>
> > Sent: Tuesday, September 26, 2023 1:07 PM
> > To: Salil Mehta <[e-mail]>
> > Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> > Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > Bonzini <[e-mail]>; Richard Henderson
> > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > 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 <[e-mail]>
> > 	Signed-off-by: Salil Mehta <[e-mail]>
> > 	Co-developed-by: Keqian Zhu <[e-mail]>
> > 	Signed-off-by: Keqian Zhu <[e-mail]>
> > 	Signed-off-by: Salil Mehta <[e-mail]>
> > 
> > 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 <[e-mail]>
and send to Linus.

Michael S. Tsirkin Sept. 27, 2023, 3:16 p.m. UTC | #12

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 <[e-mail]>
> > > > Sent: Tuesday, September 26, 2023 12:12 PM
> > > > To: Salil Mehta <[e-mail]>
> > > > Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> > > > Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > > > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > > > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > > > Bonzini <[e-mail]>; Richard Henderson
> > > > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > > > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > > > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > > > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > > > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > > > 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-
> > > > [e-mail]/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 :|

Michael S. Tsirkin Sept. 27, 2023, 3:17 p.m. UTC | #13

On Tue, Sep 26, 2023 at 08:49:27PM +0800, lixianglai wrote:
> 
> Hi Salil Mehta via  And Michael S. Tsirkin:
> > > From: Michael S. Tsirkin <[e-mail]>
> > > Sent: Tuesday, September 26, 2023 12:54 PM
> > > To: Salil Mehta <[e-mail]>
> > > Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
> > > Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
> > > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > > Bonzini <[e-mail]>; Richard Henderson
> > > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > > 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 <[e-mail]>
> > > > > Sent: Tuesday, September 26, 2023 12:12 PM
> > > > > To: Salil Mehta <[e-mail]>
> > > > > Cc: xianglai li <[e-mail]>; [e-mail];
> > > Bernhard
> > > > > Beschow <[e-mail]>; Salil Mehta <[e-mail]>;
> > > Xiaojuan
> > > > > Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
> > > > > Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
> > > > > Bonzini <[e-mail]>; Richard Henderson
> > > > > <[e-mail]>; Eduardo Habkost <[e-mail]>;
> > > > > Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
> > > > > <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
> > > > > Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
> > > > > Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
> > > > > 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-
> > > > > [e-mail]/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 <[e-mail]>
> > > > > > > Sent: Tuesday, September 26, 2023 10:54 AM
> > > > > > > To: [e-mail]
> > > > > > > Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
> > > > > > > <[e-mail]>; Salil Mehta <[e-mail]>;
> > > > > Xiaojuan
> > > > > > > Yang <[e-mail]>; Song Gao <[e-mail]>;
> > > > > Michael S.
> > > > > > > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> > > > > Sinha
> > > > > > > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > > > > > > Henderson <[e-mail]>; Eduardo Habkost
> > > > > > > <[e-mail]>; Marcel Apfelbaum
> > > <[e-mail]>;
> > > > > > > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > > > > > > <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> > > > > Peter
> > > > > > > Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo
> > > Mao
> > > > > > > <[e-mail]>
> > > > > > > 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" <[e-mail]>
> > > > > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > > > > Co-authored-by: "Salil Mehta" <[e-mail]>
> > > > > > > Cc: "Bernhard Beschow" <[e-mail]>
> > > > > > > Cc: "Salil Mehta" <[e-mail]>
> > > > > > > Cc: "Salil Mehta" <[e-mail]>
> > > > > > > Cc: Xiaojuan Yang <[e-mail]>
> > > > > > > Cc: Song Gao <[e-mail]>
> > > > > > > Cc: "Michael S. Tsirkin" <[e-mail]>
> > > > > > > Cc: Igor Mammedov <[e-mail]>
> > > > > > > Cc: Ani Sinha <[e-mail]>
> > > > > > > Cc: Paolo Bonzini <[e-mail]>
> > > > > > > Cc: Richard Henderson <[e-mail]>
> > > > > > > Cc: Eduardo Habkost <[e-mail]>
> > > > > > > Cc: Marcel Apfelbaum <[e-mail]>
> > > > > > > Cc: "Philippe Mathieu-Daudé" <[e-mail]>
> > > > > > > Cc: Yanan Wang <[e-mail]>
> > > > > > > Cc: "Daniel P. Berrangé" <[e-mail]>
> > > > > > > Cc: Peter Xu <[e-mail]>
> > > > > > > Cc: David Hildenbrand <[e-mail]>
> > > > > > > Cc: Bibo Mao <[e-mail]>
> > > > > > > Signed-off-by: xianglai li <[e-mail]>
> > > > > > > ---
> > > > > > >   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
> > > > > > >

Xianglai Li Sept. 28, 2023, 1:36 a.m. UTC | #14

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 <[e-mail]>
>>>> Sent: Tuesday, September 26, 2023 12:54 PM
>>>> To: Salil Mehta <[e-mail]>
>>>> Cc: xianglai li <[e-mail]>; [e-mail]; Bernhard
>>>> Beschow <[e-mail]>; Salil Mehta <[e-mail]>; Xiaojuan
>>>> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
>>>> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
>>>> Bonzini <[e-mail]>; Richard Henderson
>>>> <[e-mail]>; Eduardo Habkost <[e-mail]>;
>>>> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
>>>> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
>>>> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
>>>> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
>>>> 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 <[e-mail]>
>>>>>> Sent: Tuesday, September 26, 2023 12:12 PM
>>>>>> To: Salil Mehta <[e-mail]>
>>>>>> Cc: xianglai li <[e-mail]>; [e-mail];
>>>> Bernhard
>>>>>> Beschow <[e-mail]>; Salil Mehta <[e-mail]>;
>>>> Xiaojuan
>>>>>> Yang <[e-mail]>; Song Gao <[e-mail]>; Igor
>>>>>> Mammedov <[e-mail]>; Ani Sinha <[e-mail]>; Paolo
>>>>>> Bonzini <[e-mail]>; Richard Henderson
>>>>>> <[e-mail]>; Eduardo Habkost <[e-mail]>;
>>>>>> Marcel Apfelbaum <[e-mail]>; Philippe Mathieu-Daudé
>>>>>> <[e-mail]>; wangyanan (Y) <[e-mail]>; Daniel P.
>>>>>> Berrangé <[e-mail]>; Peter Xu <[e-mail]>; David
>>>>>> Hildenbrand <[e-mail]>; Bibo Mao <[e-mail]>
>>>>>> 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-
>>>>>> [e-mail]/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 <[e-mail]>
>>>>>>>> Sent: Tuesday, September 26, 2023 10:54 AM
>>>>>>>> To: [e-mail]
>>>>>>>> Cc: Bernhard Beschow <[e-mail]>; Salil Mehta
>>>>>>>> <[e-mail]>; Salil Mehta <[e-mail]>;
>>>>>> Xiaojuan
>>>>>>>> Yang <[e-mail]>; Song Gao <[e-mail]>;
>>>>>> Michael S.
>>>>>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
>>>>>> Sinha
>>>>>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>>>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>>>>>> <[e-mail]>; Marcel Apfelbaum
>>>> <[e-mail]>;
>>>>>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>>>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>>>>>> Peter
>>>>>>>> Xu <[e-mail]>; David Hildenbrand <[e-mail]>; Bibo
>>>> Mao
>>>>>>>> <[e-mail]>
>>>>>>>> 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" <[e-mail]>
>>>>>>>> Co-authored-by: "Salil Mehta" <[e-mail]>
>>>>>>>> Co-authored-by: "Salil Mehta" <[e-mail]>
>>>>>>>> Cc: "Bernhard Beschow" <[e-mail]>
>>>>>>>> Cc: "Salil Mehta" <[e-mail]>
>>>>>>>> Cc: "Salil Mehta" <[e-mail]>
>>>>>>>> Cc: Xiaojuan Yang <[e-mail]>
>>>>>>>> Cc: Song Gao <[e-mail]>
>>>>>>>> Cc: "Michael S. Tsirkin" <[e-mail]>
>>>>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>>>>> Cc: Ani Sinha <[e-mail]>
>>>>>>>> Cc: Paolo Bonzini <[e-mail]>
>>>>>>>> Cc: Richard Henderson <[e-mail]>
>>>>>>>> Cc: Eduardo Habkost <[e-mail]>
>>>>>>>> Cc: Marcel Apfelbaum <[e-mail]>
>>>>>>>> Cc: "Philippe Mathieu-Daudé" <[e-mail]>
>>>>>>>> Cc: Yanan Wang <[e-mail]>
>>>>>>>> Cc: "Daniel P. Berrangé" <[e-mail]>
>>>>>>>> Cc: Peter Xu <[e-mail]>
>>>>>>>> Cc: David Hildenbrand <[e-mail]>
>>>>>>>> Cc: Bibo Mao <[e-mail]>
>>>>>>>> Signed-off-by: xianglai li <[e-mail]>
>>>>>>>> ---
>>>>>>>>    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
>>>>>>>>

Patch

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