diff mbox

[v2,12/16] hw/intc/arm_gic: Change behavior of EOIR writes

Message ID 1414707132-24588-13-git-send-email-greg.bellows@linaro.org
State New
Headers show

Commit Message

Greg Bellows Oct. 30, 2014, 10:12 p.m. UTC 
From: Fabian Aggeler <aggelerf@ethz.ch>

Grouping (GICv2) and Security Extensions change the behavior of EOIR
writes. Completing Group0 interrupts is only allowed from Secure state
and completing Group1 interrupts from Secure state is only allowed if
AckCtl bit is set.

Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>

---

v1 -> v2
- Fix issue with EOIR writes involving AckCtl.  AckCtl is ignored on EOIR
  group 1 interrupts when non-secure.  Group 1 interrupts are only ignored when
  secure and AckCTl is clear.
---
 hw/intc/arm_gic.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Peter Maydell April 14, 2015, 7:30 p.m. UTC | #1 
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 EOIR
> writes. Completing Group0 interrupts is only allowed from Secure state
> and completing Group1 interrupts from Secure state is only allowed if
> AckCtl bit is set.
>
> Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
>
> ---
>
> v1 -> v2
> - Fix issue with EOIR writes involving AckCtl.  AckCtl is ignored on EOIR
>   group 1 interrupts when non-secure.  Group 1 interrupts are only ignored when
>   secure and AckCTl is clear.
> ---
>  hw/intc/arm_gic.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 15fd660..2d83225 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -384,6 +384,21 @@ void gic_complete_irq(GICState *s, int cpu, int irq)
>              GIC_SET_PENDING(irq, cm);
>              update = 1;
>          }
> +    } else if ((s->revision >= 2 && !s->security_extn)
> +                 || (s->security_extn && !ns_access())) {
> +        /* Handle GICv2 without Security Extensions or GIC with Security
> +         * Extensions and a secure write.
> +         */
> +        if (!GIC_TEST_GROUP0(irq, cm) && !ns_access()
> +                && !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
> +            /* Unpredictable. We choose to ignore. */
> +            DPRINTF("EOI for Group1 interrupt %d ignored "
> +                    "(AckCtl disabled)\n", irq);
> +            return;
> +        }

For GICv2 without the security extns, EOIR accesses should behave
as if they were secure, so the call to ns_access() inside this
conditional is wrong. We probably need to disentangle the v1-vs-v2
differences here.

> +    } else if (s->security_extn && ns_access() && GIC_TEST_GROUP0(irq, cm)) {
> +        DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
> +        return;
>      }

-- PMM
diff mbox

Patch

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 15fd660..2d83225 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -384,6 +384,21 @@  void gic_complete_irq(GICState *s, int cpu, int irq)
             GIC_SET_PENDING(irq, cm);
             update = 1;
         }
+    } else if ((s->revision >= 2 && !s->security_extn)
+                 || (s->security_extn && !ns_access())) {
+        /* Handle GICv2 without Security Extensions or GIC with Security
+         * Extensions and a secure write.
+         */
+        if (!GIC_TEST_GROUP0(irq, cm) && !ns_access()
+                && !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
+            /* Unpredictable. We choose to ignore. */
+            DPRINTF("EOI for Group1 interrupt %d ignored "
+                    "(AckCtl disabled)\n", irq);
+            return;
+        }
+    } else if (s->security_extn && ns_access() && GIC_TEST_GROUP0(irq, cm)) {
+        DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
+        return;
     }
 
     if (irq != s->running_irq[cpu]) {