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);
/*
* 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 @@
#define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300)
#define SPAPR_NR_XIRQS 0x1000
+#define SPAPR_NR_IPIS 0x1000
struct SpaprMachineState;
--
2.39.3
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 <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)
Thanks,
C.
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. > >
Hi Harsh, On 22/11/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/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 @@ > #define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300) > > #define SPAPR_NR_XIRQS 0x1000 > +#define SPAPR_NR_IPIS 0x1000 BTW why hexadecimal and not decimal? Regards, Phil.
On 11/22/23 12:13, Philippe Mathieu-Daudé wrote: > Hi Harsh, > > On 22/11/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/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 @@ >> #define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300) >> #define SPAPR_NR_XIRQS 0x1000 >> +#define SPAPR_NR_IPIS 0x1000 > > BTW why hexadecimal and not decimal? I think because the HW IRQs are displayed in hex under Linux. Probably. It has been a while. Thanks, C.
On 22/11/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(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2026 Red Hat, Inc.