Patchwork [09/15] openpic: always call IRQ_check from IRQ_get_next

login
register
mail settings
Submitter Scott Wood
Date Dec. 22, 2012, 2:15 a.m.
Message ID <1356142552-13453-10-git-send-email-scottwood@freescale.com>
Download mbox | patch
Permalink /patch/207909/
State New
Headers show

Comments

Scott Wood - Dec. 22, 2012, 2:15 a.m.
Previously the code relied on the queue's "next" field getting
set to -1 sometime between an update to the bitmap, and the next
call to IRQ_get_next.  Sometimes this happened after the update.
Sometimes it happened before the check.  Sometimes it didn't happen
at all.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 hw/openpic.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
Alexander Graf - Jan. 3, 2013, 6:42 p.m.
On 22.12.2012, at 03:15, Scott Wood wrote:

> Previously the code relied on the queue's "next" field getting
> set to -1 sometime between an update to the bitmap, and the next
> call to IRQ_get_next.  Sometimes this happened after the update.
> Sometimes it happened before the check.  Sometimes it didn't happen
> at all.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>

Have you verified that we don't run the check too often then? It's quite costly, no?

Applied nevertheless to ppc-next.


Alex
Scott Wood - Jan. 3, 2013, 8:09 p.m.
On 01/03/2013 12:42:09 PM, Alexander Graf wrote:
> 
> On 22.12.2012, at 03:15, Scott Wood wrote:
> 
> > Previously the code relied on the queue's "next" field getting
> > set to -1 sometime between an update to the bitmap, and the next
> > call to IRQ_get_next.  Sometimes this happened after the update.
> > Sometimes it happened before the check.  Sometimes it didn't happen
> > at all.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> 
> Have you verified that we don't run the check too often then? It's  
> quite costly, no?

Correctness takes precedence over speed, as does  
readability/maintainability if the difference is minor.  In any case,  
the check gets faster later in the patchset.

-Scott

Patch

diff --git a/hw/openpic.c b/hw/openpic.c
index 268f312..e2e7079 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -316,10 +316,8 @@  out:
 
 static int IRQ_get_next(OpenPICState *opp, IRQ_queue_t *q)
 {
-    if (q->next == -1) {
-        /* XXX: optimize */
-        IRQ_check(opp, q);
-    }
+    /* XXX: optimize */
+    IRQ_check(opp, q);
 
     return q->next;
 }
@@ -366,7 +364,7 @@  static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ)
                 __func__, n_IRQ, dst->raised.next, n_CPU);
         return;
     }
-    IRQ_get_next(opp, &dst->raised);
+    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",
@@ -937,7 +935,6 @@  static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
         DPRINTF("EOI\n");
         s_IRQ = IRQ_get_next(opp, &dst->servicing);
         IRQ_resetbit(&dst->servicing, s_IRQ);
-        dst->servicing.next = -1;
         /* Set up next servicing IRQ */
         s_IRQ = IRQ_get_next(opp, &dst->servicing);
         /* Check queued interrupts. */
@@ -1013,7 +1010,6 @@  static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
                 retval = IVPR_VECTOR(opp, src->ivpr);
             }
             IRQ_resetbit(&dst->raised, n_IRQ);
-            dst->raised.next = -1;
             if (!src->level) {
                 /* edge-sensitive IRQ */
                 src->ivpr &= ~IVPR_ACTIVITY_MASK;