[Qemu-devel] [PATCH v2 0/4] openpic cleanups and fixes

[Qemu-devel] [PATCH v2 0/4] openpic cleanups and fixes

[Scott Wood]

Respin of the patches from the 12/21 patchset that were not already applied.

Scott Wood (4):
  openpic: fix sense and priority bits
  openpic: IRQ_check: search the queue a word at a time
  openpic: move IACK to its own function
  openpic: fix CTPR and de-assertion of interrupts

 hw/openpic.c |  365 ++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 255 insertions(+), 110 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH v2 1/4] openpic: fix sense and priority bits

[Scott Wood]

Previously, the sense and priority bits were masked off when writing
to IVPR, and all interrupts were treated as edge-triggered (despite
the existence of code for handling level-triggered interrupts).

Polarity is implemented only as storage.  We don't simulate the
bad effects that you'd get on real hardware if you set this incorrectly,
but at least the guest sees the right thing when it reads back the register.

Sense now controls level/edge on FSL external interrupts (and all
interrupts on non-FSL MPIC).  FSL internal interrupts do not have a sense
bit (reads as zero), but are level.  FSL timers and IPIs do not have
sense or polarity bits (read as zero), and are edge-triggered.  To
accommodate FSL internal interrupts, QEMU's internal notion of whether an
interrupt is level-triggered is separated from the IVPR bit.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v2: use an enum rather than multiple booleans for IRQ type.

 hw/openpic.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 71 insertions(+), 6 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 35a7fe3..66179c2 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -162,6 +162,12 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
 static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
                                        uint32_t val, int idx);
 
+typedef enum IRQType {
+    IRQ_TYPE_NORMAL = 0,
+    IRQ_TYPE_FSLINT,        /* FSL internal interrupt -- level only */
+    IRQ_TYPE_FSLSPECIAL,    /* FSL timer/IPI interrupt, edge, no polarity */
+} IRQType;
+
 typedef struct IRQQueue {
     /* Round up to the nearest 64 IRQs so that the queue length
      * won't change when moving between 32 and 64 bit hosts.
@@ -178,6 +184,8 @@ typedef struct IRQSource {
     int last_cpu;
     int output;     /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
     int pending;    /* TRUE if IRQ is pending */
+    IRQType type;
+    bool level:1;   /* level-triggered */
     bool nomask:1;  /* critical interrupts ignore mask on some FSL MPICs */
 } IRQSource;
 
@@ -422,7 +430,7 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
     src = &opp->src[n_IRQ];
     DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
             n_IRQ, level, src->ivpr);
-    if (src->ivpr & IVPR_SENSE_MASK) {
+    if (src->level) {
         /* level-sensitive irq */
         src->pending = level;
         if (!level) {
@@ -455,6 +463,19 @@ static void openpic_reset(DeviceState *d)
     for (i = 0; i < opp->max_irq; i++) {
         opp->src[i].ivpr = opp->ivpr_reset;
         opp->src[i].idr  = opp->idr_reset;
+
+        switch (opp->src[i].type) {
+        case IRQ_TYPE_NORMAL:
+            opp->src[i].level = !!(opp->ivpr_reset & IVPR_SENSE_MASK);
+            break;
+
+        case IRQ_TYPE_FSLINT:
+            opp->src[i].ivpr |= IVPR_POLARITY_MASK;
+            break;
+
+        case IRQ_TYPE_FSLSPECIAL:
+            break;
+        }
     }
     /* Initialise IRQ destinations */
     for (i = 0; i < MAX_CPU; i++) {
@@ -530,10 +551,36 @@ static inline void write_IRQreg_idr(OpenPICState *opp, int n_IRQ, uint32_t val)
 
 static inline void write_IRQreg_ivpr(OpenPICState *opp, int n_IRQ, uint32_t val)
 {
-    /* NOTE: not fully accurate for special IRQs, but simple and sufficient */
+    uint32_t mask;
+
+    /* NOTE when implementing newer FSL MPIC models: starting with v4.0,
+     * the polarity bit is read-only on internal interrupts.
+     */
+    mask = IVPR_MASK_MASK | IVPR_PRIORITY_MASK | IVPR_SENSE_MASK |
+           IVPR_POLARITY_MASK | opp->vector_mask;
+
     /* ACTIVITY bit is read-only */
-    opp->src[n_IRQ].ivpr = (opp->src[n_IRQ].ivpr & IVPR_ACTIVITY_MASK) |
-        (val & (IVPR_MASK_MASK | IVPR_PRIORITY_MASK | opp->vector_mask));
+    opp->src[n_IRQ].ivpr =
+        (opp->src[n_IRQ].ivpr & IVPR_ACTIVITY_MASK) | (val & mask);
+
+    /* For FSL internal interrupts, The sense bit is reserved and zero,
+     * and the interrupt is always level-triggered.  Timers and IPIs
+     * have no sense or polarity bits, and are edge-triggered.
+     */
+    switch (opp->src[n_IRQ].type) {
+    case IRQ_TYPE_NORMAL:
+        opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ivpr & IVPR_SENSE_MASK);
+        break;
+
+    case IRQ_TYPE_FSLINT:
+        opp->src[n_IRQ].ivpr &= ~IVPR_SENSE_MASK;
+        break;
+
+    case IRQ_TYPE_FSLSPECIAL:
+        opp->src[n_IRQ].ivpr &= ~(IVPR_POLARITY_MASK | IVPR_SENSE_MASK);
+        break;
+    }
+
     openpic_update_irq(opp, n_IRQ);
     DPRINTF("Set IVPR %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
             opp->src[n_IRQ].ivpr);
@@ -976,7 +1023,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
                 retval = IVPR_VECTOR(opp, src->ivpr);
             }
             IRQ_resetbit(&dst->raised, n_IRQ);
-            if (!(src->ivpr & IVPR_SENSE_MASK)) {
+            if (!src->level) {
                 /* edge-sensitive IRQ */
                 src->ivpr &= ~IVPR_ACTIVITY_MASK;
                 src->pending = 0;
@@ -984,7 +1031,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
 
             if ((n_IRQ >= opp->irq_ipi0) &&  (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
                 src->idr &= ~(1 << idx);
-                if (src->idr && !(src->ivpr & IVPR_SENSE_MASK)) {
+                if (src->idr && !src->level) {
                     /* trigger on CPUs that didn't know about it yet */
                     openpic_set_irq(opp, n_IRQ, 1);
                     openpic_set_irq(opp, n_IRQ, 0);
@@ -1282,7 +1329,25 @@ static int openpic_init(SysBusDevice *dev)
         opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
         msi_supported = true;
         list = list_be;
+
+        for (i = 0; i < FSL_MPIC_20_MAX_EXT; i++) {
+            opp->src[i].level = false;
+        }
+
+        /* Internal interrupts, including message and MSI */
+        for (i = 16; i < MAX_SRC; i++) {
+            opp->src[i].type = IRQ_TYPE_FSLINT;
+            opp->src[i].level = true;
+        }
+
+        /* timers and IPIs */
+        for (i = MAX_SRC; i < MAX_IRQ; i++) {
+            opp->src[i].type = IRQ_TYPE_FSLSPECIAL;
+            opp->src[i].level = false;
+        }
+
         break;
+
     case OPENPIC_MODEL_RAVEN:
         opp->nb_irqs = RAVEN_MAX_EXT;
         opp->vid = VID_REVISION_1_3;
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 2/4] openpic: IRQ_check: search the queue a word at a time

[Scott Wood]

Search the queue more efficiently by first looking for a non-zero word,
and then using the common bit-searching function to find the bit within
the word.  It would be even nicer if bitops_ffsl() could be hooked up
to the compiler intrinsic so that bit-searching instructions could be
used, but that's another matter.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v2: use find_next_bit

 hw/openpic.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 66179c2..7645d67 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -277,21 +277,25 @@ static inline int IRQ_testbit(IRQQueue *q, int n_IRQ)
 
 static void IRQ_check(OpenPICState *opp, IRQQueue *q)
 {
-    int next, i;
-    int priority;
+    int irq = -1;
+    int next = -1;
+    int priority = -1;
 
-    next = -1;
-    priority = -1;
-    for (i = 0; i < opp->max_irq; i++) {
-        if (IRQ_testbit(q, i)) {
-            DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
-                    i, IVPR_PRIORITY(opp->src[i].ivpr), priority);
-            if (IVPR_PRIORITY(opp->src[i].ivpr) > priority) {
-                next = i;
-                priority = IVPR_PRIORITY(opp->src[i].ivpr);
-            }
+    for (;;) {
+        irq = find_next_bit(q->queue, opp->max_irq, irq + 1);
+        if (irq == opp->max_irq) {
+            break;
+        }
+
+        DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
+                irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
+
+        if (IVPR_PRIORITY(opp->src[irq].ivpr) > priority) {
+            next = irq;
+            priority = IVPR_PRIORITY(opp->src[irq].ivpr);
         }
     }
+
     q->next = next;
     q->priority = priority;
 }
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 3/4] openpic: move IACK to its own function

[Scott Wood]

Besides making the code cleaner, we will need a separate way to access
IACK in order to implement EPR (external proxy) interrupt delivery.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v2: rebase on latest code

 hw/openpic.c |   95 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 53 insertions(+), 42 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 7645d67..374f80e 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -975,14 +975,64 @@ static void openpic_cpu_write(void *opaque, hwaddr addr, uint64_t val,
     openpic_cpu_write_internal(opaque, addr, val, (addr & 0x1f000) >> 12);
 }
 
+
+static uint32_t openpic_iack(OpenPICState *opp, IRQDest *dst, int cpu)
+{
+    IRQSource *src;
+    int retval, irq;
+
+    DPRINTF("Lower OpenPIC INT output\n");
+    qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
+
+    irq = IRQ_get_next(opp, &dst->raised);
+    DPRINTF("IACK: irq=%d\n", irq);
+
+    if (irq == -1) {
+        /* No more interrupt pending */
+        return opp->spve;
+    }
+
+    src = &opp->src[irq];
+    if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
+            !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
+        /* - Spurious level-sensitive IRQ
+         * - Priorities has been changed
+         *   and the pending IRQ isn't allowed anymore
+         */
+        src->ivpr &= ~IVPR_ACTIVITY_MASK;
+        retval = opp->spve;
+    } else {
+        /* IRQ enter servicing state */
+        IRQ_setbit(&dst->servicing, irq);
+        retval = IVPR_VECTOR(opp, src->ivpr);
+    }
+    IRQ_resetbit(&dst->raised, irq);
+    if (!src->level) {
+        /* edge-sensitive IRQ */
+        src->ivpr &= ~IVPR_ACTIVITY_MASK;
+        src->pending = 0;
+    }
+
+    if ((irq >= opp->irq_ipi0) &&  (irq < (opp->irq_ipi0 + MAX_IPI))) {
+        src->idr &= ~(1 << cpu);
+        if (src->idr && !src->level) {
+            /* trigger on CPUs that didn't know about it yet */
+            openpic_set_irq(opp, irq, 1);
+            openpic_set_irq(opp, irq, 0);
+            /* if all CPUs knew about it, set active bit again */
+            src->ivpr |= IVPR_ACTIVITY_MASK;
+        }
+    }
+
+    return retval;
+}
+
 static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
                                           int idx)
 {
     OpenPICState *opp = opaque;
-    IRQSource *src;
     IRQDest *dst;
     uint32_t retval;
-    int n_IRQ;
 
     DPRINTF("%s: cpu %d addr %#" HWADDR_PRIx "\n", __func__, idx, addr);
     retval = 0xFFFFFFFF;
@@ -1004,46 +1054,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
         retval = idx;
         break;
     case 0xA0: /* IACK */
-        DPRINTF("Lower OpenPIC INT output\n");
-        qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
-        n_IRQ = IRQ_get_next(opp, &dst->raised);
-        DPRINTF("IACK: irq=%d\n", n_IRQ);
-        if (n_IRQ == -1) {
-            /* No more interrupt pending */
-            retval = opp->spve;
-        } else {
-            src = &opp->src[n_IRQ];
-            if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
-                !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
-                /* - Spurious level-sensitive IRQ
-                 * - Priorities has been changed
-                 *   and the pending IRQ isn't allowed anymore
-                 */
-                src->ivpr &= ~IVPR_ACTIVITY_MASK;
-                retval = opp->spve;
-            } else {
-                /* IRQ enter servicing state */
-                IRQ_setbit(&dst->servicing, n_IRQ);
-                retval = IVPR_VECTOR(opp, src->ivpr);
-            }
-            IRQ_resetbit(&dst->raised, n_IRQ);
-            if (!src->level) {
-                /* edge-sensitive IRQ */
-                src->ivpr &= ~IVPR_ACTIVITY_MASK;
-                src->pending = 0;
-            }
-
-            if ((n_IRQ >= opp->irq_ipi0) &&  (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
-                src->idr &= ~(1 << idx);
-                if (src->idr && !src->level) {
-                    /* trigger on CPUs that didn't know about it yet */
-                    openpic_set_irq(opp, n_IRQ, 1);
-                    openpic_set_irq(opp, n_IRQ, 0);
-                    /* if all CPUs knew about it, set active bit again */
-                    src->ivpr |= IVPR_ACTIVITY_MASK;
-                }
-            }
-        }
+        retval = openpic_iack(opp, dst, idx);
         break;
     case 0xB0: /* EOI */
         retval = 0;
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 4/4] openpic: fix CTPR and de-assertion of interrupts

[Scott Wood]

Properly implement level-triggered interrupts by withdrawing an
interrupt from the raised queue if the interrupt source de-asserts.
Also withdraw from the raised queue if the interrupt becomes masked.

When CTPR is written, check whether we need to raise or lower the
interrupt output.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v2: rebase on latest code and shrink a debug print to avoid
the question of whether it should be wrapped

 hw/openpic.c |  181 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 123 insertions(+), 58 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 374f80e..e773d68 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -213,6 +213,9 @@ typedef struct IRQDest {
     IRQQueue raised;
     IRQQueue servicing;
     qemu_irq *irqs;
+
+    /* Count of IRQ sources asserting on non-INT outputs */
+    uint32_t outputs_active[OPENPIC_OUTPUT_NB];
 } IRQDest;
 
 typedef struct OpenPICState {
@@ -308,7 +311,8 @@ static int IRQ_get_next(OpenPICState *opp, IRQQueue *q)
     return q->next;
 }
 
-static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
+static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ,
+                           bool active, bool was_active)
 {
     IRQDest *dst;
     IRQSource *src;
@@ -317,79 +321,113 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
     dst = &opp->dst[n_CPU];
     src = &opp->src[n_IRQ];
 
+    DPRINTF("%s: IRQ %d active %d was %d\n",
+            __func__, n_IRQ, active, was_active);
+
     if (src->output != OPENPIC_OUTPUT_INT) {
+        DPRINTF("%s: output %d irq %d active %d was %d count %d\n",
+                __func__, src->output, n_IRQ, active, was_active,
+                dst->outputs_active[src->output]);
+
         /* On Freescale MPIC, critical interrupts ignore priority,
          * IACK, EOI, etc.  Before MPIC v4.1 they also ignore
          * masking.
          */
-        src->ivpr |= IVPR_ACTIVITY_MASK;
-        DPRINTF("%s: Raise OpenPIC output %d cpu %d irq %d\n",
-                __func__, src->output, n_CPU, n_IRQ);
-        qemu_irq_raise(opp->dst[n_CPU].irqs[src->output]);
+        if (active) {
+            if (!was_active && dst->outputs_active[src->output]++ == 0) {
+                DPRINTF("%s: Raise OpenPIC output %d cpu %d irq %d\n",
+                        __func__, src->output, n_CPU, n_IRQ);
+                qemu_irq_raise(dst->irqs[src->output]);
+            }
+        } else {
+            if (was_active && --dst->outputs_active[src->output] == 0) {
+                DPRINTF("%s: Lower OpenPIC output %d cpu %d irq %d\n",
+                        __func__, src->output, n_CPU, n_IRQ);
+                qemu_irq_lower(dst->irqs[src->output]);
+            }
+        }
+
         return;
     }
 
     priority = IVPR_PRIORITY(src->ivpr);
-    if (priority <= dst->ctpr) {
-        /* Too low priority */
-        DPRINTF("%s: IRQ %d has too low priority on CPU %d\n",
-                __func__, n_IRQ, n_CPU);
-        return;
-    }
-    if (IRQ_testbit(&dst->raised, n_IRQ)) {
-        /* Interrupt miss */
-        DPRINTF("%s: IRQ %d was missed on CPU %d\n",
-                __func__, n_IRQ, n_CPU);
-        return;
-    }
-    src->ivpr |= IVPR_ACTIVITY_MASK;
-    IRQ_setbit(&dst->raised, n_IRQ);
-    if (priority < dst->raised.priority) {
-        /* An higher priority IRQ is already raised */
-        DPRINTF("%s: IRQ %d is hidden by raised IRQ %d on CPU %d\n",
-                __func__, n_IRQ, dst->raised.next, n_CPU);
-        return;
+
+    /* Even if the interrupt doesn't have enough priority,
+     * it is still raised, in case ctpr is lowered later.
+     */
+    if (active) {
+        IRQ_setbit(&dst->raised, n_IRQ);
+    } else {
+        IRQ_resetbit(&dst->raised, n_IRQ);
     }
+
     IRQ_check(opp, &dst->raised);
-    if (IRQ_get_next(opp, &dst->servicing) != -1 &&
-        priority <= dst->servicing.priority) {
-        DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n",
-                __func__, n_IRQ, dst->servicing.next, n_CPU);
-        /* Already servicing a higher priority IRQ */
-        return;
+
+    if (active && priority <= dst->ctpr) {
+        DPRINTF("%s: IRQ %d priority %d too low for ctpr %d on CPU %d\n",
+                __func__, n_IRQ, priority, dst->ctpr, n_CPU);
+        active = 0;
+    }
+
+    if (active) {
+        if (IRQ_get_next(opp, &dst->servicing) >= 0 &&
+                priority <= dst->servicing.priority) {
+            DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n",
+                    __func__, n_IRQ, dst->servicing.next, n_CPU);
+        } else {
+            DPRINTF("%s: Raise OpenPIC INT output cpu %d irq %d/%d\n",
+                    __func__, n_CPU, n_IRQ, dst->raised.next);
+            qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+        }
+    } else {
+        IRQ_get_next(opp, &dst->servicing);
+        if (dst->raised.priority > dst->ctpr &&
+                dst->raised.priority > dst->servicing.priority) {
+            DPRINTF("%s: IRQ %d inactive, IRQ %d prio %d above %d/%d, CPU %d\n",
+                    __func__, n_IRQ, dst->raised.next, dst->raised.priority,
+                    dst->ctpr, dst->servicing.priority, n_CPU);
+            /* IRQ line stays asserted */
+        } else {
+            DPRINTF("%s: IRQ %d inactive, current prio %d/%d, CPU %d\n",
+                    __func__, n_IRQ, dst->ctpr, dst->servicing.priority, n_CPU);
+            qemu_irq_lower(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+        }
     }
-    DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n", n_CPU, n_IRQ);
-    qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
 }
 
 /* update pic state because registers for n_IRQ have changed value */
 static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
 {
     IRQSource *src;
+    bool active, was_active;
     int i;
 
     src = &opp->src[n_IRQ];
+    active = src->pending;
 
-    if (!src->pending) {
-        /* no irq pending */
-        DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
-        return;
-    }
     if ((src->ivpr & IVPR_MASK_MASK) && !src->nomask) {
         /* Interrupt source is disabled */
         DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
-        return;
+        active = false;
     }
-    if (IVPR_PRIORITY(src->ivpr) == 0) {
-        /* Priority set to zero */
-        DPRINTF("%s: IRQ %d has 0 priority\n", __func__, n_IRQ);
+
+    was_active = !!(src->ivpr & IVPR_ACTIVITY_MASK);
+
+    /*
+     * We don't have a similar check for already-active because
+     * ctpr may have changed and we need to withdraw the interrupt.
+     */
+    if (!active && !was_active) {
+        DPRINTF("%s: IRQ %d is already inactive\n", __func__, n_IRQ);
         return;
     }
-    if (src->ivpr & IVPR_ACTIVITY_MASK) {
-        /* IRQ already active */
-        DPRINTF("%s: IRQ %d is already active\n", __func__, n_IRQ);
-        return;
+
+    if (active) {
+        src->ivpr |= IVPR_ACTIVITY_MASK;
+    } else {
+        src->ivpr &= ~IVPR_ACTIVITY_MASK;
     }
+
     if (src->idr == 0) {
         /* No target */
         DPRINTF("%s: IRQ %d has no target\n", __func__, n_IRQ);
@@ -398,12 +436,12 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
 
     if (src->idr == (1 << src->last_cpu)) {
         /* Only one CPU is allowed to receive this IRQ */
-        IRQ_local_pipe(opp, src->last_cpu, n_IRQ);
+        IRQ_local_pipe(opp, src->last_cpu, n_IRQ, active, was_active);
     } else if (!(src->ivpr & IVPR_MODE_MASK)) {
         /* Directed delivery mode */
         for (i = 0; i < opp->nb_cpus; i++) {
             if (src->destmask & (1 << i)) {
-                IRQ_local_pipe(opp, i, n_IRQ);
+                IRQ_local_pipe(opp, i, n_IRQ, active, was_active);
             }
         }
     } else {
@@ -413,7 +451,7 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
                 i = 0;
             }
             if (src->destmask & (1 << i)) {
-                IRQ_local_pipe(opp, i, n_IRQ);
+                IRQ_local_pipe(opp, i, n_IRQ, active, was_active);
                 src->last_cpu = i;
                 break;
             }
@@ -437,16 +475,25 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
     if (src->level) {
         /* level-sensitive irq */
         src->pending = level;
-        if (!level) {
-            src->ivpr &= ~IVPR_ACTIVITY_MASK;
-        }
+        openpic_update_irq(opp, n_IRQ);
     } else {
         /* edge-sensitive irq */
         if (level) {
             src->pending = 1;
+            openpic_update_irq(opp, n_IRQ);
+        }
+
+        if (src->output != OPENPIC_OUTPUT_INT) {
+            /* Edge-triggered interrupts shouldn't be used
+             * with non-INT delivery, but just in case,
+             * try to make it do something sane rather than
+             * cause an interrupt storm.  This is close to
+             * what you'd probably see happen in real hardware.
+             */
+            src->pending = 0;
+            openpic_update_irq(opp, n_IRQ);
         }
     }
-    openpic_update_irq(opp, n_IRQ);
 }
 
 static void openpic_reset(DeviceState *d)
@@ -934,6 +981,21 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
         break;
     case 0x80: /* CTPR */
         dst->ctpr = val & 0x0000000F;
+
+        DPRINTF("%s: set CPU %d ctpr to %d, raised %d servicing %d\n",
+                __func__, idx, dst->ctpr, dst->raised.priority,
+                dst->servicing.priority);
+
+        if (dst->raised.priority <= dst->ctpr) {
+            DPRINTF("%s: Lower OpenPIC INT output cpu %d due to ctpr\n",
+                    __func__, idx);
+            qemu_irq_lower(dst->irqs[OPENPIC_OUTPUT_INT]);
+        } else if (dst->raised.priority > dst->servicing.priority) {
+            DPRINTF("%s: Raise OpenPIC INT output cpu %d irq %d\n",
+                    __func__, idx, dst->raised.next);
+            qemu_irq_raise(dst->irqs[OPENPIC_OUTPUT_INT]);
+        }
+
         break;
     case 0x90: /* WHOAMI */
         /* Read-only register */
@@ -995,22 +1057,21 @@ static uint32_t openpic_iack(OpenPICState *opp, IRQDest *dst, int cpu)
     src = &opp->src[irq];
     if (!(src->ivpr & IVPR_ACTIVITY_MASK) ||
             !(IVPR_PRIORITY(src->ivpr) > dst->ctpr)) {
-        /* - Spurious level-sensitive IRQ
-         * - Priorities has been changed
-         *   and the pending IRQ isn't allowed anymore
-         */
-        src->ivpr &= ~IVPR_ACTIVITY_MASK;
+        fprintf(stderr, "%s: bad raised IRQ %d ctpr %d ivpr 0x%08x\n",
+                __func__, irq, dst->ctpr, src->ivpr);
+        openpic_update_irq(opp, irq);
         retval = opp->spve;
     } else {
         /* IRQ enter servicing state */
         IRQ_setbit(&dst->servicing, irq);
         retval = IVPR_VECTOR(opp, src->ivpr);
     }
-    IRQ_resetbit(&dst->raised, irq);
+
     if (!src->level) {
         /* edge-sensitive IRQ */
         src->ivpr &= ~IVPR_ACTIVITY_MASK;
         src->pending = 0;
+        IRQ_resetbit(&dst->raised, irq);
     }
 
     if ((irq >= opp->irq_ipi0) &&  (irq < (opp->irq_ipi0 + MAX_IPI))) {
@@ -1208,6 +1269,8 @@ static void openpic_save(QEMUFile* f, void *opaque)
         qemu_put_sbe32s(f, &opp->dst[i].ctpr);
         openpic_save_IRQ_queue(f, &opp->dst[i].raised);
         openpic_save_IRQ_queue(f, &opp->dst[i].servicing);
+        qemu_put_buffer(f, (uint8_t *)&opp->dst[i].outputs_active,
+                        sizeof(opp->dst[i].outputs_active));
     }
 
     for (i = 0; i < MAX_TMR; i++) {
@@ -1264,6 +1327,8 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
         qemu_get_sbe32s(f, &opp->dst[i].ctpr);
         openpic_load_IRQ_queue(f, &opp->dst[i].raised);
         openpic_load_IRQ_queue(f, &opp->dst[i].servicing);
+        qemu_get_buffer(f, (uint8_t *)&opp->dst[i].outputs_active,
+                        sizeof(opp->dst[i].outputs_active));
     }
 
     for (i = 0; i < MAX_TMR; i++) {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 0/4] openpic cleanups and fixes

[Alexander Graf]

On 04.01.2013, at 00:25, Scott Wood wrote:

> Respin of the patches from the 12/21 patchset that were not already applied.

Thanks, applied all to ppc-next.

Alex

> 
> Scott Wood (4):
>  openpic: fix sense and priority bits
>  openpic: IRQ_check: search the queue a word at a time
>  openpic: move IACK to its own function
>  openpic: fix CTPR and de-assertion of interrupts
> 
> hw/openpic.c |  365 ++++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 255 insertions(+), 110 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 

Re: [Qemu-devel] [PATCH v2 0/4] openpic cleanups and fixes

[Anthony Liguori]

Hi,

This is an automated message generated from the QEMU Patches.
Thank you for submitting this patch.  This patch no longer applies to qemu.git.

This may have occurred due to:
 
  1) Changes in mainline requiring your patch to be rebased and re-tested.

  2) Sending the mail using a tool other than git-send-email.  Please use
     git-send-email to send patches to QEMU.

  3) Basing this patch off of a branch that isn't tracking the QEMU
     master branch.  If that was done purposefully, please include the name
     of the tree in the subject line in the future to prevent this message.

     For instance: "[PATCH block-next 1/10] qcow3: add fancy new feature"

  4) You no longer wish for this patch to be applied to QEMU.  No additional
     action is required on your part.

Nacked-by: QEMU Patches <aliguori@us.ibm.com>

Below is the output from git-am:

    Applying: openpic: fix sense and priority bits
    fatal: sha1 information is lacking or useless (hw/openpic.c).
    Repository lacks necessary blobs to fall back on 3-way merge.
    Cannot fall back to three-way merge.
    Patch failed at 0001 openpic: fix sense and priority bits
    The copy of the patch that failed is found in:
       /home/aliguori/.patches/qemu-working/.git/rebase-apply/patch
    When you have resolved this problem run "git am --resolved".
    If you would prefer to skip this patch, instead run "git am --skip".
    To restore the original branch and stop patching run "git am --abort".