[Qemu-devel] [PULL 10/19] hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked

[Peter Maydell]

From: Fabian Aggeler <address@hidden>

ICCICR/GICC_CTLR is banked in GICv1 implementations with Security
Extensions or in GICv2 in independent from Security Extensions.
This makes it possible to enable forwarding of interrupts from
the CPU interfaces to the connected processors for Group0 and Group1.

We also allow to set additional bits like AckCtl and FIQEn by changing
the type from bool to uint32. Since the field does not only store the
enable bit anymore and since we are touching the vmstate, we use the
opportunity to rename the field to cpu_ctlr.

Signed-off-by: Fabian Aggeler <address@hidden>
Signed-off-by: Greg Bellows <address@hidden>
Reviewed-by: Edgar E. Iglesias <address@hidden>
Signed-off-by: Peter Maydell <address@hidden>
Message-id: address@hidden
Message-id: address@hidden
[PMM: rewrote to store state in a single uint32_t rather than
 keeping the NS and S banked variants separate; this considerably
 simplifies the get/set functions]
Signed-off-by: Peter Maydell <address@hidden>
---
 hw/intc/arm_gic.c                | 51 ++++++++++++++++++++++++++++++++++++----
 hw/intc/arm_gic_common.c         |  8 +++----
 hw/intc/arm_gic_kvm.c            |  8 +++----
 hw/intc/armv7m_nvic.c            |  2 +-
 hw/intc/gic_internal.h           | 16 +++++++++++++
 include/hw/intc/arm_gic_common.h |  5 +++-
 6 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index e6ad8de..4aaaac2 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -68,7 +68,7 @@ void gic_update(GICState *s)
         cm = 1 << cpu;
         s->current_pending[cpu] = 1023;
         if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1))
-            || !s->cpu_enabled[cpu]) {
+            || !(s->cpu_ctlr[cpu] & (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) {
             qemu_irq_lower(s->parent_irq[cpu]);
             return;
         }
@@ -242,6 +242,50 @@ void gic_set_priority(GICState *s, int cpu, int irq, 
uint8_t val)
     }
 }
 
+static uint32_t gic_get_cpu_control(GICState *s, int cpu, MemTxAttrs attrs)
+{
+    uint32_t ret = s->cpu_ctlr[cpu];
+
+    if (s->security_extn && !attrs.secure) {
+        /* Construct the NS banked view of GICC_CTLR from the correct
+         * bits of the S banked view. We don't need to move the bypass
+         * control bits because we don't implement that (IMPDEF) part
+         * of the GIC architecture.
+         */
+        ret = (ret & (GICC_CTLR_EN_GRP1 | GICC_CTLR_EOIMODE_NS)) >> 1;
+    }
+    return ret;
+}
+
+static void gic_set_cpu_control(GICState *s, int cpu, uint32_t value,
+                                MemTxAttrs attrs)
+{
+    uint32_t mask;
+
+    if (s->security_extn && !attrs.secure) {
+        /* The NS view can only write certain bits in the register;
+         * the rest are unchanged
+         */
+        mask = GICC_CTLR_EN_GRP1;
+        if (s->revision == 2) {
+            mask |= GICC_CTLR_EOIMODE_NS;
+        }
+        s->cpu_ctlr[cpu] &= ~mask;
+        s->cpu_ctlr[cpu] |= (value << 1) & mask;
+    } else {
+        if (s->revision == 2) {
+            mask = s->security_extn ? GICC_CTLR_V2_S_MASK : GICC_CTLR_V2_MASK;
+        } else {
+            mask = s->security_extn ? GICC_CTLR_V1_S_MASK : GICC_CTLR_V1_MASK;
+        }
+        s->cpu_ctlr[cpu] = value & mask;
+    }
+    DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, "
+            "Group1 Interrupts %sabled\n", cpu,
+            (s->cpu_ctlr[cpu] & GICC_CTLR_EN_GRP0) ? "En" : "Dis",
+            (s->cpu_ctlr[cpu] & GICC_CTLR_EN_GRP1) ? "En" : "Dis");
+}
+
 void gic_complete_irq(GICState *s, int cpu, int irq)
 {
     int update = 0;
@@ -756,7 +800,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int 
offset,
 {
     switch (offset) {
     case 0x00: /* Control */
-        *data = s->cpu_enabled[cpu];
+        *data = gic_get_cpu_control(s, cpu, attrs);
         break;
     case 0x04: /* Priority mask */
         *data = s->priority_mask[cpu];
@@ -806,8 +850,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int 
offset,
 {
     switch (offset) {
     case 0x00: /* Control */
-        s->cpu_enabled[cpu] = (value & 1);
-        DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis");
+        gic_set_cpu_control(s, cpu, value, attrs);
         break;
     case 0x04: /* Priority mask */
         s->priority_mask[cpu] = (value & 0xff);
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index bef76fc..044ad66 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -59,13 +59,13 @@ static const VMStateDescription vmstate_gic_irq_state = {
 
 static const VMStateDescription vmstate_gic = {
     .name = "arm_gic",
-    .version_id = 9,
-    .minimum_version_id = 9,
+    .version_id = 10,
+    .minimum_version_id = 10,
     .pre_save = gic_pre_save,
     .post_load = gic_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(ctlr, GICState),
-        VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU),
+        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU),
         VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
                              vmstate_gic_irq_state, gic_irq_state),
         VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
@@ -134,7 +134,7 @@ static void arm_gic_common_reset(DeviceState *dev)
         s->current_pending[i] = 1023;
         s->running_irq[i] = 1023;
         s->running_priority[i] = 0x100;
-        s->cpu_enabled[i] = false;
+        s->cpu_ctlr[i] = 0;
     }
     for (i = 0; i < GIC_NR_SGIS; i++) {
         GIC_SET_ENABLED(i, ALL_CPU_MASK);
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 3591ca7..c5a2f81 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -414,8 +414,8 @@ static void kvm_arm_gic_put(GICState *s)
      */
 
     for (cpu = 0; cpu < s->num_cpu; cpu++) {
-        /* s->cpu_enabled[cpu] -> GICC_CTLR */
-        reg = s->cpu_enabled[cpu];
+        /* s->cpu_ctlr[cpu] -> GICC_CTLR */
+        reg = s->cpu_ctlr[cpu];
         kvm_gicc_access(s, 0x00, cpu, &reg, true);
 
         /* s->priority_mask[cpu] -> GICC_PMR */
@@ -506,9 +506,9 @@ static void kvm_arm_gic_get(GICState *s)
      */
 
     for (cpu = 0; cpu < s->num_cpu; cpu++) {
-        /* GICC_CTLR -> s->cpu_enabled[cpu] */
+        /* GICC_CTLR -> s->cpu_ctlr[cpu] */
         kvm_gicc_access(s, 0x00, cpu, &reg, false);
-        s->cpu_enabled[cpu] = (reg & 1);
+        s->cpu_ctlr[cpu] = reg;
 
         /* GICC_PMR -> s->priority_mask[cpu] */
         kvm_gicc_access(s, 0x04, cpu, &reg, false);
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 16f0dca2..a6567a3 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -474,7 +474,7 @@ static void armv7m_nvic_reset(DeviceState *dev)
      * as enabled by default, and with a priority mask which allows
      * all interrupts through.
      */
-    s->gic.cpu_enabled[0] = true;
+    s->gic.cpu_ctlr[0] = GICC_CTLR_EN_GRP0;
     s->gic.priority_mask[0] = 0x100;
     /* The NVIC as a whole is always enabled. */
     s->gic.ctlr = 1;
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 3b4b3fb..81c764c 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -57,6 +57,22 @@
 #define GICD_CTLR_EN_GRP0 (1U << 0)
 #define GICD_CTLR_EN_GRP1 (1U << 1)
 
+#define GICC_CTLR_EN_GRP0    (1U << 0)
+#define GICC_CTLR_EN_GRP1    (1U << 1)
+#define GICC_CTLR_ACK_CTL    (1U << 2)
+#define GICC_CTLR_FIQ_EN     (1U << 3)
+#define GICC_CTLR_CBPR       (1U << 4) /* GICv1: SBPR */
+#define GICC_CTLR_EOIMODE    (1U << 9)
+#define GICC_CTLR_EOIMODE_NS (1U << 10)
+
+/* Valid bits for GICC_CTLR for GICv1, v1 with security extensions,
+ * GICv2 and GICv2 with security extensions:
+ */
+#define GICC_CTLR_V1_MASK    0x1
+#define GICC_CTLR_V1_S_MASK  0x1f
+#define GICC_CTLR_V2_MASK    0x21f
+#define GICC_CTLR_V2_S_MASK  0x61f
+
 /* The special cases for the revision property: */
 #define REV_11MPCORE 0
 #define REV_NVIC 0xffffffff
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 261402f..899db3d 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -59,7 +59,10 @@ typedef struct GICState {
      * of this register is just an alias of bit 1 of the S banked version.
      */
     uint32_t ctlr;
-    bool cpu_enabled[GIC_NCPU];
+    /* GICC_CTLR; again, the NS banked version is just aliases of bits of
+     * the S banked register, so our state only needs to store the S version.
+     */
+    uint32_t cpu_ctlr[GIC_NCPU];
 
     gic_irq_state irq_state[GIC_MAXIRQ];
     uint8_t irq_target[GIC_MAXIRQ];
-- 
1.9.1