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

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

Hi Peter

> -----Original Message-----
> From: Peter Maydell [mailto:address@hidden
> Sent: Thursday, October 13, 2016 6:59 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <address@hidden>
> Cc: QEMU Developers <address@hidden>; qemu-arm <qemu-
> address@hidden>; address@hidden
> Subject: Re: [PATCH] nvic: allow to set pending status for not active
> interrupts
> 
> 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.
>
Sure.

> 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);
>         }
>     }
It seem thet level triggered interrupt could be removed from fi for NVIC.
I do not have guest to model that, but I will at least check if this
do not brake anything.

Thanks,
Marcin 
> 
> thanks
> -- PMM