Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150512' into staging

target-arm queue:
 * Support TZ and grouping in the GIC
 * hw/sd: sd_reset cleanup
 * armv7m_nvic: fix bug in systick device

# gpg: Signature made Tue May 12 12:02:26 2015 BST using RSA key ID 14360CDE
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>"

* remotes/pmaydell/tags/pull-target-arm-20150512:
  hw/arm/highbank.c: Wire FIQ between CPU <> GIC
  hw/arm/vexpress.c: Wire FIQ between CPU <> GIC
  hw/arm/virt.c: Wire FIQ between CPU <> GIC
  hw/intc/arm_gic: Add grouping support to gic_update()
  hw/intc/arm_gic: Change behavior of IAR writes
  hw/intc/arm_gic: Change behavior of EOIR writes
  hw/intc/arm_gic: Handle grouping for GICC_HPPIR
  hw/intc/arm_gic: Restrict priority view
  hw/intc/arm_gic: Implement Non-secure view of RPR
  hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked
  hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked
  hw/intc/arm_gic: Make ICDDCR/GICD_CTLR banked
  hw/intc/arm_gic_kvm.c: Save and restore GICD_IGROUPRn state
  hw/intc/arm_gic: Add Interrupt Group Registers
  hw/intc/arm_gic: Switch to read/write callbacks with tx attributes
  hw/intc/arm_gic: Add Security Extensions property
  hw/intc/arm_gic: Create outbound FIQ lines
  hw/sd: Don't pass BlockBackend to sd_reset()
  armv7m_nvic: systick: Reload the RELOAD value and count down only if ENABLE bit is set

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

1 files changed, 385 insertions, 84 deletions

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a04c822832..c1d2e704ec 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -45,6 +45,14 @@ static inline int gic_get_current_cpu(GICState *s)
     return 0;
 }
 
+/* Return true if this GIC config has interrupt groups, which is
+ * true if we're a GICv2, or a GICv1 with the security extensions.
+ */
+static inline bool gic_has_groups(GICState *s)
+{
+    return s->revision == 2 || s->security_extn;
+}
+
 /* TODO: Many places that call this routine could be optimized.  */
 /* Update interrupt status after enabled or pending bits have been changed.  */
 void gic_update(GICState *s)
@@ -52,15 +60,17 @@ void gic_update(GICState *s)
     int best_irq;
     int best_prio;
     int irq;
-    int level;
+    int irq_level, fiq_level;
     int cpu;
     int cm;
 
     for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
         cm = 1 << cpu;
         s->current_pending[cpu] = 1023;
-        if (!s->enabled || !s->cpu_enabled[cpu]) {
+        if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1))
+            || !(s->cpu_ctlr[cpu] & (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) {
             qemu_irq_lower(s->parent_irq[cpu]);
+            qemu_irq_lower(s->parent_fiq[cpu]);
             return;
         }
         best_prio = 0x100;
@@ -74,15 +84,31 @@ void gic_update(GICState *s)
                 }
             }
         }
-        level = 0;
+
+        irq_level = fiq_level = 0;
+
         if (best_prio < s->priority_mask[cpu]) {
             s->current_pending[cpu] = best_irq;
             if (best_prio < s->running_priority[cpu]) {
-                DPRINTF("Raised pending IRQ %d (cpu %d)\n", best_irq, cpu);
-                level = 1;
+                int group = GIC_TEST_GROUP(best_irq, cm);
+
+                if (extract32(s->ctlr, group, 1) &&
+                    extract32(s->cpu_ctlr[cpu], group, 1)) {
+                    if (group == 0 && s->cpu_ctlr[cpu] & GICC_CTLR_FIQ_EN) {
+                        DPRINTF("Raised pending FIQ %d (cpu %d)\n",
+                                best_irq, cpu);
+                        fiq_level = 1;
+                    } else {
+                        DPRINTF("Raised pending IRQ %d (cpu %d)\n",
+                                best_irq, cpu);
+                        irq_level = 1;
+                    }
+                }
             }
         }
-        qemu_set_irq(s->parent_irq[cpu], level);
+
+        qemu_set_irq(s->parent_irq[cpu], irq_level);
+        qemu_set_irq(s->parent_fiq[cpu], fiq_level);
     }
 }
 
@@ -167,6 +193,32 @@ static void gic_set_irq(void *opaque, int irq, int level)
     gic_update(s);
 }
 
+static uint16_t gic_get_current_pending_irq(GICState *s, int cpu,
+                                            MemTxAttrs attrs)
+{
+    uint16_t pending_irq = s->current_pending[cpu];
+
+    if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
+        int group = GIC_TEST_GROUP(pending_irq, (1 << cpu));
+        /* On a GIC without the security extensions, reading this register
+         * behaves in the same way as a secure access to a GIC with them.
+         */
+        bool secure = !s->security_extn || attrs.secure;
+
+        if (group == 0 && !secure) {
+            /* Group0 interrupts hidden from Non-secure access */
+            return 1023;
+        }
+        if (group == 1 && secure && !(s->cpu_ctlr[cpu] & GICC_CTLR_ACK_CTL)) {
+            /* Group1 interrupts only seen by Secure access if
+             * AckCtl bit set.
+             */
+            return 1022;
+        }
+    }
+    return pending_irq;
+}
+
 static void gic_set_running_irq(GICState *s, int cpu, int irq)
 {
     s->running_irq[cpu] = irq;
@@ -178,14 +230,24 @@ static void gic_set_running_irq(GICState *s, int cpu, int irq)
     gic_update(s);
 }
 
-uint32_t gic_acknowledge_irq(GICState *s, int cpu)
+uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
 {
     int ret, irq, src;
     int cm = 1 << cpu;
-    irq = s->current_pending[cpu];
-    if (irq == 1023
-            || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
-        DPRINTF("ACK no pending IRQ\n");
+
+    /* gic_get_current_pending_irq() will return 1022 or 1023 appropriately
+     * for the case where this GIC supports grouping and the pending interrupt
+     * is in the wrong group.
+     */
+    irq = gic_get_current_pending_irq(s, cpu, attrs);;
+
+    if (irq >= GIC_MAXIRQ) {
+        DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
+        return irq;
+    }
+
+    if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
+        DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
         return 1023;
     }
     s->last_active[irq][cpu] = s->running_irq[cpu];
@@ -224,8 +286,16 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu)
     return ret;
 }
 
-void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val)
+void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
+                      MemTxAttrs attrs)
 {
+    if (s->security_extn && !attrs.secure) {
+        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
+            return; /* Ignore Non-secure access of Group0 IRQ */
+        }
+        val = 0x80 | (val >> 1); /* Non-secure view */
+    }
+
     if (irq < GIC_INTERNAL) {
         s->priority1[irq][cpu] = val;
     } else {
@@ -233,7 +303,113 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val)
     }
 }
 
-void gic_complete_irq(GICState *s, int cpu, int irq)
+static uint32_t gic_get_priority(GICState *s, int cpu, int irq,
+                                 MemTxAttrs attrs)
+{
+    uint32_t prio = GIC_GET_PRIORITY(irq, cpu);
+
+    if (s->security_extn && !attrs.secure) {
+        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
+            return 0; /* Non-secure access cannot read priority of Group0 IRQ */
+        }
+        prio = (prio << 1) & 0xff; /* Non-secure view */
+    }
+    return prio;
+}
+
+static void gic_set_priority_mask(GICState *s, int cpu, uint8_t pmask,
+                                  MemTxAttrs attrs)
+{
+    if (s->security_extn && !attrs.secure) {
+        if (s->priority_mask[cpu] & 0x80) {
+            /* Priority Mask in upper half */
+            pmask = 0x80 | (pmask >> 1);
+        } else {
+            /* Non-secure write ignored if priority mask is in lower half */
+            return;
+        }
+    }
+    s->priority_mask[cpu] = pmask;
+}
+
+static uint32_t gic_get_priority_mask(GICState *s, int cpu, MemTxAttrs attrs)
+{
+    uint32_t pmask = s->priority_mask[cpu];
+
+    if (s->security_extn && !attrs.secure) {
+        if (pmask & 0x80) {
+            /* Priority Mask in upper half, return Non-secure view */
+            pmask = (pmask << 1) & 0xff;
+        } else {
+            /* Priority Mask in lower half, RAZ */
+            pmask = 0;
+        }
+    }
+    return pmask;
+}
+
+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");
+}
+
+static uint8_t gic_get_running_priority(GICState *s, int cpu, MemTxAttrs attrs)
+{
+    if (s->security_extn && !attrs.secure) {
+        if (s->running_priority[cpu] & 0x80) {
+            /* Running priority in upper half of range: return the Non-secure
+             * view of the priority.
+             */
+            return s->running_priority[cpu] << 1;
+        } else {
+            /* Running priority in lower half of range: RAZ */
+            return 0;
+        }
+    } else {
+        return s->running_priority[cpu];
+    }
+}
+
+void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 {
     int update = 0;
     int cm = 1 << cpu;
@@ -263,6 +439,16 @@ void gic_complete_irq(GICState *s, int cpu, int irq)
         }
     }
 
+    if (s->security_extn && !attrs.secure && !GIC_TEST_GROUP(irq, cm)) {
+        DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
+        return;
+    }
+
+    /* Secure EOI with GICC_CTLR.AckCtl == 0 when the IRQ is a Group 1
+     * interrupt is UNPREDICTABLE. We choose to handle it as if AckCtl == 1,
+     * i.e. go ahead and complete the irq anyway.
+     */
+
     if (irq != s->running_irq[cpu]) {
         /* Complete an IRQ that is not currently running.  */
         int tmp = s->running_irq[cpu];
@@ -282,7 +468,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq)
     }
 }
 
-static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
+static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
 {
     GICState *s = (GICState *)opaque;
     uint32_t res;
@@ -295,15 +481,42 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
     cpu = gic_get_current_cpu(s);
     cm = 1 << cpu;
     if (offset < 0x100) {
-        if (offset == 0)
-            return s->enabled;
+        if (offset == 0) {      /* GICD_CTLR */
+            if (s->security_extn && !attrs.secure) {
+                /* The NS bank of this register is just an alias of the
+                 * EnableGrp1 bit in the S bank version.
+                 */
+                return extract32(s->ctlr, 1, 1);
+            } else {
+                return s->ctlr;
+            }
+        }
         if (offset == 4)
-            return ((s->num_irq / 32) - 1) | ((NUM_CPU(s) - 1) << 5);
+            /* Interrupt Controller Type Register */
+            return ((s->num_irq / 32) - 1)
+                    | ((NUM_CPU(s) - 1) << 5)
+                    | (s->security_extn << 10);
         if (offset < 0x08)
             return 0;
         if (offset >= 0x80) {
-            /* Interrupt Security , RAZ/WI */
-            return 0;
+            /* Interrupt Group Registers: these RAZ/WI if this is an NS
+             * access to a GIC with the security extensions, or if the GIC
+             * doesn't have groups at all.
+             */
+            res = 0;
+            if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
+                /* Every byte offset holds 8 group status bits */
+                irq = (offset - 0x080) * 8 + GIC_BASE_IRQ;
+                if (irq >= s->num_irq) {
+                    goto bad_reg;
+                }
+                for (i = 0; i < 8; i++) {
+                    if (GIC_TEST_GROUP(irq + i, cm)) {
+                        res |= (1 << i);
+                    }
+                }
+            }
+            return res;
         }
         goto bad_reg;
     } else if (offset < 0x200) {
@@ -354,7 +567,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
         irq = (offset - 0x400) + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
-        res = GIC_GET_PRIORITY(irq, cpu);
+        res = gic_get_priority(s, cpu, irq, attrs);
     } else if (offset < 0xc00) {
         /* Interrupt CPU Target.  */
         if (s->num_cpu == 1 && s->revision != REV_11MPCORE) {
@@ -415,24 +628,30 @@ bad_reg:
     return 0;
 }
 
-static uint32_t gic_dist_readw(void *opaque, hwaddr offset)
-{
-    uint32_t val;
-    val = gic_dist_readb(opaque, offset);
-    val |= gic_dist_readb(opaque, offset + 1) << 8;
-    return val;
-}
-
-static uint32_t gic_dist_readl(void *opaque, hwaddr offset)
+static MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data,
+                                 unsigned size, MemTxAttrs attrs)
 {
-    uint32_t val;
-    val = gic_dist_readw(opaque, offset);
-    val |= gic_dist_readw(opaque, offset + 2) << 16;
-    return val;
+    switch (size) {
+    case 1:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        return MEMTX_OK;
+    case 2:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
+        return MEMTX_OK;
+    case 4:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
+        *data |= gic_dist_readb(opaque, offset + 2, attrs) << 16;
+        *data |= gic_dist_readb(opaque, offset + 3, attrs) << 24;
+        return MEMTX_OK;
+    default:
+        return MEMTX_ERROR;
+    }
 }
 
 static void gic_dist_writeb(void *opaque, hwaddr offset,
-                            uint32_t value)
+                            uint32_t value, MemTxAttrs attrs)
 {
     GICState *s = (GICState *)opaque;
     int irq;
@@ -442,12 +661,41 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
     cpu = gic_get_current_cpu(s);
     if (offset < 0x100) {
         if (offset == 0) {
-            s->enabled = (value & 1);
-            DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis");
+            if (s->security_extn && !attrs.secure) {
+                /* NS version is just an alias of the S version's bit 1 */
+                s->ctlr = deposit32(s->ctlr, 1, 1, value);
+            } else if (gic_has_groups(s)) {
+                s->ctlr = value & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1);
+            } else {
+                s->ctlr = value & GICD_CTLR_EN_GRP0;
+            }
+            DPRINTF("Distributor: Group0 %sabled; Group 1 %sabled\n",
+                    s->ctlr & GICD_CTLR_EN_GRP0 ? "En" : "Dis",
+                    s->ctlr & GICD_CTLR_EN_GRP1 ? "En" : "Dis");
         } else if (offset < 4) {
             /* ignored.  */
         } else if (offset >= 0x80) {
-            /* Interrupt Security Registers, RAZ/WI */
+            /* Interrupt Group Registers: RAZ/WI for NS access to secure
+             * GIC, or for GICs without groups.
+             */
+            if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
+                /* Every byte offset holds 8 group status bits */
+                irq = (offset - 0x80) * 8 + GIC_BASE_IRQ;
+                if (irq >= s->num_irq) {
+                    goto bad_reg;
+                }
+                for (i = 0; i < 8; i++) {
+                    /* Group bits are banked for private interrupts */
+                    int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
+                    if (value & (1 << i)) {
+                        /* Group1 (Non-secure) */
+                        GIC_SET_GROUP(irq + i, cm);
+                    } else {
+                        /* Group0 (Secure) */
+                        GIC_CLEAR_GROUP(irq + i, cm);
+                    }
+                }
+            }
         } else {
             goto bad_reg;
         }
@@ -537,7 +785,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         irq = (offset - 0x400) + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
-        gic_set_priority(s, cpu, irq, value);
+        gic_set_priority(s, cpu, irq, value, attrs);
     } else if (offset < 0xc00) {
         /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the
          * annoying exception of the 11MPCore's GIC.
@@ -609,14 +857,14 @@ bad_reg:
 }
 
 static void gic_dist_writew(void *opaque, hwaddr offset,
-                            uint32_t value)
+                            uint32_t value, MemTxAttrs attrs)
 {
-    gic_dist_writeb(opaque, offset, value & 0xff);
-    gic_dist_writeb(opaque, offset + 1, value >> 8);
+    gic_dist_writeb(opaque, offset, value & 0xff, attrs);
+    gic_dist_writeb(opaque, offset + 1, value >> 8, attrs);
 }
 
 static void gic_dist_writel(void *opaque, hwaddr offset,
-                            uint32_t value)
+                            uint32_t value, MemTxAttrs attrs)
 {
     GICState *s = (GICState *)opaque;
     if (offset == 0xf00) {
@@ -652,63 +900,110 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
         gic_update(s);
         return;
     }
-    gic_dist_writew(opaque, offset, value & 0xffff);
-    gic_dist_writew(opaque, offset + 2, value >> 16);
+    gic_dist_writew(opaque, offset, value & 0xffff, attrs);
+    gic_dist_writew(opaque, offset + 2, value >> 16, attrs);
+}
+
+static MemTxResult gic_dist_write(void *opaque, hwaddr offset, uint64_t data,
+                                  unsigned size, MemTxAttrs attrs)
+{
+    switch (size) {
+    case 1:
+        gic_dist_writeb(opaque, offset, data, attrs);
+        return MEMTX_OK;
+    case 2:
+        gic_dist_writew(opaque, offset, data, attrs);
+        return MEMTX_OK;
+    case 4:
+        gic_dist_writel(opaque, offset, data, attrs);
+        return MEMTX_OK;
+    default:
+        return MEMTX_ERROR;
+    }
 }
 
 static const MemoryRegionOps gic_dist_ops = {
-    .old_mmio = {
-        .read = { gic_dist_readb, gic_dist_readw, gic_dist_readl, },
-        .write = { gic_dist_writeb, gic_dist_writew, gic_dist_writel, },
-    },
+    .read_with_attrs = gic_dist_read,
+    .write_with_attrs = gic_dist_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static uint32_t gic_cpu_read(GICState *s, int cpu, int offset)
+static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
+                                uint64_t *data, MemTxAttrs attrs)
 {
     switch (offset) {
     case 0x00: /* Control */
-        return s->cpu_enabled[cpu];
+        *data = gic_get_cpu_control(s, cpu, attrs);
+        break;
     case 0x04: /* Priority mask */
-        return s->priority_mask[cpu];
+        *data = gic_get_priority_mask(s, cpu, attrs);
+        break;
     case 0x08: /* Binary Point */
-        return 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 */
-        return gic_acknowledge_irq(s, cpu);
+        *data = gic_acknowledge_irq(s, cpu, attrs);
+        break;
     case 0x14: /* Running Priority */
-        return s->running_priority[cpu];
+        *data = gic_get_running_priority(s, cpu, attrs);
+        break;
     case 0x18: /* Highest Pending Interrupt */
-        return s->current_pending[cpu];
+        *data = gic_get_current_pending_irq(s, cpu, attrs);
+        break;
     case 0x1c: /* Aliased Binary Point */
-        return 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:
-        return s->apr[(offset - 0xd0) / 4][cpu];
+        *data = s->apr[(offset - 0xd0) / 4][cpu];
+        break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gic_cpu_read: Bad offset %x\n", (int)offset);
-        return 0;
+        return MEMTX_ERROR;
     }
+    return MEMTX_OK;
 }
 
-static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value)
+static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
+                                 uint32_t value, MemTxAttrs attrs)
 {
     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);
+        gic_set_priority_mask(s, cpu, value, attrs);
         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;
+        gic_complete_irq(s, cpu, value & 0x3ff, attrs);
+        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:
@@ -717,56 +1012,59 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value)
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gic_cpu_write: Bad offset %x\n", (int)offset);
-        return;
+        return MEMTX_ERROR;
     }
     gic_update(s);
+    return MEMTX_OK;
 }
 
 /* Wrappers to read/write the GIC CPU interface for the current CPU */
-static uint64_t gic_thiscpu_read(void *opaque, hwaddr addr,
-                                 unsigned size)
+static MemTxResult gic_thiscpu_read(void *opaque, hwaddr addr, uint64_t *data,
+                                    unsigned size, MemTxAttrs attrs)
 {
     GICState *s = (GICState *)opaque;
-    return gic_cpu_read(s, gic_get_current_cpu(s), addr);
+    return gic_cpu_read(s, gic_get_current_cpu(s), addr, data, attrs);
 }
 
-static void gic_thiscpu_write(void *opaque, hwaddr addr,
-                              uint64_t value, unsigned size)
+static MemTxResult gic_thiscpu_write(void *opaque, hwaddr addr,
+                                     uint64_t value, unsigned size,
+                                     MemTxAttrs attrs)
 {
     GICState *s = (GICState *)opaque;
-    gic_cpu_write(s, gic_get_current_cpu(s), addr, value);
+    return gic_cpu_write(s, gic_get_current_cpu(s), addr, value, attrs);
 }
 
 /* Wrappers to read/write the GIC CPU interface for a specific CPU.
  * These just decode the opaque pointer into GICState* + cpu id.
  */
-static uint64_t gic_do_cpu_read(void *opaque, hwaddr addr,
-                                unsigned size)
+static MemTxResult gic_do_cpu_read(void *opaque, hwaddr addr, uint64_t *data,
+                                   unsigned size, MemTxAttrs attrs)
 {
     GICState **backref = (GICState **)opaque;
     GICState *s = *backref;
     int id = (backref - s->backref);
-    return gic_cpu_read(s, id, addr);
+    return gic_cpu_read(s, id, addr, data, attrs);
 }
 
-static void gic_do_cpu_write(void *opaque, hwaddr addr,
-                             uint64_t value, unsigned size)
+static MemTxResult gic_do_cpu_write(void *opaque, hwaddr addr,
+                                    uint64_t value, unsigned size,
+                                    MemTxAttrs attrs)
 {
     GICState **backref = (GICState **)opaque;
     GICState *s = *backref;
     int id = (backref - s->backref);
-    gic_cpu_write(s, id, addr, value);
+    return gic_cpu_write(s, id, addr, value, attrs);
 }
 
 static const MemoryRegionOps gic_thiscpu_ops = {
-    .read = gic_thiscpu_read,
-    .write = gic_thiscpu_write,
+    .read_with_attrs = gic_thiscpu_read,
+    .write_with_attrs = gic_thiscpu_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static const MemoryRegionOps gic_cpu_ops = {
-    .read = gic_do_cpu_read,
-    .write = gic_do_cpu_write,
+    .read_with_attrs = gic_do_cpu_read,
+    .write_with_attrs = gic_do_cpu_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -790,6 +1088,9 @@ void gic_init_irqs_and_distributor(GICState *s)
     for (i = 0; i < NUM_CPU(s); i++) {
         sysbus_init_irq(sbd, &s->parent_irq[i]);
     }
+    for (i = 0; i < NUM_CPU(s); i++) {
+        sysbus_init_irq(sbd, &s->parent_fiq[i]);
+    }
     memory_region_init_io(&s->iomem, OBJECT(s), &gic_dist_ops, s,
                           "gic_dist", 0x1000);
 }