[PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property

Luc Michel posted 47 patches 2 months, 3 weeks ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
Posted by Luc Michel 2 months, 3 weeks ago
From: Francisco Iglesias <francisco.iglesias@xilinx.com>

Introduce a 'first-cpu-index' property for specifying the first QEMU CPU
connected to the GICv3. This makes it possible to have multiple instances
of the GICv3 connected to different CPU clusters.

For KVM, mark this property has unsupported. It probably does not make
much sense as it is intented to be used to model non-SMP systems.

Signed-off-by: Luc Michel <luc.michel@amd.com>
Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>
---
 include/hw/intc/arm_gicv3_common.h | 1 +
 hw/intc/arm_gicv3_common.c         | 3 ++-
 hw/intc/arm_gicv3_cpuif.c          | 2 +-
 hw/intc/arm_gicv3_kvm.c            | 6 ++++++
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index c18503869f9..3c2ed30de71 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -226,10 +226,11 @@ struct GICv3State {
     MemoryRegion iomem_dist; /* Distributor */
     GICv3RedistRegion *redist_regions; /* Redistributor Regions */
     uint32_t *redist_region_count; /* redistributor count within each region */
     uint32_t nb_redist_regions; /* number of redist regions */
 
+    uint32_t first_cpu_idx;
     uint32_t num_cpu;
     uint32_t num_irq;
     uint32_t revision;
     uint32_t maint_irq;
     bool lpi_enable;
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index e438d8c042d..2d0df6da86c 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -434,11 +434,11 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
     }
 
     s->cpu = g_new0(GICv3CPUState, s->num_cpu);
 
     for (i = 0; i < s->num_cpu; i++) {
-        CPUState *cpu = qemu_get_cpu(i);
+        CPUState *cpu = qemu_get_cpu(s->first_cpu_idx + i);
         uint64_t cpu_affid;
 
         s->cpu[i].cpu = cpu;
         s->cpu[i].gic = s;
         /* Store GICv3CPUState in CPUARMState gicv3state pointer */
@@ -620,10 +620,11 @@ static const Property arm_gicv3_common_properties[] = {
     DEFINE_PROP_BOOL("force-8-bit-prio", GICv3State, force_8bit_prio, 0),
     DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
                       redist_region_count, qdev_prop_uint32, uint32_t),
     DEFINE_PROP_LINK("sysmem", GICv3State, dma, TYPE_MEMORY_REGION,
                      MemoryRegion *),
+    DEFINE_PROP_UINT32("first-cpu-index", GICv3State, first_cpu_idx, 0),
 };
 
 static void arm_gicv3_common_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 4b4cf091570..1af7690b958 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -3022,11 +3022,11 @@ void gicv3_init_cpuif(GICv3State *s)
      * registers with the CPU
      */
     int i;
 
     for (i = 0; i < s->num_cpu; i++) {
-        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
+        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(s->first_cpu_idx + i));
         GICv3CPUState *cs = &s->cpu[i];
 
         /*
          * If the CPU doesn't define a GICv3 configuration, probably because
          * in real hardware it doesn't have one, then we use default values
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 6166283cd1a..e6a09c9b7d0 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -807,10 +807,16 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
     if (s->nmi_support) {
         error_setg(errp, "NMI is not supported with the in-kernel GIC");
         return;
     }
 
+    if (s->first_cpu_idx != 0) {
+        error_setg(errp, "Non-zero first-cpu-idx is unsupported with the "
+                   "in-kernel GIC");
+        return;
+    }
+
     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
 
     for (i = 0; i < s->num_cpu; i++) {
         ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
 
-- 
2.50.1
Re: [PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
Posted by Peter Maydell 2 months ago
On Fri, 22 Aug 2025 at 16:17, Luc Michel <luc.michel@amd.com> wrote:
>
> From: Francisco Iglesias <francisco.iglesias@xilinx.com>
>
> Introduce a 'first-cpu-index' property for specifying the first QEMU CPU
> connected to the GICv3. This makes it possible to have multiple instances
> of the GICv3 connected to different CPU clusters.
>
> For KVM, mark this property has unsupported. It probably does not make
> much sense as it is intented to be used to model non-SMP systems.
>
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
RE: [PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
Posted by Boddu, Sai Pavan 2 months ago
[AMD Official Use Only - AMD Internal Distribution Only]

Hi Luc,

Looks good for me.

>-----Original Message-----
>From: Luc Michel <luc.michel@amd.com>
>Sent: Friday, August 22, 2025 8:46 PM
>To: qemu-devel@nongnu.org; qemu-arm@nongnu.org
>Cc: Francisco Iglesias <francisco.iglesias@xilinx.com>; Peter Maydell
><peter.maydell@linaro.org>; Iglesias, Francisco <francisco.iglesias@amd.com>;
>Iglesias, Edgar <Edgar.Iglesias@amd.com>; Philippe Mathieu-Daudé
><philmd@linaro.org>; Alistair Francis <alistair@alistair23.me>; Konrad, Frederic
><Frederic.Konrad@amd.com>; Boddu, Sai Pavan <sai.pavan.boddu@amd.com>;
>Michel, Luc <Luc.Michel@amd.com>
>Subject: [PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
>
>From: Francisco Iglesias <francisco.iglesias@xilinx.com>
>
>Introduce a 'first-cpu-index' property for specifying the first QEMU CPU connected to
>the GICv3. This makes it possible to have multiple instances of the GICv3
>connected to different CPU clusters.
>
>For KVM, mark this property has unsupported. It probably does not make much
>sense as it is intented to be used to model non-SMP systems.

Reviewed-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com

Regards,
Sai Pavan
>
>Signed-off-by: Luc Michel <luc.michel@amd.com>
>Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>
>---
> include/hw/intc/arm_gicv3_common.h | 1 +
> hw/intc/arm_gicv3_common.c         | 3 ++-
> hw/intc/arm_gicv3_cpuif.c          | 2 +-
> hw/intc/arm_gicv3_kvm.c            | 6 ++++++
> 4 files changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/include/hw/intc/arm_gicv3_common.h
>b/include/hw/intc/arm_gicv3_common.h
>index c18503869f9..3c2ed30de71 100644
>--- a/include/hw/intc/arm_gicv3_common.h
>+++ b/include/hw/intc/arm_gicv3_common.h
>@@ -226,10 +226,11 @@ struct GICv3State {
>     MemoryRegion iomem_dist; /* Distributor */
>     GICv3RedistRegion *redist_regions; /* Redistributor Regions */
>     uint32_t *redist_region_count; /* redistributor count within each region */
>     uint32_t nb_redist_regions; /* number of redist regions */
>
>+    uint32_t first_cpu_idx;
>     uint32_t num_cpu;
>     uint32_t num_irq;
>     uint32_t revision;
>     uint32_t maint_irq;
>     bool lpi_enable;
>diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index
>e438d8c042d..2d0df6da86c 100644
>--- a/hw/intc/arm_gicv3_common.c
>+++ b/hw/intc/arm_gicv3_common.c
>@@ -434,11 +434,11 @@ static void arm_gicv3_common_realize(DeviceState *dev,
>Error **errp)
>     }
>
>     s->cpu = g_new0(GICv3CPUState, s->num_cpu);
>
>     for (i = 0; i < s->num_cpu; i++) {
>-        CPUState *cpu = qemu_get_cpu(i);
>+        CPUState *cpu = qemu_get_cpu(s->first_cpu_idx + i);
>         uint64_t cpu_affid;
>
>         s->cpu[i].cpu = cpu;
>         s->cpu[i].gic = s;
>         /* Store GICv3CPUState in CPUARMState gicv3state pointer */ @@ -620,10
>+620,11 @@ static const Property arm_gicv3_common_properties[] = {
>     DEFINE_PROP_BOOL("force-8-bit-prio", GICv3State, force_8bit_prio, 0),
>     DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
>                       redist_region_count, qdev_prop_uint32, uint32_t),
>     DEFINE_PROP_LINK("sysmem", GICv3State, dma,
>TYPE_MEMORY_REGION,
>                      MemoryRegion *),
>+    DEFINE_PROP_UINT32("first-cpu-index", GICv3State, first_cpu_idx,
>+ 0),
> };
>
> static void arm_gicv3_common_class_init(ObjectClass *klass, const void *data)  {
>     DeviceClass *dc = DEVICE_CLASS(klass); diff --git a/hw/intc/arm_gicv3_cpuif.c
>b/hw/intc/arm_gicv3_cpuif.c index 4b4cf091570..1af7690b958 100644
>--- a/hw/intc/arm_gicv3_cpuif.c
>+++ b/hw/intc/arm_gicv3_cpuif.c
>@@ -3022,11 +3022,11 @@ void gicv3_init_cpuif(GICv3State *s)
>      * registers with the CPU
>      */
>     int i;
>
>     for (i = 0; i < s->num_cpu; i++) {
>-        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
>+        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(s->first_cpu_idx + i));
>         GICv3CPUState *cs = &s->cpu[i];
>
>         /*
>          * If the CPU doesn't define a GICv3 configuration, probably because
>          * in real hardware it doesn't have one, then we use default values diff --git
>a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index
>6166283cd1a..e6a09c9b7d0 100644
>--- a/hw/intc/arm_gicv3_kvm.c
>+++ b/hw/intc/arm_gicv3_kvm.c
>@@ -807,10 +807,16 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
>Error **errp)
>     if (s->nmi_support) {
>         error_setg(errp, "NMI is not supported with the in-kernel GIC");
>         return;
>     }
>
>+    if (s->first_cpu_idx != 0) {
>+        error_setg(errp, "Non-zero first-cpu-idx is unsupported with the "
>+                   "in-kernel GIC");
>+        return;
>+    }
>+
>     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
>
>     for (i = 0; i < s->num_cpu; i++) {
>         ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
>
>--
>2.50.1
Re: [PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
Posted by Edgar E. Iglesias 2 months, 2 weeks ago
On Fri, Aug 22, 2025 at 05:15:48PM +0200, Luc Michel wrote:
> From: Francisco Iglesias <francisco.iglesias@xilinx.com>
> 
> Introduce a 'first-cpu-index' property for specifying the first QEMU CPU
> connected to the GICv3. This makes it possible to have multiple instances
> of the GICv3 connected to different CPU clusters.
> 
> For KVM, mark this property has unsupported. It probably does not make
> much sense as it is intented to be used to model non-SMP systems.
> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>

Peter, were you looking at solving this with links to CPU's for the next
GIC? Perhaps we should do the same here?

I'm also fine with this patch as simple way of solving this problem.

Cheers,
Edgar




> ---
>  include/hw/intc/arm_gicv3_common.h | 1 +
>  hw/intc/arm_gicv3_common.c         | 3 ++-
>  hw/intc/arm_gicv3_cpuif.c          | 2 +-
>  hw/intc/arm_gicv3_kvm.c            | 6 ++++++
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index c18503869f9..3c2ed30de71 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -226,10 +226,11 @@ struct GICv3State {
>      MemoryRegion iomem_dist; /* Distributor */
>      GICv3RedistRegion *redist_regions; /* Redistributor Regions */
>      uint32_t *redist_region_count; /* redistributor count within each region */
>      uint32_t nb_redist_regions; /* number of redist regions */
>  
> +    uint32_t first_cpu_idx;
>      uint32_t num_cpu;
>      uint32_t num_irq;
>      uint32_t revision;
>      uint32_t maint_irq;
>      bool lpi_enable;
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index e438d8c042d..2d0df6da86c 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -434,11 +434,11 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
>      }
>  
>      s->cpu = g_new0(GICv3CPUState, s->num_cpu);
>  
>      for (i = 0; i < s->num_cpu; i++) {
> -        CPUState *cpu = qemu_get_cpu(i);
> +        CPUState *cpu = qemu_get_cpu(s->first_cpu_idx + i);
>          uint64_t cpu_affid;
>  
>          s->cpu[i].cpu = cpu;
>          s->cpu[i].gic = s;
>          /* Store GICv3CPUState in CPUARMState gicv3state pointer */
> @@ -620,10 +620,11 @@ static const Property arm_gicv3_common_properties[] = {
>      DEFINE_PROP_BOOL("force-8-bit-prio", GICv3State, force_8bit_prio, 0),
>      DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
>                        redist_region_count, qdev_prop_uint32, uint32_t),
>      DEFINE_PROP_LINK("sysmem", GICv3State, dma, TYPE_MEMORY_REGION,
>                       MemoryRegion *),
> +    DEFINE_PROP_UINT32("first-cpu-index", GICv3State, first_cpu_idx, 0),
>  };
>  
>  static void arm_gicv3_common_class_init(ObjectClass *klass, const void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 4b4cf091570..1af7690b958 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -3022,11 +3022,11 @@ void gicv3_init_cpuif(GICv3State *s)
>       * registers with the CPU
>       */
>      int i;
>  
>      for (i = 0; i < s->num_cpu; i++) {
> -        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
> +        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(s->first_cpu_idx + i));
>          GICv3CPUState *cs = &s->cpu[i];
>  
>          /*
>           * If the CPU doesn't define a GICv3 configuration, probably because
>           * in real hardware it doesn't have one, then we use default values
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 6166283cd1a..e6a09c9b7d0 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -807,10 +807,16 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>      if (s->nmi_support) {
>          error_setg(errp, "NMI is not supported with the in-kernel GIC");
>          return;
>      }
>  
> +    if (s->first_cpu_idx != 0) {
> +        error_setg(errp, "Non-zero first-cpu-idx is unsupported with the "
> +                   "in-kernel GIC");
> +        return;
> +    }
> +
>      gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
>  
>      for (i = 0; i < s->num_cpu; i++) {
>          ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
>  
> -- 
> 2.50.1
> 
>
Re: [PATCH v4 23/47] hw/intc/arm_gicv3: Introduce a 'first-cpu-index' property
Posted by Peter Maydell 2 months, 2 weeks ago
On Tue, 26 Aug 2025 at 20:33, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
>
> On Fri, Aug 22, 2025 at 05:15:48PM +0200, Luc Michel wrote:
> > From: Francisco Iglesias <francisco.iglesias@xilinx.com>
> >
> > Introduce a 'first-cpu-index' property for specifying the first QEMU CPU
> > connected to the GICv3. This makes it possible to have multiple instances
> > of the GICv3 connected to different CPU clusters.
> >
> > For KVM, mark this property has unsupported. It probably does not make
> > much sense as it is intented to be used to model non-SMP systems.
> >
> > Signed-off-by: Luc Michel <luc.michel@amd.com>
> > Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>
>
> Peter, were you looking at solving this with links to CPU's for the next
> GIC? Perhaps we should do the same here?
>
> I'm also fine with this patch as simple way of solving this problem.

Yeah, links to CPUs is my plan for GICv5. We don't currently
support having an array of link properties, so you need this patch:
https://lore.kernel.org/qemu-devel/20250606140327.1289993-1-peter.maydell@linaro.org/
which provides the macro for declaring the property array and some
functions for setting it up in the board/SoC code (and a sketch
in the commit message of how to do it).

If you want to have a go at doing it that way, feel free; but
I'm happy with doing what this patch does, since it's what we
already did for GICv2. We can always come back and do the
refactoring later.

thanks
-- PMM