diff mbox

[3/7] pseries: Add support for level interrupts to XICS

Message ID 1330399093-20384-4-git-send-email-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson Feb. 28, 2012, 3:18 a.m. UTC 
The pseries "xics" interrupt controller, like most interrupt
controllers can support both message (i.e. edge sensitive) interrupts
and level sensitive interrupts, but it needs to know which are which.

When I implemented the xics emulation for qemu, the only devices we
supported were the PAPR virtual IO devices.  These devices only use
message interrupts, so they were the only ones I implemented in xics.

Since then, however, we have added support for PCI devices, which use
level sensitive interrupts.  It turns out the message interrupt logic
still actually works most of the time for these, but there are
circumstances where we can lost interrupts due to the incorrect
interrupt logic.

This patch, therefore, implements the correct xics level-sensitive
interrupt logic.  The type of the interrupt is set when a device
allocates a new xics interrupt.

Signed-off-by: David Gibson <david@gibson@.dropbear.id.au>
---
 hw/spapr.c     |    4 +-
 hw/spapr.h     |   12 +++++-
 hw/spapr_pci.c |    2 +-
 hw/spapr_vio.c |    2 +-
 hw/xics.c      |  122 +++++++++++++++++++++++++++++++++++++++++---------------
 hw/xics.h      |    2 +-
 6 files changed, 106 insertions(+), 38 deletions(-)

Comments

Alexander Graf March 6, 2012, 11:30 p.m. UTC | #1 
On 28.02.2012, at 04:18, David Gibson wrote:

> The pseries "xics" interrupt controller, like most interrupt
> controllers can support both message (i.e. edge sensitive) interrupts
> and level sensitive interrupts, but it needs to know which are which.
> 
> When I implemented the xics emulation for qemu, the only devices we
> supported were the PAPR virtual IO devices.  These devices only use
> message interrupts, so they were the only ones I implemented in xics.
> 
> Since then, however, we have added support for PCI devices, which use
> level sensitive interrupts.  It turns out the message interrupt logic
> still actually works most of the time for these, but there are
> circumstances where we can lost interrupts due to the incorrect
> interrupt logic.
> 
> This patch, therefore, implements the correct xics level-sensitive
> interrupt logic.  The type of the interrupt is set when a device
> allocates a new xics interrupt.
> 
> Signed-off-by: David Gibson <david@gibson@.dropbear.id.au>

This looks wrong

> ---
> hw/spapr.c     |    4 +-
> hw/spapr.h     |   12 +++++-
> hw/spapr_pci.c |    2 +-
> hw/spapr_vio.c |    2 +-
> hw/xics.c      |  122 +++++++++++++++++++++++++++++++++++++++++---------------
> hw/xics.h      |    2 +-
> 6 files changed, 106 insertions(+), 38 deletions(-)
> 
> diff --git a/hw/spapr.c b/hw/spapr.c
> index dffb6a2..828bc53 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -83,7 +83,7 @@
> 
> sPAPREnvironment *spapr;
> 
> -qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
> +qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, int lsi)
> {
>     uint32_t irq;
>     qemu_irq qirq;
> @@ -95,7 +95,7 @@ qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
>         irq = spapr->next_irq++;
>     }
> 
> -    qirq = xics_find_qirq(spapr->icp, irq);
> +    qirq = xics_assign_irq(spapr->icp, irq, lsi);

Find becomes assign? Is this on purpose?

While at it, could you make the "lsi" thing an enum? That'd make things easier to track :).


Alex
David Gibson March 7, 2012, 5:43 a.m. UTC | #2 
On Wed, Mar 07, 2012 at 12:30:40AM +0100, Alexander Graf wrote:
> 
> On 28.02.2012, at 04:18, David Gibson wrote:
> 
> > The pseries "xics" interrupt controller, like most interrupt
> > controllers can support both message (i.e. edge sensitive) interrupts
> > and level sensitive interrupts, but it needs to know which are which.
> > 
> > When I implemented the xics emulation for qemu, the only devices we
> > supported were the PAPR virtual IO devices.  These devices only use
> > message interrupts, so they were the only ones I implemented in xics.
> > 
> > Since then, however, we have added support for PCI devices, which use
> > level sensitive interrupts.  It turns out the message interrupt logic
> > still actually works most of the time for these, but there are
> > circumstances where we can lost interrupts due to the incorrect
> > interrupt logic.
> > 
> > This patch, therefore, implements the correct xics level-sensitive
> > interrupt logic.  The type of the interrupt is set when a device
> > allocates a new xics interrupt.
> > 
> > Signed-off-by: David Gibson <david@gibson@.dropbear.id.au>
> 
> This looks wrong

Oops, I'll fix that up.

> 
> > ---
> > hw/spapr.c     |    4 +-
> > hw/spapr.h     |   12 +++++-
> > hw/spapr_pci.c |    2 +-
> > hw/spapr_vio.c |    2 +-
> > hw/xics.c      |  122 +++++++++++++++++++++++++++++++++++++++++---------------
> > hw/xics.h      |    2 +-
> > 6 files changed, 106 insertions(+), 38 deletions(-)
> > 
> > diff --git a/hw/spapr.c b/hw/spapr.c
> > index dffb6a2..828bc53 100644
> > --- a/hw/spapr.c
> > +++ b/hw/spapr.c
> > @@ -83,7 +83,7 @@
> > 
> > sPAPREnvironment *spapr;
> > 
> > -qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
> > +qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, int lsi)
> > {
> >     uint32_t irq;
> >     qemu_irq qirq;
> > @@ -95,7 +95,7 @@ qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
> >         irq = spapr->next_irq++;
> >     }
> > 
> > -    qirq = xics_find_qirq(spapr->icp, irq);
> > +    qirq = xics_assign_irq(spapr->icp, irq, lsi);
> 
> Find becomes assign? Is this on purpose?

Yes.  Previously xics_find_qirq() just returned the qmeu_irq
associated with a given xics irq number, without changing any state.
Now it also flags whether the irq is an lsi or msi, so the old name
didn't seem right any more.

> While at it, could you make the "lsi" thing an enum? That'd make
> things easier to track :).

Ok, will do.
diff mbox

Patch

diff --git a/hw/spapr.c b/hw/spapr.c
index dffb6a2..828bc53 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -83,7 +83,7 @@ 
 
 sPAPREnvironment *spapr;
 
-qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
+qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, int lsi)
 {
     uint32_t irq;
     qemu_irq qirq;
@@ -95,7 +95,7 @@  qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
         irq = spapr->next_irq++;
     }
 
-    qirq = xics_find_qirq(spapr->icp, irq);
+    qirq = xics_assign_irq(spapr->icp, irq, lsi);
     if (!qirq) {
         return NULL;
     }
diff --git a/hw/spapr.h b/hw/spapr.h
index e946a34..4aae1a0 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -286,7 +286,17 @@  void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
 target_ulong spapr_hypercall(CPUState *env, target_ulong opcode,
                              target_ulong *args);
 
-qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num);
+qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, int lsi);
+
+static inline qemu_irq spapr_allocate_msi(uint32_t hint, uint32_t *irq_num)
+{
+    return spapr_allocate_irq(hint, irq_num, 0);
+}
+
+static inline qemu_irq spapr_allocate_lsi(uint32_t hint, uint32_t *irq_num)
+{
+    return spapr_allocate_irq(hint, irq_num, 1);
+}
 
 static inline uint32_t rtas_ld(target_ulong phys, int n)
 {
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 1e8d03e..28a46bd 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -178,7 +178,7 @@  static int spapr_phb_init(SysBusDevice *s)
         qemu_irq qirq;
         uint32_t num;
 
-        qirq = spapr_allocate_irq(0, &num);
+        qirq = spapr_allocate_lsi(0, &num);
         if (!qirq) {
             return -1;
         }
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index ea317ef..ca0c8e7 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -670,7 +670,7 @@  static int spapr_vio_busdev_init(DeviceState *qdev)
         dev->qdev.id = id;
     }
 
-    dev->qirq = spapr_allocate_irq(dev->vio_irq_num, &dev->vio_irq_num);
+    dev->qirq = spapr_allocate_msi(dev->vio_irq_num, &dev->vio_irq_num);
     if (!dev->qirq) {
         return -1;
     }
diff --git a/hw/xics.c b/hw/xics.c
index 1c5eaa4..3bdff4e 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -132,9 +132,9 @@  static void icp_eoi(struct icp_state *icp, int server, uint32_t xirr)
 {
     struct icp_server_state *ss = icp->ss + server;
 
-    ics_eoi(icp->ics, xirr & XISR_MASK);
     /* Send EOI -> ICS */
     ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
+    ics_eoi(icp->ics, xirr & XISR_MASK);
     if (!XISR(ss)) {
         icp_resend(icp, server);
     }
@@ -165,8 +165,9 @@  struct ics_irq_state {
     int server;
     uint8_t priority;
     uint8_t saved_priority;
-    /* int pending:1; */
-    /* int presented:1; */
+    int lsi:1;
+    int asserted:1;
+    int sent:1;
     int rejected:1;
     int masked_pending:1;
 };
@@ -185,9 +186,32 @@  static int ics_valid_irq(struct ics_state *ics, uint32_t nr)
         && (nr < (ics->offset + ics->nr_irqs));
 }
 
-static void ics_set_irq_msi(void *opaque, int srcno, int val)
+static void resend_msi(struct ics_state *ics, int srcno)
+{
+    struct ics_irq_state *irq = ics->irqs + srcno;
+
+    /* FIXME: filter by server#? */
+    if (irq->rejected) {
+        irq->rejected = 0;
+        if (irq->priority != 0xff) {
+            icp_irq(ics->icp, irq->server, srcno + ics->offset,
+                    irq->priority);
+        }
+    }
+}
+
+static void resend_lsi(struct ics_state *ics, int srcno)
+{
+    struct ics_irq_state *irq = ics->irqs + srcno;
+
+    if ((irq->priority != 0xff) && irq->asserted && !irq->sent) {
+        irq->sent = 1;
+        icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+    }
+}
+
+static void set_irq_msi(struct ics_state *ics, int srcno, int val)
 {
-    struct ics_state *ics = (struct ics_state *)opaque;
     struct ics_irq_state *irq = ics->irqs + srcno;
 
     if (val) {
@@ -200,71 +224,105 @@  static void ics_set_irq_msi(void *opaque, int srcno, int val)
     }
 }
 
-static void ics_reject_msi(struct ics_state *ics, int nr)
+static void set_irq_lsi(struct ics_state *ics, int srcno, int val)
 {
-    struct ics_irq_state *irq = ics->irqs + nr - ics->offset;
+    struct ics_irq_state *irq = ics->irqs + srcno;
 
-    irq->rejected = 1;
+    irq->asserted = val;
+    resend_lsi(ics, srcno);
 }
 
-static void ics_resend_msi(struct ics_state *ics)
+static void ics_set_irq(void *opaque, int srcno, int val)
 {
-    int i;
+    struct ics_state *ics = (struct ics_state *)opaque;
+    struct ics_irq_state *irq = ics->irqs + srcno;
 
-    for (i = 0; i < ics->nr_irqs; i++) {
-        struct ics_irq_state *irq = ics->irqs + i;
+    if (irq->lsi) {
+        set_irq_lsi(ics, srcno, val);
+    } else {
+        set_irq_msi(ics, srcno, val);
+    }
+}
 
-        /* FIXME: filter by server#? */
-        if (irq->rejected) {
-            irq->rejected = 0;
-            if (irq->priority != 0xff) {
-                icp_irq(ics->icp, irq->server, i + ics->offset, irq->priority);
-            }
-        }
+static void write_xive_msi(struct ics_state *ics, int srcno)
+{
+    struct ics_irq_state *irq = ics->irqs + srcno;
+
+    if (!irq->masked_pending || (irq->priority == 0xff)) {
+        return;
     }
+
+    irq->masked_pending = 0;
+    icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
 }
 
-static void ics_write_xive_msi(struct ics_state *ics, int nr, int server,
-                               uint8_t priority)
+static void write_xive_lsi(struct ics_state *ics, int srcno)
 {
-    struct ics_irq_state *irq = ics->irqs + nr - ics->offset;
+    resend_lsi(ics, srcno);
+}
+
+static void ics_write_xive(struct ics_state *ics, int nr, int server,
+                           uint8_t priority)
+{
+    int srcno = nr - ics->offset;
+    struct ics_irq_state *irq = ics->irqs + srcno;
 
     irq->server = server;
     irq->priority = priority;
 
-    if (!irq->masked_pending || (priority == 0xff)) {
-        return;
+    if (irq->lsi) {
+        write_xive_lsi(ics, srcno);
+    } else {
+        write_xive_msi(ics, srcno);
     }
-
-    irq->masked_pending = 0;
-    icp_irq(ics->icp, server, nr, priority);
 }
 
 static void ics_reject(struct ics_state *ics, int nr)
 {
-    ics_reject_msi(ics, nr);
+    struct ics_irq_state *irq = ics->irqs + nr - ics->offset;
+
+    irq->rejected = 1; /* Irrelevant but harmless for LSI */
+    irq->sent = 0; /* Irrelevant but harmless for MSI */
 }
 
 static void ics_resend(struct ics_state *ics)
 {
-    ics_resend_msi(ics);
+    int i;
+
+    for (i = 0; i < ics->nr_irqs; i++) {
+        struct ics_irq_state *irq = ics->irqs + i;
+
+        /* FIXME: filter by server#? */
+        if (irq->lsi) {
+            resend_lsi(ics, i);
+        } else {
+            resend_msi(ics, i);
+        }
+    }
 }
 
 static void ics_eoi(struct ics_state *ics, int nr)
 {
+    int srcno = nr - ics->offset;
+    struct ics_irq_state *irq = ics->irqs + srcno;
+
+    if (irq->lsi) {
+        irq->sent = 0;
+    }
 }
 
 /*
  * Exported functions
  */
 
-qemu_irq xics_find_qirq(struct icp_state *icp, int irq)
+qemu_irq xics_assign_irq(struct icp_state *icp, int irq, int lsi)
 {
     if ((irq < icp->ics->offset)
         || (irq >= (icp->ics->offset + icp->ics->nr_irqs))) {
         return NULL;
     }
 
+    icp->ics->irqs[irq - icp->ics->offset].lsi = !!lsi;
     return icp->ics->qirqs[irq - icp->ics->offset];
 }
 
@@ -332,7 +390,7 @@  static void rtas_set_xive(sPAPREnvironment *spapr, uint32_t token,
         return;
     }
 
-    ics_write_xive_msi(ics, nr, server, priority);
+    ics_write_xive(ics, nr, server, priority);
 
     rtas_st(rets, 0, 0); /* Success */
 }
@@ -477,7 +535,7 @@  struct icp_state *xics_system_init(int nr_irqs)
         ics->irqs[i].saved_priority = 0xff;
     }
 
-    ics->qirqs = qemu_allocate_irqs(ics_set_irq_msi, ics, nr_irqs);
+    ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, nr_irqs);
 
     spapr_register_hypercall(H_CPPR, h_cppr);
     spapr_register_hypercall(H_IPI, h_ipi);
diff --git a/hw/xics.h b/hw/xics.h
index 83c1182..7cb2b64 100644
--- a/hw/xics.h
+++ b/hw/xics.h
@@ -31,7 +31,7 @@ 
 
 struct icp_state;
 
-qemu_irq xics_find_qirq(struct icp_state *icp, int irq);
+qemu_irq xics_assign_irq(struct icp_state *icp, int irq, int lsi);
 
 struct icp_state *xics_system_init(int nr_irqs);