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

[zhuxiaodong]

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.

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.
Thanks

Xiaodong Zhu

-----邮件原件-----
发件人: Peter Maydell [mailto:address@hidden 
发送时间: 2014年4月15日 19:12
收件人: zhuxiaodong
抄送: address@hidden; Christoffer Dall
主题: Re: [Qemu-devel] report a suspect bug about arm gic

On 15 April 2014 02:33, zhuxiaodong <address@hidden> wrote:
>         I am a user of qemu. I found that in qemu2.0.0-rc0 the gic 
> model was updated. However, it seems loss ability to bind irqs to any 
> specified core when the board includes multiple cortex-a9 cores. The 
> problematic codes maybe locate at hw/intc/arm_gic.c:

CCing Christoffer who dealt with the GIC most recently.

> gic_test_pending checks if the pending bits of an irq are set. I think 
> the pending bits reflect core bindings according to the func 
> “gic_set_irq”. But in the following codes:
>
>
> 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 }

> if level trigger the core target parameter of the irq is ignored and 
> its pending bits will not be set. So how does the core target 
> parameter make its affect?

I think this should be OK, because gic_test_pending() doesn't only test the 
pending latch value:

        return (s->irq_state[irq].pending & cm) ||
            (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));

If the irq is level triggered then the return value will be from 
GIC_TEST_LEVEL(irq, cm).

This corresponds to the hardware behaviour described by the circuit diagram 
Figure 4.10 at the bottom of Section 4.3.8 ("Interrupt Clear-Pending Registers, 
GICD_ICPENDRn") in the
GICv2 architecture specification. In QEMU terms, the
s->irq_state[irq].pending bits set by GIC_SET_PENDING and
tested by GIC_CLEAR_PENDING correspond to the state of the flipflop in that 
diagram. The gic_test_pending() function corresponds to the 
status_includes_pending output in that diagram.

So we should retain the fact that the level triggered interrupt is still 
asserted on a particular core via the
GIC_SET_LEVEL() call, and then pull it back out using GIC_TEST_LEVEL().

thanks
-- PMM