Hi Peter,
Can you help me understand arm_vgic.c, specifically, see the quoted code
below and my question:
> /* Process a change in an external IRQ input. */
> static void gic_set_irq(void *opaque, int irq, int level)
> {
> /* Meaning of the 'irq' parameter:
> * [0..N-1] : external interrupts
> * [N..N+31] : PPI (internal) interrupts for CPU 0
> * [N+32..N+63] : PPI (internal interrupts for CPU 1
> * ...
> */
> GICState *s = (GICState *)opaque;
> int cm, target;
> if (irq < (s->num_irq - GIC_INTERNAL)) {
> /* The first external input line is internal interrupt 32. */
> cm = ALL_CPU_MASK;
> irq += GIC_INTERNAL;
> target = GIC_TARGET(irq);
> } else {
> int cpu;
> irq -= (s->num_irq - GIC_INTERNAL);
> cpu = irq / GIC_INTERNAL;
> irq %= GIC_INTERNAL;
> cm = 1 << cpu;
> target = cm;
> }
>
> if (level == GIC_TEST_LEVEL(irq, cm)) {
> return;
> }
>
> if (level) {
> GIC_SET_LEVEL(irq, cm);
If this is an edge-triggered interrupt, is the function called with
level==0 right after? I assume so.
> if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) {
The way I read the code is that gic_irq_state->level means the state of
the input line to the distributor, and gic_irq_state->pending is
what GICD_ISPENDRn tells you: that the interrupt is pending on at least
one CPU.
So the question is, why does an edge-triggered interrupt override the
disabled state?
A possible answer could be that because disabling the interrupt doesn't
mean that it cannot become pending (only that it doesn't get forwarded to
the CPU) we need to remember this state for an edge-triggered interrupt,
because we can't re-sample the line.
But, looking at the emulation code for GICD_IxPENDRn contradicts this,
because this returns a raw view of pending IRQs, and the docs clearly
state that reads should only return 1 for IRQs that are actually pending
on one or more CPU interfaces (I take this to mean enabled, and thus
forwarded to the CPU interface), so reading this registers would give a
false impression of an edge-triggered interrupt, which is disabled, but
not pending on any CPU interfaces.
If I read the specs wrong, and pending simply is:
- either an edge-triggered line was raised
- a level triggered line is held high (assuming active-HIGH)
and GICD_IxPENDRn returns a raw view of the above, well then I don't
understand the second operand to the if statement, because it shouldn't
matter if the interrupt is enabled or disabled, it's just the raw
hardware state.
> DPRINTF("Set %d pending mask %x\n", irq, target);
> GIC_SET_PENDING(irq, target);
> }
> } else {
> GIC_CLEAR_LEVEL(irq, cm);
> }
> gic_update(s);
> }
There's an extra technicality based on my reading of the GIC specs that
make me think we're missing a bit of state in the kernel. If we look
at the handling of GICD_ICPENDRn, then we simply clear the state of the
irq, but the docs actually specify that it should only make the
interrupt inactive if the only reason it became pending was a write to
GICD_ISPENDRn (implying that the hardware level-triggered line was not
high).
Not sure it's worth fixing - I don't think Linux is writing to these
registers, but I haven't verified it. Marc?
Thanks,
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
