[PATCH v3 11/34] spapr: Fix indexing of XICS irqs

From: David Gibson <david@gibson.dropbear.id.au>
To: qemu-ppc@nongnu.org, clg@kaod.org, qemu-devel@nongnu.org
Cc: "Jason Wang" <jasowang@redhat.com>,
	"Riku Voipio" <riku.voipio@iki.fi>,
	"Laurent Vivier" <laurent@vivier.eu>,
	groug@kaod.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	philmd@redhat.com, "David Gibson" <david@gibson.dropbear.id.au>
Subject: [PATCH v3 11/34] spapr: Fix indexing of XICS irqs
Date: Wed,  2 Oct 2019 12:51:45 +1000	[thread overview]
Message-ID: <20191002025208.3487-12-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20191002025208.3487-1-david@gibson.dropbear.id.au>

spapr global irq numbers are different from the source numbers on the ICS
when using XICS - they're offset by XICS_IRQ_BASE (0x1000).  But
spapr_irq_set_irq_xics() was passing through the global irq number to
the ICS code unmodified.

We only got away with this because of a counteracting bug - we were
incorrectly adjusting the qemu_irq we returned for a requested global irq
number.

That approach mostly worked but is very confusing, incorrectly relies on
the way the qemu_irq array is allocated, and undermines the intention of
having the global array of qemu_irqs for spapr have a consistent meaning
regardless of irq backend.

So, fix both set_irq and qemu_irq indexing.  We rename some parameters at
the same time to make it clear that they are referring to spapr global
irq numbers.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_irq.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index cded3a0154..8f79aa829f 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -153,10 +153,9 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num)
 static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq)
 {
     ICSState *ics = spapr->ics;
-    uint32_t srcno = irq - ics->offset;
 
     if (ics_valid_irq(ics, irq)) {
-        return spapr->qirqs[srcno];
+        return spapr->qirqs[irq];
     }
 
     return NULL;
@@ -204,9 +203,10 @@ static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id)
     return 0;
 }
 
-static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val)
+static void spapr_irq_set_irq_xics(void *opaque, int irq, int val)
 {
     SpaprMachineState *spapr = opaque;
+    uint32_t srcno = irq - spapr->ics->offset;
 
     ics_set_irq(spapr->ics, srcno, val);
 }
@@ -377,14 +377,14 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp)
     spapr_xive_mmio_set_enabled(spapr->xive, true);
 }
 
-static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
+static void spapr_irq_set_irq_xive(void *opaque, int irq, int val)
 {
     SpaprMachineState *spapr = opaque;
 
     if (kvm_irqchip_in_kernel()) {
-        kvmppc_xive_source_set_irq(&spapr->xive->source, srcno, val);
+        kvmppc_xive_source_set_irq(&spapr->xive->source, irq, val);
     } else {
-        xive_source_set_irq(&spapr->xive->source, srcno, val);
+        xive_source_set_irq(&spapr->xive->source, irq, val);
     }
 }
 
@@ -558,11 +558,11 @@ static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp)
     spapr_irq_current(spapr)->reset(spapr, errp);
 }
 
-static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
+static void spapr_irq_set_irq_dual(void *opaque, int irq, int val)
 {
     SpaprMachineState *spapr = opaque;
 
-    spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
+    spapr_irq_current(spapr)->set_irq(spapr, irq, val);
 }
 
 static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
-- 
2.21.0


