Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.

[Cédric Le Goater]

Hello Harsh,

Please add to your .git/config file:

[diff]
        orderFile = /path/to/qemu/scripts/git.orderfile


On 11/22/23 10:28, Harsh Prateek Bora wrote:

spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
the range of CPU IPIs during initialization of nr-irqs property.
It is more appropriate to have its own define which can be further
reused as appropriate for correct interpretation.

Signed-off-by: Harsh Prateek Bora <hars...@linux.ibm.com>
Suggested-by: Cedric Le Goater <c...@kaod.org>
---
  hw/ppc/spapr_irq.c         | 4 ++--
  include/hw/ppc/spapr_irq.h | 1 +
  2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..0c5db6b161 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -329,7 +329,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
          int i;

dev = qdev_new(TYPE_SPAPR_XIVE);

-        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_XIRQ_BASE);
+        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_NR_IPIS);

SPAPR_IRQ_NR_IPIS ?

          /*
           * 8 XIVE END structures per CPU. One for each available
           * priority
@@ -356,7 +356,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
      }

spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,

-                                      smc->nr_xirqs + SPAPR_XIRQ_BASE);
+                                      smc->nr_xirqs + SPAPR_NR_IPIS);

/*

       * Mostly we don't actually need this until reset, except that not
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index c22a72c9e2..e7a80a8349 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -28,6 +28,7 @@

In include/hw/ppc/spapr_irq.h, we should describe the ranges a bit more.
See commit dcc345b61ebe and ad8de98636e7 for more info. Something like :

  /*
   * The XIVE IRQ backend uses the same layout as the XICS backend but
   * covers the full range of the IRQ number space. The IRQ numbers for
   * the CPU IPIs are allocated at the bottom of this space, below 4K,
   * to preserve compatibility with XICS which does not use that range.
   */

  /*
   * CPU IPI range (XIVE only)
   */
  #define SPAPR_IRQ_IPI        0x0
  #define SPAPR_IRQ_NR_IPIS    0x1000

  /*
   * IRQ range offsets per device type
   */
  #define SPAPR_XIRQ_BASE      XICS_IRQ_BASE /* 0x1000 */


And to make sure the ranges don't overlap, let's add :

  QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE)


Thanks,

C.