| Message ID | 1414707132-24588-14-git-send-email-greg.bellows@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 30 October 2014 at 22:12, Greg Bellows <greg.bellows@linaro.org> wrote: > From: Fabian Aggeler <aggelerf@ethz.ch> > > Grouping (GICv2) and Security Extensions change the behavior of IAR > reads. Acknowledging Group0 interrupts is only allowed from Secure > state and acknowledging Group1 interrupts from Secure state is only > allowed if AckCtl bit is set. Subject says "IAR writes" but it means "IAR reads". > > Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch> > > --- > > v1 -> v2 > - Fix issue in gic_acknowledge_irq() where the GICC_CTLR_S_ACK_CTL flag is > applied without first checking whether the read is secure or non-secure. > Secure reads of IAR when AckCtl is 0 return a spurious ID of 1022, but > non-secure ignores the flag. > --- > hw/intc/arm_gic.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 2d83225..7eb72df 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -190,11 +190,36 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu) > int ret, irq, src; > int cm = 1 << cpu; > irq = s->current_pending[cpu]; > + bool isGrp0; > if (irq == 1023 > || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) { > DPRINTF("ACK no pending IRQ\n"); > return 1023; > } > + > + if (s->revision >= 2 || s->security_extn) { > + isGrp0 = GIC_TEST_GROUP0(irq, (1 << cpu)); > + if ((isGrp0 && (!s->enabled_grp[0] > + || !(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0))) > + || (!isGrp0 && (!s->enabled_grp[1] > + || !(s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1)))) { > + return 1023; > + } > + > + if ((s->revision >= 2 && !s->security_extn) > + || (s->security_extn && !ns_access())) { > + if (!isGrp0 && !ns_access() && > + !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) { > + DPRINTF("Read of IAR ignored for Group1 interrupt %d " > + "(AckCtl disabled)\n", irq); > + return 1022; > + } > + } else if (s->security_extn && ns_access() && isGrp0) { > + DPRINTF("Non-secure read of IAR ignored for Group0 interrupt %d\n", > + irq); > + return 1023; > + } > + } This doesn't quite line up with the pseudocode in the GIC spec. It's probably going to be easier to read with some utility functions for 'grouping enabled' etc. > s->last_active[irq][cpu] = s->running_irq[cpu]; > > if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { > -- > 1.8.3.2 > -- PMM
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 2d83225..7eb72df 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -190,11 +190,36 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu) int ret, irq, src; int cm = 1 << cpu; irq = s->current_pending[cpu]; + bool isGrp0; if (irq == 1023 || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) { DPRINTF("ACK no pending IRQ\n"); return 1023; } + + if (s->revision >= 2 || s->security_extn) { + isGrp0 = GIC_TEST_GROUP0(irq, (1 << cpu)); + if ((isGrp0 && (!s->enabled_grp[0] + || !(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0))) + || (!isGrp0 && (!s->enabled_grp[1] + || !(s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1)))) { + return 1023; + } + + if ((s->revision >= 2 && !s->security_extn) + || (s->security_extn && !ns_access())) { + if (!isGrp0 && !ns_access() && + !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) { + DPRINTF("Read of IAR ignored for Group1 interrupt %d " + "(AckCtl disabled)\n", irq); + return 1022; + } + } else if (s->security_extn && ns_access() && isGrp0) { + DPRINTF("Non-secure read of IAR ignored for Group0 interrupt %d\n", + irq); + return 1023; + } + } s->last_active[irq][cpu] = s->running_irq[cpu]; if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {