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

[Harsh Prateek Bora]

On 11/22/23 17:01, Cédric Le Goater wrote:
> Hello Harsh,
>
> Please add to your .git/config file:
>
> [diff]
>      orderFile = /path/to/qemu/scripts/git.orderfile

Sure, thanks for the suggestion.

> 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 <harshpb@linux.ibm.com>
> > Suggested-by: Cedric Le Goater <clg@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)

Yeh, this looks much better. Will update and post.

regards,
Harsh
> Thanks,
>
> C.