On 11/24/23 09:01, Harsh Prateek Bora wrote:
On 11/23/23 19:42, Cédric Le Goater wrote:
On 11/23/23 10:31, Harsh Prateek Bora wrote:
On 11/23/23 14:20, CI've applied these patches and verified on the
latest upstream qemu. The code is working as expected.
Tested-by: Kowshik Jois<kowsjois@linux.ibm.com>édric Le Goater wrote:
On 11/23/23 06:57, 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>
One comment below
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks, responding below ..
---
include/hw/ppc/spapr_irq.h | 14 +++++++++++++-
hw/ppc/spapr_irq.c | 6 ++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index c22a72c9e2..4fd2d5853d 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -14,9 +14,21 @@
#include "qom/object.h"
/*
- * IRQ range offsets per device type
+ * 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 */
#define SPAPR_IRQ_EPOW (SPAPR_XIRQ_BASE + 0x0000)
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..97b2fc42ab 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -23,6 +23,8 @@
#include "trace.h"
+QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE);
+
I would have put the check in include/hw/ppc/spapr_irq.h but since
SPAPR_XIRQ_BASE is only used in hw/ppc/spapr_irq.c which is always
compiled, this is fine. You might want to change that in case a
respin is asked for.
I had initially tried keeping it in spapr_irq.h , but that would
give a build break for XICS_IRQ_BASE not defined since that gets
defined in spapr_xics.h and is included later in some files,
however, the QEMU_BUILD_BUG_ON expects it to be defined before it
reaches here.
ah. good catch. this went unnoticed and is a bit ugly. We should fix
in some ways. May with a define SPAPR_XIRQ_BASE to 0x1000 simply ?
Hmm, I can do that if a re-spin is reqd, or can be done as a separate
patch later also along with other improvements.
yes. This is food for thoughts for further improvements.
Thanks,
C.
Also, we could probably define the ICS offset to SPAPR_XIRQ_BASE
directly under spapr_irq_init() and get rid of ics_instance_init().
The HW IRQ Number offset in the PNV ICS instances is assigned
dynamically by the OS (see pnv_phb3). So it should befine to do
the same for spapr. In which case we can get rid of XICS_IRQ_BASE.
Hmm, I am not so familiar with XICS yet, so not sure if we really need
to do that, but it can be done along with other improvements if needed.
regards,
Harsh
Thanks,
C.
regards,
Harsh
Thanks,
C.
static const TypeInfo spapr_intc_info = {
.name = TYPE_SPAPR_INTC,
.parent = TYPE_INTERFACE,
@@ -329,7 +331,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_IRQ_NR_IPIS);
/*
* 8 XIVE END structures per CPU. One for each available
* priority
@@ -356,7 +358,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_IRQ_NR_IPIS);
/*
* Mostly we don't actually need this until reset, except
that not
I've applied these patches and verified on the latest upstream qemu.
The code is working as expected.
Tested-by: Kowshik Jois<kowsjois@linux.ibm.com>