Message ID | 1475833331-26882-1-git-send-email-marcin.krzeminski@nokia.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 7 October 2016 at 10:42, <marcin.krzeminski@nokia.com> wrote: > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > 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 <marcin.krzeminski@nokia.com> > --- > 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
Hi Peter > -----Original Message----- > From: Peter Maydell [mailto:peter.maydell@linaro.org] > Sent: Thursday, October 13, 2016 6:59 PM > To: Krzeminski, Marcin (Nokia - PL/Wroclaw) > <marcin.krzeminski@nokia.com> > Cc: QEMU Developers <qemu-devel@nongnu.org>; qemu-arm <qemu- > arm@nongnu.org>; rfsw-patches@mlist.nokia.com > Subject: Re: [PATCH] nvic: allow to set pending status for not active > interrupts > > On 7 October 2016 at 10:42, <marcin.krzeminski@nokia.com> wrote: > > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > > > 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 <marcin.krzeminski@nokia.com> > > --- > > 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
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); }