[Qemu-devel] [PATCH 4/9] hw/armv7m_nvic: Use MemoryRegions for NVIC specific registers

[Peter Maydell]

Implement the NVIC specific register areas using a set of
overlaid MemoryRegions in a container, rather than by having
the arm_gic read/write functions use special purpose callbacks.

Signed-off-by: Peter Maydell <address@hidden>
---
 hw/arm_gic.c     |   33 ++++-------------------
 hw/armv7m_nvic.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 28 deletions(-)

diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 3293ae4..2ec10ce 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -37,17 +37,17 @@ do { printf("arm_gic: " fmt , ## __VA_ARGS__); } while (0)
 #endif
 
 #ifdef NVIC
-static const uint8_t gic_id[] =
-{ 0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1 };
 /* The NVIC has 16 internal vectors.  However these are not exposed
    through the normal GIC interface.  */
 #define GIC_BASE_IRQ    32
 #else
-static const uint8_t gic_id[] =
-{ 0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1 };
 #define GIC_BASE_IRQ    0
 #endif
 
+static const uint8_t gic_id[] = {
+    0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
+};
+
 #define FROM_SYSBUSGIC(type, dev) \
     DO_UPCAST(type, gic, FROM_SYSBUS(gic_state, dev))
 
@@ -312,7 +312,6 @@ static uint32_t gic_dist_readb(void *opaque, 
target_phys_addr_t offset)
     cpu = gic_get_current_cpu(s);
     cm = 1 << cpu;
     if (offset < 0x100) {
-#ifndef NVIC
         if (offset == 0)
             return s->enabled;
         if (offset == 4)
@@ -323,7 +322,6 @@ static uint32_t gic_dist_readb(void *opaque, 
target_phys_addr_t offset)
             /* Interrupt Security , RAZ/WI */
             return 0;
         }
-#endif
         goto bad_reg;
     } else if (offset < 0x200) {
         /* Interrupt Set/Clear Enable.  */
@@ -385,6 +383,7 @@ static uint32_t gic_dist_readb(void *opaque, 
target_phys_addr_t offset)
         } else {
             res = GIC_TARGET(irq);
         }
+#endif
     } else if (offset < 0xf00) {
         /* Interrupt Configuration.  */
         irq = (offset - 0xc00) * 2 + GIC_BASE_IRQ;
@@ -397,7 +396,6 @@ static uint32_t gic_dist_readb(void *opaque, 
target_phys_addr_t offset)
             if (GIC_TEST_TRIGGER(irq + i))
                 res |= (2 << (i * 2));
         }
-#endif
     } else if (offset < 0xfe0) {
         goto bad_reg;
     } else /* offset >= 0xfe0 */ {
@@ -424,13 +422,6 @@ static uint32_t gic_dist_readw(void *opaque, 
target_phys_addr_t offset)
 static uint32_t gic_dist_readl(void *opaque, target_phys_addr_t offset)
 {
     uint32_t val;
-#ifdef NVIC
-    gic_state *s = (gic_state *)opaque;
-    uint32_t addr;
-    addr = offset;
-    if (addr < 0x100 || addr > 0xd00)
-        return nvic_readl(s, addr);
-#endif
     val = gic_dist_readw(opaque, offset);
     val |= gic_dist_readw(opaque, offset + 2) << 16;
     return val;
@@ -446,9 +437,6 @@ static void gic_dist_writeb(void *opaque, 
target_phys_addr_t offset,
 
     cpu = gic_get_current_cpu(s);
     if (offset < 0x100) {
-#ifdef NVIC
-        goto bad_reg;
-#else
         if (offset == 0) {
             s->enabled = (value & 1);
             DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis");
@@ -459,7 +447,6 @@ static void gic_dist_writeb(void *opaque, 
target_phys_addr_t offset,
         } else {
             goto bad_reg;
         }
-#endif
     } else if (offset < 0x180) {
         /* Interrupt Set Enable.  */
         irq = (offset - 0x100) * 8 + GIC_BASE_IRQ;
@@ -552,6 +539,7 @@ static void gic_dist_writeb(void *opaque, 
target_phys_addr_t offset,
         else if (irq < GIC_INTERNAL)
             value = ALL_CPU_MASK;
         s->irq_target[irq] = value & ALL_CPU_MASK;
+#endif
     } else if (offset < 0xf00) {
         /* Interrupt Configuration.  */
         irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
@@ -571,7 +559,6 @@ static void gic_dist_writeb(void *opaque, 
target_phys_addr_t offset,
                 GIC_CLEAR_TRIGGER(irq + i);
             }
         }
-#endif
     } else {
         /* 0xf00 is only handled for 32-bit writes.  */
         goto bad_reg;
@@ -593,14 +580,6 @@ static void gic_dist_writel(void *opaque, 
target_phys_addr_t offset,
                             uint32_t value)
 {
     gic_state *s = (gic_state *)opaque;
-#ifdef NVIC
-    uint32_t addr;
-    addr = offset;
-    if (addr < 0x100 || (addr > 0xd00 && addr != 0xf00)) {
-        nvic_writel(s, addr, value);
-        return;
-    }
-#endif
     if (offset == 0xf00) {
         int cpu;
         int irq;
diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index 653c011..747e245 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -30,9 +30,16 @@ typedef struct {
         int64_t tick;
         QEMUTimer *timer;
     } systick;
+    MemoryRegion sysregmem;
+    MemoryRegion gic_iomem_alias;
+    MemoryRegion container;
     uint32_t num_irq;
 } nvic_state;
 
+static const uint8_t nvic_id[] = {
+    0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
+};
+
 /* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
 #define SYSTICK_SCALE 1000ULL
 
@@ -358,12 +365,54 @@ static void nvic_writel(void *opaque, uint32_t offset, 
uint32_t value)
     case 0xd38: /* Bus Fault Address.  */
     case 0xd3c: /* Aux Fault Status.  */
         goto bad_reg;
+    case 0xf00: /* Software Triggered Interrupt Register */
+        if ((value & 0x1ff) < s->num_irq) {
+            gic_set_pending_private(&s->gic, 0, value & 0x1ff);
+        }
+        break;
     default:
     bad_reg:
         hw_error("NVIC: Bad write offset 0x%x\n", offset);
     }
 }
 
+static uint64_t nvic_sysreg_read(void *opaque, target_phys_addr_t addr,
+                                 unsigned size)
+{
+    /* At the moment we only support the ID registers for byte/word access.
+     * This is not strictly correct as a few of the other registers also
+     * allow byte access.
+     */
+    uint32_t offset = addr;
+    if (offset >= 0xfe0) {
+        if (offset & 3) {
+            return 0;
+        }
+        return nvic_id[(offset - 0xfe0) >> 2];
+    }
+    if (size == 4) {
+        return nvic_readl(opaque, offset);
+    }
+    hw_error("NVIC: Bad read of size %d at offset 0x%x\n", size, offset);
+}
+
+static void nvic_sysreg_write(void *opaque, target_phys_addr_t addr,
+                              uint64_t value, unsigned size)
+{
+    uint32_t offset = addr;
+    if (size == 4) {
+        nvic_writel(opaque, offset, value);
+        return;
+    }
+    hw_error("NVIC: Bad write of size %d at offset 0x%x\n", size, offset);
+}
+
+static const MemoryRegionOps nvic_sysreg_ops = {
+    .read = nvic_sysreg_read,
+    .write = nvic_sysreg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static const VMStateDescription vmstate_nvic = {
     .name = "armv7m_nvic",
     .version_id = 1,
@@ -399,7 +448,30 @@ static int armv7m_nvic_init(SysBusDevice *dev)
     /* The NVIC always has only one CPU */
     s->gic.num_cpu = 1;
     gic_init(&s->gic, s->num_irq);
-    memory_region_add_subregion(get_system_memory(), 0xe000e000, 
&s->gic.iomem);
+    /* The NVIC and system controller register area looks like this:
+     *  0..0xff : system control registers, including systick
+     *  0x100..0xcff : GIC-like registers
+     *  0xd00..0xfff : system control registers
+     * We use overlaying to put the GIC like registers
+     * over the top of the system control register region.
+     */
+    memory_region_init(&s->container, "nvic", 0x1000);
+    /* The system register region goes at the bottom of the priority
+     * stack as it covers the whole page.
+     */
+    memory_region_init_io(&s->sysregmem, &nvic_sysreg_ops, s,
+                          "nvic_sysregs", 0x1000);
+    memory_region_add_subregion(&s->container, 0, &s->sysregmem);
+    /* Alias the GIC region so we can get only the section of it
+     * we need, and layer it on top of the system register region.
+     */
+    memory_region_init_alias(&s->gic_iomem_alias, "nvic-gic", &s->gic.iomem,
+                             0x100, 0xc00);
+    memory_region_add_subregion_overlap(&s->container, 0x100, &s->gic.iomem, 
1);
+    /* Map the whole thing into system memory at the location required
+     * by the v7M architecture.
+     */
+    memory_region_add_subregion(get_system_memory(), 0xe000e000, 
&s->container);
     s->systick.timer = qemu_new_timer_ns(vm_clock, systick_timer_tick, s);
     return 0;
 }
-- 
1.7.1