Re: [Qemu-arm] [v2] nvic: set pending status for not active interrupts

[Peter Maydell]

On 31 October 2016 at 09:11, Krzeminski, Marcin (Nokia - PL/Wroclaw)
<address@hidden> wrote:
> Peter,
>
>> -----Original Message-----
>> From: Peter Maydell [mailto:address@hidden
>> > +static void gic_set_irq_nvic(GICState *s, int irq, int level,
>> > +                                 int cm, int target) {
>> > +    if (level) {
>> > +        GIC_SET_LEVEL(irq, cm);
>> > +        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)
>> > +                || !GIC_TEST_ACTIVE(irq, cm)) {
>> > +            DPRINTF("Set %d pending mask %x\n", irq, target);
>> > +            GIC_SET_PENDING(irq, target);
>> > +        }
>>
>> Why is GIC_TEST_ENABLED() in this condition, when the idea is that we don't
>> care about the enabled status for whether we can pend the interrupt?
>>
>> I think this should actually just always do
>>    GIC_SET_PENDING(irq, target)
>> whenever level is high
>>
>> because:
>>  (1) pending status can be set for disabled interrupts
>>  (2) edge-triggered IRQs definitely always re-pend on rising edge
>>  (3) I think level-triggered IRQs do too (it's a bit
>>      less clear in the docs, but DUI0552A 4.2.9 says we pend on
>>      a rising edge and doesn't say that applies only to edge
>>      triggered IRQs)
>>
>> I don't have any real hardware to hand to test against, though.
>>
> Yes, it works exactly as you're saying (checked on HW), if level is still
> high after exit interrupt handler, interrupt is re-pend.

"after exiting interrupt handler" is the wrong condition to check.
You need to:
 * cause the interrupt line to be set
 * enter the interrupt handler as a result (int becomes active)
 * cause the interrupt line to be lowered (while in the handler)
 * cause the interrupt line to be set again (still in the handler)
 * check the interrupt pending state (still in the handler)

If you only check the pending state after exiting the handler,
then you can't tell whether it was pended on the rising edge
while active, or for the "re-pend if high when we leave the
interrupt handler". We know the hardware does the latter, it's
the former we're unsure about.

thanks
-- PMM