Re: [Qemu-arm] [PATCH] nvic: allow to set pending status for not active interrupts

[Peter Maydell]

On 7 October 2016 at 10:42,  <address@hidden> wrote:
> From: Marcin Krzeminski <address@hidden>
>
> According to ARM DUI 0552A 4.2.10. NVIC set pending status
> also for disabled interrupts. This patch adds possibility
> to emulate this in Qemu.
>
> Signed-off-by: Marcin Krzeminski <address@hidden>
> ---
>  hw/intc/arm_gic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index b30cc91..85be6e4 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -147,7 +147,8 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, 
> int level,
>  {
>      if (level) {
>          GIC_SET_LEVEL(irq, cm);
> -        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) {
> +        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)
> +                || (!GIC_TEST_ACTIVE(irq, cm) &&  s->revision == REV_NVIC)) {
>              DPRINTF("Set %d pending mask %x\n", irq, target);
>              GIC_SET_PENDING(irq, target);
>          }
> --

Thanks for this patch. I agree that the current behaviour isn't
correct.

I think it would be cleaner to define a new gic_set_irq_nvic() which
has the NVIC specific behaviour, rather than sticking an "if this
is really an NVIC" check into this function.

You probably also want to check whether the logic for re-pending
level triggered interrupts in gic_complete_irq() also needs a
similar change:

    if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
        /* Mark level triggered interrupts as pending if they are still
           raised.  */
        if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
            && GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
            DPRINTF("Set %d pending mask %x\n", irq, cm);
            GIC_SET_PENDING(irq, cm);
        }
    }

thanks
-- PMM