| Message ID | 20231122092845.973949-2-harshpb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce SPAPR_NR_IPIS and fix max-cpus | expand |
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>
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.
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. > >
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;
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(-)