Re: [Qemu-devel] 答复: report a suspect bug about arm gic

[Christoffer Dall]

On Wed, Apr 16, 2014 at 02:46:52AM +0000, zhuxiaodong wrote:
> I don't doubt about the algorithm of level interrupts. 
> The problem may come from the level value tested by GIC_TEST_LEVEL(irq, cm).
> It is set in gic_set_irq_generic():
> > 115 static void gic_set_irq_generic(GICState *s, int irq, int level,
> > 116                                 int cm, int target)
> > 117 {
> > 118     if (level) {
> > 119         GIC_SET_LEVEL(irq, cm);
> > 120         DPRINTF("Set %d pending mask %x\n", irq, target);
> > 121         if (GIC_TEST_EDGE_TRIGGER(irq)) {
> > 122             GIC_SET_PENDING(irq, target);
> > 123         }
> > 124     } else {
> > 125         GIC_CLEAR_LEVEL(irq, cm);
> > 126     }
> > 127 }
> At line 119 we can see that it is always set to cm.

Actually the bits of the cm is or'ed onto the level bitmask.

> 
> And gic_set_irq_generic is called by gic_set_irq (void *opaque, int irq, int 
> level):
> 132 static void gic_set_irq(void *opaque, int irq, int level)
> 133 {
> 134     /* Meaning of the 'irq' parameter:
> 135      *  [0..N-1] : external interrupts
> 136      *  [N..N+31] : PPI (internal) interrupts for CPU 0
> 137      *  [N+32..N+63] : PPI (internal interrupts for CPU 1
> 138      *  ...
> 139      */
> 140     GICState *s = (GICState *)opaque;
> 141     int cm, target;
> 142     if (irq < (s->num_irq - GIC_INTERNAL)) {
> 143         /* The first external input line is internal interrupt 32.  */
> 144         cm = ALL_CPU_MASK;
> 145         irq += GIC_INTERNAL;
> 146         target = GIC_TARGET(irq);
> At line 144 we can see that cm is always set to ALL_CPU_MASK if irq is a SPI. 
> That means GIC_TEST_LEVEL can success for any cpu!
> 
> So when GIC_TEST_LEVEL(irq,cm) is invoked by gic_update, at the first loop 
> for cpu 0, the irq will be choosen ahead of time even if cpu 0 is not the 
> target cpu.
> 
> Please consider about this case.

I think what you're trying to say is that gic_set_irq_...() does not
respect settings in the GICD_ITARGETSR (which is what GIC_TARGET(irq)
checks for).  In fact I don't think it should, but we should check the
GICD_ITARGETSR value in gic_update to comply with the architecture
specification that states that changes to GICD_ITARGETSR have an effect
on pending interrupts (but not on active, or on active and pending
interrupts until the status is cleared on the latter).

I haven't checked the 11mpcore or the nvic docs yet on how the behavior
should be for those.

-Christoffer