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

[Peter Maydell]

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