[Qemu-devel] [PATCH v4 07/17] hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked

[Peter Maydell]

From: Fabian Aggeler <address@hidden>

This register is banked in GICs with Security Extensions. Storing the
non-secure copy of BPR in the abpr, which is an alias to the non-secure
copy for secure access. ABPR itself is only accessible from secure state
if the GIC implements Security Extensions.

Signed-off-by: Fabian Aggeler <address@hidden>
Signed-off-by: Greg Bellows <address@hidden>
Message-id: address@hidden
[PMM: rewrote to fix style issues and correct handling of GICv2
 without security extensions]
Signed-off-by: Peter Maydell <address@hidden>
---
 hw/intc/arm_gic.c                | 31 ++++++++++++++++++++++++++-----
 include/hw/intc/arm_gic_common.h | 11 ++++++++---
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 4f13ff2..e6ad8de 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -762,7 +762,12 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int 
offset,
         *data = s->priority_mask[cpu];
         break;
     case 0x08: /* Binary Point */
-        *data = s->bpr[cpu];
+        if (s->security_extn && !attrs.secure) {
+            /* BPR is banked. Non-secure copy stored in ABPR. */
+            *data = s->abpr[cpu];
+        } else {
+            *data = s->bpr[cpu];
+        }
         break;
     case 0x0c: /* Acknowledge */
         *data = gic_acknowledge_irq(s, cpu);
@@ -774,7 +779,16 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int 
offset,
         *data = s->current_pending[cpu];
         break;
     case 0x1c: /* Aliased Binary Point */
-        *data = s->abpr[cpu];
+        /* GIC v2, no security: ABPR
+         * GIC v1, no security: not implemented (RAZ/WI)
+         * With security extensions, secure access: ABPR (alias of NS BPR)
+         * With security extensions, nonsecure access: RAZ/WI
+         */
+        if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
+            *data = 0;
+        } else {
+            *data = s->abpr[cpu];
+        }
         break;
     case 0xd0: case 0xd4: case 0xd8: case 0xdc:
         *data = s->apr[(offset - 0xd0) / 4][cpu];
@@ -799,14 +813,21 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, 
int offset,
         s->priority_mask[cpu] = (value & 0xff);
         break;
     case 0x08: /* Binary Point */
-        s->bpr[cpu] = (value & 0x7);
+        if (s->security_extn && !attrs.secure) {
+            s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR);
+        } else {
+            s->bpr[cpu] = MAX(value & 0x7, GIC_MIN_BPR);
+        }
         break;
     case 0x10: /* End Of Interrupt */
         gic_complete_irq(s, cpu, value & 0x3ff);
         return MEMTX_OK;
     case 0x1c: /* Aliased Binary Point */
-        if (s->revision >= 2) {
-            s->abpr[cpu] = (value & 0x7);
+        if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
+            /* unimplemented, or NS access: RAZ/WI */
+            return MEMTX_OK;
+        } else {
+            s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR);
         }
         break;
     case 0xd0: case 0xd4: case 0xd8: case 0xdc:
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index d5d3877..261402f 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -34,6 +34,9 @@
 #define MAX_NR_GROUP_PRIO 128
 #define GIC_NR_APRS (MAX_NR_GROUP_PRIO / 32)
 
+#define GIC_MIN_BPR 0
+#define GIC_MIN_ABPR (GIC_MIN_BPR + 1)
+
 typedef struct gic_irq_state {
     /* The enable bits are only banked for per-cpu interrupts.  */
     uint8_t enabled;
@@ -76,9 +79,11 @@ typedef struct GICState {
     uint16_t running_priority[GIC_NCPU];
     uint16_t current_pending[GIC_NCPU];
 
-    /* We present the GICv2 without security extensions to a guest and
-     * therefore the guest can configure the GICC_CTLR to configure group 1
-     * binary point in the abpr.
+    /* If we present the GICv2 without security extensions to a guest,
+     * the guest can configure the GICC_CTLR to configure group 1 binary point
+     * in the abpr.
+     * For a GIC with Security Extensions we use use bpr for the
+     * secure copy and abpr as storage for the non-secure copy of the register.
      */
     uint8_t  bpr[GIC_NCPU];
     uint8_t  abpr[GIC_NCPU];
-- 
1.9.1