[Qemu-devel] [PATCH 00/11] ARMv7M: QOMify

[Qemu-devel] [PATCH 00/11] ARMv7M: QOMify

[Peter Maydell]

This patch series attempts to bring the v7M code
up to something resembling modern practice for
QEMU devices:
 * proper QOM object for the armv7m "container"
   which holds the CPU, NVIC, etc cpu-internal devices"
 * split the systick out into its own device
 * use memory region links rather than having the
   NVIC and bitband map themselves directly into the
   system memory space
 * have stm32f205 create the armv7m container via the
   QOM APIs rather than armv7m_init(), since that fits better
   with the rest of that SoC's code

These patches sit on top of the NVIC rewrite patchset;
you can find a git branch with the whole lot at:

  https://git.linaro.org/people/peter.maydell/qemu-arm.git v7m-qomify

thanks
-- PMM

Peter Maydell (11):
  armv7m: Abstract out the "load kernel" code
  armv7m: Move NVICState struct definition into header
  armv7m: QOMify the armv7m container
  armv7m: Use QOMified armv7m object in armv7m_init()
  armv7m: Make ARMv7M object take memory region link
  armv7m: Make NVIC expose a memory region rather than mapping itself
  armv7m: Make bitband device take the address space to access
  armv7m: Don't put core v7M devices under CONFIG_STELLARIS
  armv7m: Split systick out from NVIC
  stm32f205: Create armv7m object without using armv7m_init()
  stm32f205: Rename 'nvic' local to 'armv7m'

 hw/intc/Makefile.objs             |   2 +-
 hw/timer/Makefile.objs            |   1 +
 include/hw/arm/arm.h              |  12 ++
 include/hw/arm/armv7m.h           |  63 +++++++
 include/hw/arm/armv7m_nvic.h      |  62 +++++++
 include/hw/arm/stm32f205_soc.h    |   4 +-
 include/hw/timer/armv7m_systick.h |  34 ++++
 hw/arm/armv7m.c                   | 380 +++++++++++++++++++++++++-------------
 hw/arm/netduino2.c                |   7 +-
 hw/arm/stm32f205_soc.c            |  28 ++-
 hw/intc/armv7m_nvic.c             | 214 ++++-----------------
 hw/timer/armv7m_systick.c         | 239 ++++++++++++++++++++++++
 default-configs/arm-softmmu.mak   |   2 +
 hw/timer/trace-events             |   6 +
 14 files changed, 733 insertions(+), 321 deletions(-)
 create mode 100644 include/hw/arm/armv7m.h
 create mode 100644 include/hw/arm/armv7m_nvic.h
 create mode 100644 include/hw/timer/armv7m_systick.h
 create mode 100644 hw/timer/armv7m_systick.c

-- 
2.7.4

[Qemu-devel] [PATCH 01/11] armv7m: Abstract out the "load kernel" code

[Peter Maydell]

Abstract the "load kernel" code out of armv7m_init() into its own
function.  This includes the registration of the CPU reset function,
to parallel how we handle this for A profile cores.

We make the function public so that boards which choose to
directly instantiate an ARMv7M device object can call it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/arm.h | 12 ++++++++++++
 hw/arm/armv7m.c      | 23 ++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index c175c0e..a3f79d3 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -26,6 +26,18 @@ typedef enum {
 /* armv7m.c */
 DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
                       const char *kernel_filename, const char *cpu_model);
+/**
+ * armv7m_load_kernel:
+ * @cpu: CPU
+ * @kernel_filename: file to load
+ * @mem_size: mem_size: maximum image size to load
+ *
+ * Load the guest image for an ARMv7M system. This must be called by
+ * any ARMv7M board, either directly or via armv7m_init(). (This is
+ * necessary to ensure that the CPU resets correctly on system reset,
+ * as well as for kernel loading.)
+ */
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);
 
 /*
  * struct used as a parameter of the arm_load_kernel machine init
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 0c9ca7b..b2cc6e9 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
     ARMCPU *cpu;
     CPUARMState *env;
     DeviceState *nvic;
-    int image_size;
-    uint64_t entry;
-    uint64_t lowaddr;
-    int big_endian;
 
     if (cpu_model == NULL) {
 	cpu_model = "cortex-m3";
@@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
     qdev_init_nofail(nvic);
     sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
                        qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
+    armv7m_load_kernel(cpu, kernel_filename, mem_size);
+    return nvic;
+}
+
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
+{
+    int image_size;
+    uint64_t entry;
+    uint64_t lowaddr;
+    int big_endian;
 
 #ifdef TARGET_WORDS_BIGENDIAN
     big_endian = 1;
@@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
         }
     }
 
+    /* CPU objects (unlike devices) are not automatically reset on system
+     * reset, so we must always register a handler to do so. Unlike
+     * A-profile CPUs, we don't need to do anything special in the
+     * handler to arrange that it starts correctly.
+     * This is arguably the wrong place to do this, but it matches the
+     * way A-profile does it. Note that this means that every M profile
+     * board must call this function!
+     */
     qemu_register_reset(armv7m_reset, cpu);
-    return nvic;
 }
 
 static Property bitband_properties[] = {
-- 
2.7.4

[Qemu-devel] [PATCH 02/11] armv7m: Move NVICState struct definition into header

[Peter Maydell]

Move the NVICState struct definition into a header, so we can
embed it into other QOM objects like SoCs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armv7m_nvic.h | 66 ++++++++++++++++++++++++++++++++++++++++++++
 hw/intc/armv7m_nvic.c        | 49 +-------------------------------
 2 files changed, 67 insertions(+), 48 deletions(-)
 create mode 100644 include/hw/arm/armv7m_nvic.h

diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
new file mode 100644
index 0000000..39b94ee
--- /dev/null
+++ b/include/hw/arm/armv7m_nvic.h
@@ -0,0 +1,66 @@
+/*
+ * ARMv7M NVIC object
+ *
+ * Copyright (c) 2017 Linaro Ltd
+ * Written by Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This code is licensed under the GPL version 2 or later.
+ */
+
+#ifndef HW_ARM_ARMV7M_NVIC_H
+#define HW_ARM_ARMV7M_NVIC_H
+
+#include "target/arm/cpu.h"
+#include "hw/sysbus.h"
+
+#define TYPE_NVIC "armv7m_nvic"
+
+#define NVIC(obj) \
+    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
+
+/* Highest permitted number of exceptions (architectural limit) */
+#define NVIC_MAX_VECTORS 512
+
+typedef struct VecInfo {
+    /* Exception priorities can range from -3 to 255; only the unmodifiable
+     * priority values for RESET, NMI and HardFault can be negative.
+     */
+    int16_t prio;
+    uint8_t enabled;
+    uint8_t pending;
+    uint8_t active;
+    uint8_t level; /* exceptions <=15 never set level */
+} VecInfo;
+
+typedef struct NVICState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    ARMCPU *cpu;
+
+    VecInfo vectors[NVIC_MAX_VECTORS];
+    uint32_t prigroup;
+
+    /* vectpending and exception_prio are both cached state that can
+     * be recalculated from the vectors[] array and the prigroup field.
+     */
+    unsigned int vectpending; /* highest prio pending enabled exception */
+    int exception_prio; /* group prio of the highest prio active exception */
+
+    struct {
+        uint32_t control;
+        uint32_t reload;
+        int64_t tick;
+        QEMUTimer *timer;
+    } systick;
+
+    MemoryRegion sysregmem;
+    MemoryRegion container;
+
+    uint32_t num_irq;
+    qemu_irq excpout;
+    qemu_irq sysresetreq;
+} NVICState;
+
+#endif
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 76097b4..f2ada39 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -17,6 +17,7 @@
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/arm/arm.h"
+#include "hw/arm/armv7m_nvic.h"
 #include "target/arm/cpu.h"
 #include "exec/address-spaces.h"
 #include "qemu/log.h"
@@ -47,7 +48,6 @@
  * "exception" more or less interchangeably.
  */
 #define NVIC_FIRST_IRQ 16
-#define NVIC_MAX_VECTORS 512
 #define NVIC_MAX_IRQ (NVIC_MAX_VECTORS - NVIC_FIRST_IRQ)
 
 /* Effective running priority of the CPU when no exception is active
@@ -55,53 +55,6 @@
  */
 #define NVIC_NOEXC_PRIO 0x100
 
-typedef struct VecInfo {
-    /* Exception priorities can range from -3 to 255; only the unmodifiable
-     * priority values for RESET, NMI and HardFault can be negative.
-     */
-    int16_t prio;
-    uint8_t enabled;
-    uint8_t pending;
-    uint8_t active;
-    uint8_t level; /* exceptions <=15 never set level */
-} VecInfo;
-
-typedef struct NVICState {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    ARMCPU *cpu;
-
-    VecInfo vectors[NVIC_MAX_VECTORS];
-    uint32_t prigroup;
-
-    /* vectpending and exception_prio are both cached state that can
-     * be recalculated from the vectors[] array and the prigroup field.
-     */
-    unsigned int vectpending; /* highest prio pending enabled exception */
-    int exception_prio; /* group prio of the highest prio active exception */
-
-    struct {
-        uint32_t control;
-        uint32_t reload;
-        int64_t tick;
-        QEMUTimer *timer;
-    } systick;
-
-    MemoryRegion sysregmem;
-    MemoryRegion container;
-
-    uint32_t num_irq;
-    qemu_irq excpout;
-    qemu_irq sysresetreq;
-} NVICState;
-
-#define TYPE_NVIC "armv7m_nvic"
-
-#define NVIC(obj) \
-    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
-
 static const uint8_t nvic_id[] = {
     0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
 };
-- 
2.7.4

[Qemu-devel] [PATCH 03/11] armv7m: QOMify the armv7m container

[Peter Maydell]

Create a proper QOM object for the armv7m container, which
holds the CPU, the NVIC and the bitband regions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armv7m.h |  51 ++++++++++++++++++
 hw/arm/armv7m.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 179 insertions(+), 12 deletions(-)
 create mode 100644 include/hw/arm/armv7m.h

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
new file mode 100644
index 0000000..193ad71
--- /dev/null
+++ b/include/hw/arm/armv7m.h
@@ -0,0 +1,51 @@
+/*
+ * ARMv7M CPU object
+ *
+ * Copyright (c) 2017 Linaro Ltd
+ * Written by Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This code is licensed under the GPL version 2 or later.
+ */
+
+#ifndef HW_ARM_ARMV7M_H
+#define HW_ARM_ARMV7M_H
+
+#include "hw/sysbus.h"
+#include "hw/arm/armv7m_nvic.h"
+
+#define TYPE_BITBAND "ARM,bitband-memory"
+#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+    uint32_t base;
+} BitBandState;
+
+#define TYPE_ARMV7M "armv7m"
+#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
+
+#define ARMV7M_NUM_BITBANDS 2
+
+/* ARMv7M container object.
+ * + Unnamed GPIO input lines: external IRQ lines for the NVIC
+ * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
+ * + Property "cpu-model": CPU model to instantiate
+ * + Property "num-irq": number of external IRQ lines
+ */
+typedef struct ARMv7MState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+    NVICState nvic;
+    BitBandState bitband[ARMV7M_NUM_BITBANDS];
+    ARMCPU *cpu;
+
+    /* Properties */
+    char *cpu_model;
+} ARMv7MState;
+
+#endif
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index b2cc6e9..56d02d4 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/arm/armv7m.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -120,18 +121,6 @@ static const MemoryRegionOps bitband_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-#define TYPE_BITBAND "ARM,bitband-memory"
-#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
-
-typedef struct {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    MemoryRegion iomem;
-    uint32_t base;
-} BitBandState;
-
 static void bitband_init(Object *obj)
 {
     BitBandState *s = BITBAND(obj);
@@ -159,6 +148,132 @@ static void armv7m_bitband_init(void)
 
 /* Board init.  */
 
+static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
+    0x20000000, 0x40000000
+};
+
+static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
+    0x22000000, 0x42000000
+};
+
+static void armv7m_instance_init(Object *obj)
+{
+    ARMv7MState *s = ARMV7M(obj);
+    int i;
+
+    /* Can't init the cpu here, we don't yet know which model to use */
+
+    object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
+    qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
+    object_property_add_alias(obj, "num-irq",
+                              OBJECT(&s->nvic), "num-irq", &error_abort);
+
+    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+        object_initialize(&s->bitband[i], sizeof(s->bitband[i]), TYPE_BITBAND);
+        qdev_set_parent_bus(DEVICE(&s->bitband[i]), sysbus_get_default());
+    }
+}
+
+static void armv7m_realize(DeviceState *dev, Error **errp)
+{
+    /* here realize our children */
+    ARMv7MState *s = ARMV7M(dev);
+    Error *err = NULL;
+    int i;
+    char **cpustr;
+    ObjectClass *oc;
+    const char *typename;
+    CPUClass *cc;
+
+    cpustr = g_strsplit(s->cpu_model, ",", 2);
+
+    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
+    if (!oc) {
+        error_setg(errp, "Unknown CPU model %s", cpustr[0]);
+        g_strfreev(cpustr);
+        return;
+    }
+
+    cc = CPU_CLASS(oc);
+    typename = object_class_get_name(oc);
+    cc->parse_features(typename, cpustr[1], &err);
+    g_strfreev(cpustr);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    s->cpu = ARM_CPU(object_new(typename));
+    if (!s->cpu) {
+        error_setg(errp, "Unknown CPU model %s", s->cpu_model);
+        return;
+    }
+
+    object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    /* Note that we must realize the NVIC after the CPU */
+    object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    /* Alias the NVIC's input and output GPIOs as our own so the board
+     * code can wire them up. (We do this in realize because the
+     * NVIC doesn't create the input GPIO array until realize.)
+     */
+    qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
+    qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
+
+    /* Wire the NVIC up to the CPU */
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
+                       qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
+    s->cpu->env.nvic = &s->nvic;
+
+    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+        Object *obj = OBJECT(&s->bitband[i]);
+        SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
+
+        object_property_set_int(obj, bitband_input_addr[i], "base", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+        object_property_set_bool(obj, true, "realized", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
+    }
+}
+
+static Property armv7m_properties[] = {
+    DEFINE_PROP_STRING("cpu-model", ARMv7MState, cpu_model),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void armv7m_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = armv7m_realize;
+    dc->props = armv7m_properties;
+}
+
+static const TypeInfo armv7m_info = {
+    .name = TYPE_ARMV7M,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ARMv7MState),
+    .instance_init = armv7m_instance_init,
+    .class_init = armv7m_class_init,
+};
+
 static void armv7m_reset(void *opaque)
 {
     ARMCPU *cpu = opaque;
@@ -264,6 +379,7 @@ static const TypeInfo bitband_info = {
 static void armv7m_register_types(void)
 {
     type_register_static(&bitband_info);
+    type_register_static(&armv7m_info);
 }
 
 type_init(armv7m_register_types)
-- 
2.7.4

[Qemu-devel] [PATCH 04/11] armv7m: Use QOMified armv7m object in armv7m_init()

[Peter Maydell]

Make the legacy armv7m_init() function use the newly QOMified
armv7m object rather than doing everything by hand.

We can return the armv7m object rather than the NVIC from
armv7m_init() because its interface to the rest of the
board (GPIOs, etc) is identical.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/armv7m.c | 49 ++++++++++++-------------------------------------
 1 file changed, 12 insertions(+), 37 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 56d02d4..36f213c 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -131,21 +131,6 @@ static void bitband_init(Object *obj)
     sysbus_init_mmio(dev, &s->iomem);
 }
 
-static void armv7m_bitband_init(void)
-{
-    DeviceState *dev;
-
-    dev = qdev_create(NULL, TYPE_BITBAND);
-    qdev_prop_set_uint32(dev, "base", 0x20000000);
-    qdev_init_nofail(dev);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x22000000);
-
-    dev = qdev_create(NULL, TYPE_BITBAND);
-    qdev_prop_set_uint32(dev, "base", 0x40000000);
-    qdev_init_nofail(dev);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x42000000);
-}
-
 /* Board init.  */
 
 static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
@@ -283,35 +268,25 @@ static void armv7m_reset(void *opaque)
 
 /* Init CPU and memory for a v7-M based board.
    mem_size is in bytes.
-   Returns the NVIC array.  */
+   Returns the ARMv7M device.  */
 
 DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
                       const char *kernel_filename, const char *cpu_model)
 {
-    ARMCPU *cpu;
-    CPUARMState *env;
-    DeviceState *nvic;
+    DeviceState *armv7m;
 
     if (cpu_model == NULL) {
-	cpu_model = "cortex-m3";
+        cpu_model = "cortex-m3";
     }
-    cpu = cpu_arm_init(cpu_model);
-    if (cpu == NULL) {
-        fprintf(stderr, "Unable to find CPU definition\n");
-        exit(1);
-    }
-    env = &cpu->env;
-
-    armv7m_bitband_init();
-
-    nvic = qdev_create(NULL, "armv7m_nvic");
-    qdev_prop_set_uint32(nvic, "num-irq", num_irq);
-    env->nvic = nvic;
-    qdev_init_nofail(nvic);
-    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
-                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
-    armv7m_load_kernel(cpu, kernel_filename, mem_size);
-    return nvic;
+
+    armv7m = qdev_create(NULL, "armv7m");
+    qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
+    qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
+    /* This will exit with an error if the user passed us a bad cpu_model */
+    qdev_init_nofail(armv7m);
+
+    armv7m_load_kernel(ARM_CPU(first_cpu), kernel_filename, mem_size);
+    return armv7m;
 }
 
 void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
-- 
2.7.4

[Qemu-devel] [PATCH 05/11] armv7m: Make ARMv7M object take memory region link

[Peter Maydell]

Make the ARMv7M object take a memory region link which it uses
to wire up the bitband rather than having them always put
themselves in the system address space.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armv7m.h | 10 ++++++++++
 hw/arm/armv7m.c         | 23 ++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 193ad71..3333c91 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -35,6 +35,9 @@ typedef struct {
  * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
  * + Property "cpu-model": CPU model to instantiate
  * + Property "num-irq": number of external IRQ lines
+ * + Property "memory": MemoryRegion defining the physical address space
+ *   that CPU accesses see. (The NVIC, bitbanding and other CPU-internal
+ *   devices will be automatically layered on top of this view.)
  */
 typedef struct ARMv7MState {
     /*< private >*/
@@ -44,8 +47,15 @@ typedef struct ARMv7MState {
     BitBandState bitband[ARMV7M_NUM_BITBANDS];
     ARMCPU *cpu;
 
+    /* MemoryRegion we pass to the CPU, with our devices layered on
+     * top of the ones the board provides in board_memory.
+     */
+    MemoryRegion container;
+
     /* Properties */
     char *cpu_model;
+    /* MemoryRegion the board provides to us (with its devices, RAM, etc) */
+    MemoryRegion *board_memory;
 } ARMv7MState;
 
 #endif
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 36f213c..638c597 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -18,6 +18,7 @@
 #include "elf.h"
 #include "sysemu/qtest.h"
 #include "qemu/error-report.h"
+#include "exec/address-spaces.h"
 
 /* Bitbanded IO.  Each word corresponds to a single bit.  */
 
@@ -148,6 +149,14 @@ static void armv7m_instance_init(Object *obj)
 
     /* Can't init the cpu here, we don't yet know which model to use */
 
+    object_property_add_link(obj, "memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->board_memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX);
+
     object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
     qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
     object_property_add_alias(obj, "num-irq",
@@ -170,6 +179,13 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     const char *typename;
     CPUClass *cc;
 
+    if (!s->board_memory) {
+        error_setg(errp, "memory property was not set");
+        return;
+    }
+
+    memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
+
     cpustr = g_strsplit(s->cpu_model, ",", 2);
 
     oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
@@ -194,6 +210,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
+                             &error_abort);
     object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
@@ -234,7 +252,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             return;
         }
 
-        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
+        memory_region_add_subregion(&s->container, bitband_output_addr[i],
+                                    sysbus_mmio_get_region(sbd, 0));
     }
 }
 
@@ -282,6 +301,8 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
     armv7m = qdev_create(NULL, "armv7m");
     qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
     qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
+    object_property_set_link(OBJECT(armv7m), OBJECT(get_system_memory()),
+                                     "memory", &error_abort);
     /* This will exit with an error if the user passed us a bad cpu_model */
     qdev_init_nofail(armv7m);
 
-- 
2.7.4

[Qemu-devel] [PATCH 06/11] armv7m: Make NVIC expose a memory region rather than mapping itself

[Peter Maydell]

Make the NVIC device expose a memory region for its users
to map, rather than mapping itself into the system memory
space on realize, and get the one user (the ARMv7M object)
to do this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/armv7m.c       | 7 ++++++-
 hw/intc/armv7m_nvic.c | 7 ++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 638c597..fb21f74 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -172,6 +172,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 {
     /* here realize our children */
     ARMv7MState *s = ARMV7M(dev);
+    SysBusDevice *sbd;
     Error *err = NULL;
     int i;
     char **cpustr;
@@ -233,10 +234,14 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
 
     /* Wire the NVIC up to the CPU */
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
+    sbd = SYS_BUS_DEVICE(&s->nvic);
+    sysbus_connect_irq(sbd, 0,
                        qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
     s->cpu->env.nvic = &s->nvic;
 
+    memory_region_add_subregion(&s->container, 0xe000e000,
+                                sysbus_mmio_get_region(sbd, 0));
+
     for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
         Object *obj = OBJECT(&s->bitband[i]);
         SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index f2ada39..c814e16 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -19,7 +19,6 @@
 #include "hw/arm/arm.h"
 #include "hw/arm/armv7m_nvic.h"
 #include "target/arm/cpu.h"
-#include "exec/address-spaces.h"
 #include "qemu/log.h"
 #include "trace.h"
 
@@ -1043,10 +1042,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
                           "nvic_sysregs", 0x1000);
     memory_region_add_subregion(&s->container, 0, &s->sysregmem);
 
-    /* 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);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
+
     s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH 07/11] armv7m: Make bitband device take the address space to access

[Peter Maydell]

Instead of the bitband device doing a cpu_physical_memory_read/write,
make it take a MemoryRegion which specifies where it should be
accessing, and use address_space_read/write to access the
corresponding AddressSpace.

Since this entails pretty much a rewrite, convert away from
old_mmio in the process.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armv7m.h |   2 +
 hw/arm/armv7m.c         | 166 +++++++++++++++++++++++-------------------------
 2 files changed, 81 insertions(+), 87 deletions(-)

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 3333c91..a9b3f2a 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -21,8 +21,10 @@ typedef struct {
     SysBusDevice parent_obj;
     /*< public >*/
 
+    AddressSpace *source_as;
     MemoryRegion iomem;
     uint32_t base;
+    MemoryRegion *source_memory;
 } BitBandState;
 
 #define TYPE_ARMV7M "armv7m"
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index fb21f74..4481106 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -23,103 +23,73 @@
 /* Bitbanded IO.  Each word corresponds to a single bit.  */
 
 /* Get the byte address of the real memory for a bitband access.  */
-static inline uint32_t bitband_addr(void * opaque, uint32_t addr)
+static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
 {
-    uint32_t res;
-
-    res = *(uint32_t *)opaque;
-    res |= (addr & 0x1ffffff) >> 5;
-    return res;
-
-}
-
-static uint32_t bitband_readb(void *opaque, hwaddr offset)
-{
-    uint8_t v;
-    cpu_physical_memory_read(bitband_addr(opaque, offset), &v, 1);
-    return (v & (1 << ((offset >> 2) & 7))) != 0;
-}
-
-static void bitband_writeb(void *opaque, hwaddr offset,
-                           uint32_t value)
-{
-    uint32_t addr;
-    uint8_t mask;
-    uint8_t v;
-    addr = bitband_addr(opaque, offset);
-    mask = (1 << ((offset >> 2) & 7));
-    cpu_physical_memory_read(addr, &v, 1);
-    if (value & 1)
-        v |= mask;
-    else
-        v &= ~mask;
-    cpu_physical_memory_write(addr, &v, 1);
-}
-
-static uint32_t bitband_readw(void *opaque, hwaddr offset)
-{
-    uint32_t addr;
-    uint16_t mask;
-    uint16_t v;
-    addr = bitband_addr(opaque, offset) & ~1;
-    mask = (1 << ((offset >> 2) & 15));
-    mask = tswap16(mask);
-    cpu_physical_memory_read(addr, &v, 2);
-    return (v & mask) != 0;
-}
-
-static void bitband_writew(void *opaque, hwaddr offset,
-                           uint32_t value)
-{
-    uint32_t addr;
-    uint16_t mask;
-    uint16_t v;
-    addr = bitband_addr(opaque, offset) & ~1;
-    mask = (1 << ((offset >> 2) & 15));
-    mask = tswap16(mask);
-    cpu_physical_memory_read(addr, &v, 2);
-    if (value & 1)
-        v |= mask;
-    else
-        v &= ~mask;
-    cpu_physical_memory_write(addr, &v, 2);
+    return s->base | (offset & 0x1ffffff) >> 5;
 }
 
-static uint32_t bitband_readl(void *opaque, hwaddr offset)
+static MemTxResult bitband_read(void *opaque, hwaddr offset,
+                                uint64_t *data, unsigned size, MemTxAttrs attrs)
 {
-    uint32_t addr;
-    uint32_t mask;
-    uint32_t v;
-    addr = bitband_addr(opaque, offset) & ~3;
-    mask = (1 << ((offset >> 2) & 31));
-    mask = tswap32(mask);
-    cpu_physical_memory_read(addr, &v, 4);
-    return (v & mask) != 0;
+    BitBandState *s = opaque;
+    uint8_t buf[4];
+    MemTxResult res;
+    int bitpos, bit;
+    hwaddr addr;
+
+    assert(size <= 4);
+
+    /* Find address in underlying memory and round down to multiple of size */
+    addr = bitband_addr(s, offset) & (-size);
+    res = address_space_read(s->source_as, addr, attrs, buf, size);
+    if (res) {
+        return res;
+    }
+    /* Bit position in the N bytes read... */
+    bitpos = (offset >> 2) & ((size * 8) - 1);
+    /* ...converted to byte in buffer and bit in byte */
+    bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
+    *data = bit;
+    return MEMTX_OK;
 }
 
-static void bitband_writel(void *opaque, hwaddr offset,
-                           uint32_t value)
+static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
+                                 unsigned size, MemTxAttrs attrs)
 {
-    uint32_t addr;
-    uint32_t mask;
-    uint32_t v;
-    addr = bitband_addr(opaque, offset) & ~3;
-    mask = (1 << ((offset >> 2) & 31));
-    mask = tswap32(mask);
-    cpu_physical_memory_read(addr, &v, 4);
-    if (value & 1)
-        v |= mask;
-    else
-        v &= ~mask;
-    cpu_physical_memory_write(addr, &v, 4);
+    BitBandState *s = opaque;
+    uint8_t buf[4];
+    MemTxResult res;
+    int bitpos, bit;
+    hwaddr addr;
+
+    assert(size <= 4);
+
+    /* Find address in underlying memory and round down to multiple of size */
+    addr = bitband_addr(s, offset) & (-size);
+    res = address_space_read(s->source_as, addr, attrs, buf, size);
+    if (res) {
+        return res;
+    }
+    /* Bit position in the N bytes read... */
+    bitpos = (offset >> 2) & ((size * 8) - 1);
+    /* ...converted to byte in buffer and bit in byte */
+    bit = 1 << (bitpos & 7);
+    if (value & 1) {
+        buf[bitpos >> 3] |= bit;
+    } else {
+        buf[bitpos >> 3] &= ~bit;
+    }
+    return address_space_write(s->source_as, addr, attrs, buf, size);
 }
 
 static const MemoryRegionOps bitband_ops = {
-    .old_mmio = {
-        .read = { bitband_readb, bitband_readw, bitband_readl, },
-        .write = { bitband_writeb, bitband_writew, bitband_writel, },
-    },
+    .read_with_attrs = bitband_read,
+    .write_with_attrs = bitband_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 4,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,
 };
 
 static void bitband_init(Object *obj)
@@ -127,11 +97,30 @@ static void bitband_init(Object *obj)
     BitBandState *s = BITBAND(obj);
     SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-    memory_region_init_io(&s->iomem, obj, &bitband_ops, &s->base,
+    object_property_add_link(obj, "source-memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->source_memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
                           "bitband", 0x02000000);
     sysbus_init_mmio(dev, &s->iomem);
 }
 
+static void bitband_realize(DeviceState *dev, Error **errp)
+{
+    BitBandState *s = BITBAND(dev);
+
+    if (!s->source_memory) {
+        error_setg(errp, "source-memory property not set");
+        return;
+    }
+
+    s->source_as = address_space_init_shareable(s->source_memory,
+                                                "bitband-source");
+}
+
 /* Board init.  */
 
 static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
@@ -251,6 +240,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             error_propagate(errp, err);
             return;
         }
+        object_property_set_link(obj, OBJECT(s->board_memory),
+                                 "source-memory", &error_abort);
         object_property_set_bool(obj, true, "realized", &err);
         if (err != NULL) {
             error_propagate(errp, err);
@@ -366,6 +357,7 @@ static void bitband_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->realize = bitband_realize;
     dc->props = bitband_properties;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH 08/11] armv7m: Don't put core v7M devices under CONFIG_STELLARIS

[Peter Maydell]

The NVIC is a core v7M device that exists for all v7M CPUs;
put it under a CONFIG_ARM_V7M rather than hiding it under
CONFIG_STELLARIS.

(We'll use CONFIG_ARM_V7M for the SysTick device too
when we split it out of the NVIC.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/Makefile.objs           | 2 +-
 default-configs/arm-softmmu.mak | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 8948106..adedd0d 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -24,7 +24,7 @@ obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
 obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
-obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
+obj-$(CONFIG_ARM_V7M) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
 obj-$(CONFIG_IOAPIC) += ioapic.o
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index fdf4089..1e3bd2b 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -42,6 +42,8 @@ CONFIG_ARM11MPCORE=y
 CONFIG_A9MPCORE=y
 CONFIG_A15MPCORE=y
 
+CONFIG_ARM_V7M=y
+
 CONFIG_ARM_GIC=y
 CONFIG_ARM_GIC_KVM=$(CONFIG_KVM)
 CONFIG_ARM_TIMER=y
-- 
2.7.4

[Qemu-devel] [PATCH 09/11] armv7m: Split systick out from NVIC

[Peter Maydell]

The SysTick timer isn't really part of the NVIC proper;
we just modelled it that way back when we couldn't
easily have devices that only occupied a small chunk
of a memory region. Split it out into its own device.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/timer/Makefile.objs            |   1 +
 include/hw/arm/armv7m_nvic.h      |  10 +-
 include/hw/timer/armv7m_systick.h |  34 ++++++
 hw/intc/armv7m_nvic.c             | 160 ++++++-------------------
 hw/timer/armv7m_systick.c         | 239 ++++++++++++++++++++++++++++++++++++++
 hw/timer/trace-events             |   6 +
 6 files changed, 317 insertions(+), 133 deletions(-)
 create mode 100644 include/hw/timer/armv7m_systick.h
 create mode 100644 hw/timer/armv7m_systick.c

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index fc99668..dd6f27e 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -1,5 +1,6 @@
 common-obj-$(CONFIG_ARM_TIMER) += arm_timer.o
 common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
+common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o
 common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
 common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
 common-obj-$(CONFIG_DS1338) += ds1338.o
diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
index 39b94ee..1d145fb 100644
--- a/include/hw/arm/armv7m_nvic.h
+++ b/include/hw/arm/armv7m_nvic.h
@@ -12,6 +12,7 @@
 
 #include "target/arm/cpu.h"
 #include "hw/sysbus.h"
+#include "hw/timer/armv7m_systick.h"
 
 #define TYPE_NVIC "armv7m_nvic"
 
@@ -48,19 +49,14 @@ typedef struct NVICState {
     unsigned int vectpending; /* highest prio pending enabled exception */
     int exception_prio; /* group prio of the highest prio active exception */
 
-    struct {
-        uint32_t control;
-        uint32_t reload;
-        int64_t tick;
-        QEMUTimer *timer;
-    } systick;
-
     MemoryRegion sysregmem;
     MemoryRegion container;
 
     uint32_t num_irq;
     qemu_irq excpout;
     qemu_irq sysresetreq;
+
+    SysTickState systick;
 } NVICState;
 
 #endif
diff --git a/include/hw/timer/armv7m_systick.h b/include/hw/timer/armv7m_systick.h
new file mode 100644
index 0000000..cca04de
--- /dev/null
+++ b/include/hw/timer/armv7m_systick.h
@@ -0,0 +1,34 @@
+/*
+ * ARMv7M SysTick timer
+ *
+ * Copyright (c) 2006-2007 CodeSourcery.
+ * Written by Paul Brook
+ * Copyright (c) 2017 Linaro Ltd
+ * Written by Peter Maydell
+ *
+ * This code is licensed under the GPL (version 2 or later).
+ */
+
+#ifndef HW_TIMER_ARMV7M_SYSTICK_H
+#define HW_TIMER_ARMV7M_SYSTICK_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_SYSTICK "armv7m_systick"
+
+#define SYSTICK(obj) OBJECT_CHECK(SysTickState, (obj), TYPE_SYSTICK)
+
+typedef struct SysTickState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    uint32_t control;
+    uint32_t reload;
+    int64_t tick;
+    QEMUTimer *timer;
+    MemoryRegion iomem;
+    qemu_irq irq;
+} SysTickState;
+
+#endif
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index c814e16..32ffa0b 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -58,65 +58,6 @@ 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
-
-#define SYSTICK_ENABLE    (1 << 0)
-#define SYSTICK_TICKINT   (1 << 1)
-#define SYSTICK_CLKSOURCE (1 << 2)
-#define SYSTICK_COUNTFLAG (1 << 16)
-
-int system_clock_scale;
-
-/* Conversion factor from qemu timer to SysTick frequencies.  */
-static inline int64_t systick_scale(NVICState *s)
-{
-    if (s->systick.control & SYSTICK_CLKSOURCE)
-        return system_clock_scale;
-    else
-        return 1000;
-}
-
-static void systick_reload(NVICState *s, int reset)
-{
-    /* The Cortex-M3 Devices Generic User Guide says that "When the
-     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
-     * SYST RVR register and then counts down". So, we need to check the
-     * ENABLE bit before reloading the value.
-     */
-    if ((s->systick.control & SYSTICK_ENABLE) == 0) {
-        return;
-    }
-
-    if (reset)
-        s->systick.tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    s->systick.tick += (s->systick.reload + 1) * systick_scale(s);
-    timer_mod(s->systick.timer, s->systick.tick);
-}
-
-static void systick_timer_tick(void * opaque)
-{
-    NVICState *s = (NVICState *)opaque;
-    s->systick.control |= SYSTICK_COUNTFLAG;
-    if (s->systick.control & SYSTICK_TICKINT) {
-        /* Trigger the interrupt.  */
-        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
-    }
-    if (s->systick.reload == 0) {
-        s->systick.control &= ~SYSTICK_ENABLE;
-    } else {
-        systick_reload(s, 0);
-    }
-}
-
-static void systick_reset(NVICState *s)
-{
-    s->systick.control = 0;
-    s->systick.reload = 0;
-    s->systick.tick = 0;
-    timer_del(s->systick.timer);
-}
-
 static int nvic_pending_prio(NVICState *s)
 {
     /* return the priority of the current pending interrupt,
@@ -462,30 +403,6 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
     switch (offset) {
     case 4: /* Interrupt Control Type.  */
         return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;
-    case 0x10: /* SysTick Control and Status.  */
-        val = s->systick.control;
-        s->systick.control &= ~SYSTICK_COUNTFLAG;
-        return val;
-    case 0x14: /* SysTick Reload Value.  */
-        return s->systick.reload;
-    case 0x18: /* SysTick Current Value.  */
-        {
-            int64_t t;
-            if ((s->systick.control & SYSTICK_ENABLE) == 0)
-                return 0;
-            t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            if (t >= s->systick.tick)
-                return 0;
-            val = ((s->systick.tick - (t + 1)) / systick_scale(s)) + 1;
-            /* The interrupt in triggered when the timer reaches zero.
-               However the counter is not reloaded until the next clock
-               tick.  This is a hack to return zero during the first tick.  */
-            if (val > s->systick.reload)
-                val = 0;
-            return val;
-        }
-    case 0x1c: /* SysTick Calibration Value.  */
-        return 10000;
     case 0xd00: /* CPUID Base.  */
         return cpu->midr;
     case 0xd04: /* Interrupt Control State.  */
@@ -620,40 +537,8 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
 static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
 {
     ARMCPU *cpu = s->cpu;
-    uint32_t oldval;
+
     switch (offset) {
-    case 0x10: /* SysTick Control and Status.  */
-        oldval = s->systick.control;
-        s->systick.control &= 0xfffffff8;
-        s->systick.control |= value & 7;
-        if ((oldval ^ value) & SYSTICK_ENABLE) {
-            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            if (value & SYSTICK_ENABLE) {
-                if (s->systick.tick) {
-                    s->systick.tick += now;
-                    timer_mod(s->systick.timer, s->systick.tick);
-                } else {
-                    systick_reload(s, 1);
-                }
-            } else {
-                timer_del(s->systick.timer);
-                s->systick.tick -= now;
-                if (s->systick.tick < 0)
-                  s->systick.tick = 0;
-            }
-        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
-            /* This is a hack. Force the timer to be reloaded
-               when the reference clock is changed.  */
-            systick_reload(s, 1);
-        }
-        break;
-    case 0x14: /* SysTick Reload Value.  */
-        s->systick.reload = value;
-        break;
-    case 0x18: /* SysTick Current Value.  Writes reload the timer.  */
-        systick_reload(s, 1);
-        s->systick.control &= ~SYSTICK_COUNTFLAG;
-        break;
     case 0xd04: /* Interrupt Control State.  */
         if (value & (1 << 31)) {
             armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI);
@@ -952,16 +837,12 @@ static const VMStateDescription vmstate_VecInfo = {
 
 static const VMStateDescription vmstate_nvic = {
     .name = "armv7m_nvic",
-    .version_id = 3,
-    .minimum_version_id = 3,
+    .version_id = 4,
+    .minimum_version_id = 4,
     .post_load = &nvic_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT_ARRAY(vectors, NVICState, NVIC_MAX_VECTORS, 1,
                              vmstate_VecInfo, VecInfo),
-        VMSTATE_UINT32(systick.control, NVICState),
-        VMSTATE_UINT32(systick.reload, NVICState),
-        VMSTATE_INT64(systick.tick, NVICState),
-        VMSTATE_TIMER_PTR(systick.timer, NVICState),
         VMSTATE_UINT32(prigroup, NVICState),
         VMSTATE_END_OF_LIST()
     }
@@ -999,13 +880,26 @@ static void armv7m_nvic_reset(DeviceState *dev)
 
     s->exception_prio = NVIC_NOEXC_PRIO;
     s->vectpending = 0;
+}
 
-    systick_reset(s);
+static void nvic_systick_trigger(void *opaque, int n, int level)
+{
+    NVICState *s = opaque;
+
+    if (level) {
+        /* SysTick just asked us to pend its exception.
+         * (This is different from an external interrupt line's
+         * behaviour.)
+         */
+        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
+    }
 }
 
 static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
 {
     NVICState *s = NVIC(dev);
+    SysBusDevice *systick_sbd;
+    Error *err = NULL;
 
     s->cpu = ARM_CPU(qemu_get_cpu(0));
     assert(s->cpu);
@@ -1020,10 +914,19 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
     /* include space for internal exception vectors */
     s->num_irq += NVIC_FIRST_IRQ;
 
+    object_property_set_bool(OBJECT(&s->systick), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+    systick_sbd = SYS_BUS_DEVICE(&s->systick);
+    sysbus_connect_irq(systick_sbd, 0,
+                       qdev_get_gpio_in_named(dev, "systick-trigger", 0));
+
     /* The NVIC and System Control Space (SCS) starts at 0xe000e000
      * and looks like this:
      *  0x004 - ICTR
-     *  0x010 - 0x1c - systick
+     *  0x010 - 0xff - systick
      *  0x100..0x7ec - NVIC
      *  0x7f0..0xcff - Reserved
      *  0xd00..0xd3c - SCS registers
@@ -1041,10 +944,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s,
                           "nvic_sysregs", 0x1000);
     memory_region_add_subregion(&s->container, 0, &s->sysregmem);
+    memory_region_add_subregion_overlap(&s->container, 0x10,
+                                        sysbus_mmio_get_region(systick_sbd, 0),
+                                        1);
 
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
-
-    s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
 }
 
 static void armv7m_nvic_instance_init(Object *obj)
@@ -1059,8 +963,12 @@ static void armv7m_nvic_instance_init(Object *obj)
     NVICState *nvic = NVIC(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
+    object_initialize(&nvic->systick, sizeof(nvic->systick), TYPE_SYSTICK);
+    qdev_set_parent_bus(DEVICE(&nvic->systick), sysbus_get_default());
+
     sysbus_init_irq(sbd, &nvic->excpout);
     qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1);
+    qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", 1);
 }
 
 static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
new file mode 100644
index 0000000..3fd5e72
--- /dev/null
+++ b/hw/timer/armv7m_systick.c
@@ -0,0 +1,239 @@
+/*
+ * ARMv7M SysTick timer
+ *
+ * Copyright (c) 2006-2007 CodeSourcery.
+ * Written by Paul Brook
+ * Copyright (c) 2017 Linaro Ltd
+ * Written by Peter Maydell
+ *
+ * This code is licensed under the GPL (version 2 or later).
+ */
+
+#include "qemu/osdep.h"
+#include "hw/timer/armv7m_systick.h"
+#include "qemu-common.h"
+#include "hw/sysbus.h"
+#include "qemu/timer.h"
+#include "qemu/log.h"
+#include "trace.h"
+
+/* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
+#define SYSTICK_SCALE 1000ULL
+
+#define SYSTICK_ENABLE    (1 << 0)
+#define SYSTICK_TICKINT   (1 << 1)
+#define SYSTICK_CLKSOURCE (1 << 2)
+#define SYSTICK_COUNTFLAG (1 << 16)
+
+int system_clock_scale;
+
+/* Conversion factor from qemu timer to SysTick frequencies.  */
+static inline int64_t systick_scale(SysTickState *s)
+{
+    if (s->control & SYSTICK_CLKSOURCE) {
+        return system_clock_scale;
+    } else {
+        return 1000;
+    }
+}
+
+static void systick_reload(SysTickState *s, int reset)
+{
+    /* The Cortex-M3 Devices Generic User Guide says that "When the
+     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
+     * SYST RVR register and then counts down". So, we need to check the
+     * ENABLE bit before reloading the value.
+     */
+    trace_systick_reload();
+
+    if ((s->control & SYSTICK_ENABLE) == 0) {
+        return;
+    }
+
+    if (reset) {
+        s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    }
+    s->tick += (s->reload + 1) * systick_scale(s);
+    timer_mod(s->timer, s->tick);
+}
+
+static void systick_timer_tick(void *opaque)
+{
+    SysTickState *s = (SysTickState *)opaque;
+
+    trace_systick_timer_tick();
+
+    s->control |= SYSTICK_COUNTFLAG;
+    if (s->control & SYSTICK_TICKINT) {
+        /* Tell the NVIC to pend the SysTick exception */
+        qemu_irq_pulse(s->irq);
+    }
+    if (s->reload == 0) {
+        s->control &= ~SYSTICK_ENABLE;
+    } else {
+        systick_reload(s, 0);
+    }
+}
+
+static uint64_t systick_read(void *opaque, hwaddr addr, unsigned size)
+{
+    SysTickState *s = opaque;
+    uint32_t val;
+
+    switch (addr) {
+    case 0x0: /* SysTick Control and Status.  */
+        val = s->control;
+        s->control &= ~SYSTICK_COUNTFLAG;
+        break;
+    case 0x4: /* SysTick Reload Value.  */
+        val = s->reload;
+        break;
+    case 0x8: /* SysTick Current Value.  */
+    {
+        int64_t t;
+
+        if ((s->control & SYSTICK_ENABLE) == 0) {
+            val = 0;
+            break;
+        }
+        t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        if (t >= s->tick) {
+            val = 0;
+            break;
+        }
+        val = ((s->tick - (t + 1)) / systick_scale(s)) + 1;
+        /* The interrupt in triggered when the timer reaches zero.
+           However the counter is not reloaded until the next clock
+           tick.  This is a hack to return zero during the first tick.  */
+        if (val > s->reload) {
+            val = 0;
+        }
+        break;
+    }
+    case 0xc: /* SysTick Calibration Value.  */
+        val = 10000;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SysTick: Bad read offset 0x%" HWADDR_PRIx "\n", addr);
+        break;
+    }
+
+    trace_systick_read(addr, val, size);
+    return val;
+}
+
+static void systick_write(void *opaque, hwaddr addr,
+                          uint64_t value, unsigned size)
+{
+    SysTickState *s = opaque;
+
+    trace_systick_write(addr, value, size);
+
+    switch (addr) {
+    case 0x0: /* SysTick Control and Status.  */
+    {
+        uint32_t oldval = s->control;
+
+        s->control &= 0xfffffff8;
+        s->control |= value & 7;
+        if ((oldval ^ value) & SYSTICK_ENABLE) {
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+            if (value & SYSTICK_ENABLE) {
+                if (s->tick) {
+                    s->tick += now;
+                    timer_mod(s->timer, s->tick);
+                } else {
+                    systick_reload(s, 1);
+                }
+            } else {
+                timer_del(s->timer);
+                s->tick -= now;
+                if (s->tick < 0) {
+                    s->tick = 0;
+                }
+            }
+        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
+            /* This is a hack. Force the timer to be reloaded
+               when the reference clock is changed.  */
+            systick_reload(s, 1);
+        }
+        break;
+    }
+    case 0x4: /* SysTick Reload Value.  */
+        s->reload = value;
+        break;
+    case 0x8: /* SysTick Current Value.  Writes reload the timer.  */
+        systick_reload(s, 1);
+        s->control &= ~SYSTICK_COUNTFLAG;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SysTick: Bad write offset 0x%" HWADDR_PRIx "\n", addr);
+    }
+}
+
+static const MemoryRegionOps systick_ops = {
+    .read = systick_read,
+    .write = systick_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static void systick_reset(DeviceState *dev)
+{
+    SysTickState *s = SYSTICK(dev);
+
+    s->control = 0;
+    s->reload = 0;
+    s->tick = 0;
+    timer_del(s->timer);
+}
+
+static void systick_instance_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    SysTickState *s = SYSTICK(obj);
+
+    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick", 0xe0);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
+}
+
+static const VMStateDescription vmstate_systick = {
+    .name = "armv7m_systick",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(control, SysTickState),
+        VMSTATE_UINT32(reload, SysTickState),
+        VMSTATE_INT64(tick, SysTickState),
+        VMSTATE_TIMER_PTR(timer, SysTickState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void systick_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_systick;
+    dc->reset = systick_reset;
+}
+
+static const TypeInfo armv7m_systick_info = {
+    .name = TYPE_SYSTICK,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_init = systick_instance_init,
+    .instance_size = sizeof(SysTickState),
+    .class_init = systick_class_init,
+};
+
+static void armv7m_systick_register_types(void)
+{
+    type_register_static(&armv7m_systick_info);
+}
+
+type_init(armv7m_systick_register_types)
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index 3495c41..d17cfe6 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -49,3 +49,9 @@ aspeed_timer_ctrl_pulse_enable(uint8_t i, bool enable) "Timer %" PRIu8 ": %d"
 aspeed_timer_set_ctrl2(uint32_t value) "Value: 0x%" PRIx32
 aspeed_timer_set_value(int timer, int reg, uint32_t value) "Timer %d register %d: 0x%" PRIx32
 aspeed_timer_read(uint64_t offset, unsigned size, uint64_t value) "From 0x%" PRIx64 ": of size %u: 0x%" PRIx64
+
+# hw/timer/armv7m_systick.c
+systick_reload(void) "systick reload"
+systick_timer_tick(void) "systick reload"
+systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
-- 
2.7.4

[Qemu-devel] [PATCH 10/11] stm32f205: Create armv7m object without using armv7m_init()

[Peter Maydell]

Switch the stm32f205 SoC to create the armv7m object directly
rather than via the armv7m_init() wrapper. This fits better
with the SoC model's very QOMified design.

In particular this means we can push loading the guest image
out to the top level board code where it belongs, rather
than the SoC object having a QOM property for the filename
to load.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/stm32f205_soc.h |  4 +++-
 hw/arm/netduino2.c             |  7 ++++---
 hw/arm/stm32f205_soc.c         | 16 +++++++++++++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 1332141..e2dce11 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -31,6 +31,7 @@
 #include "hw/adc/stm32f2xx_adc.h"
 #include "hw/or-irq.h"
 #include "hw/ssi/stm32f2xx_spi.h"
+#include "hw/arm/armv7m.h"
 
 #define TYPE_STM32F205_SOC "stm32f205-soc"
 #define STM32F205_SOC(obj) \
@@ -51,9 +52,10 @@ typedef struct STM32F205State {
     SysBusDevice parent_obj;
     /*< public >*/
 
-    char *kernel_filename;
     char *cpu_model;
 
+    ARMv7MState armv7m;
+
     STM32F2XXSyscfgState syscfg;
     STM32F2XXUsartState usart[STM_NUM_USARTS];
     STM32F2XXTimerState timer[STM_NUM_TIMERS];
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 23d7928..3cfe332 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -27,17 +27,18 @@
 #include "hw/boards.h"
 #include "qemu/error-report.h"
 #include "hw/arm/stm32f205_soc.h"
+#include "hw/arm/arm.h"
 
 static void netduino2_init(MachineState *machine)
 {
     DeviceState *dev;
 
     dev = qdev_create(NULL, TYPE_STM32F205_SOC);
-    if (machine->kernel_filename) {
-        qdev_prop_set_string(dev, "kernel-filename", machine->kernel_filename);
-    }
     qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
     object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
+
+    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+                       FLASH_SIZE);
 }
 
 static void netduino2_machine_init(MachineClass *mc)
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 38425bd..e6bd73a 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -49,6 +49,9 @@ static void stm32f205_soc_initfn(Object *obj)
     STM32F205State *s = STM32F205_SOC(obj);
     int i;
 
+    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
+    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
+
     object_initialize(&s->syscfg, sizeof(s->syscfg), TYPE_STM32F2XX_SYSCFG);
     qdev_set_parent_bus(DEVICE(&s->syscfg), sysbus_get_default());
 
@@ -110,8 +113,16 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
     vmstate_register_ram_global(sram);
     memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
 
-    nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
-                       s->kernel_filename, s->cpu_model);
+    nvic = DEVICE(&s->armv7m);
+    qdev_prop_set_uint32(nvic, "num-irq", 96);
+    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
+    object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
+                                     "memory", &error_abort);
+    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
     /* System configuration controller */
     dev = DEVICE(&s->syscfg);
@@ -192,7 +203,6 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
 }
 
 static Property stm32f205_soc_properties[] = {
-    DEFINE_PROP_STRING("kernel-filename", STM32F205State, kernel_filename),
     DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
     DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.7.4

[Qemu-devel] [PATCH 11/11] stm32f205: Rename 'nvic' local to 'armv7m'

[Peter Maydell]

The local variable 'nvic' in stm32f205_soc_realize() no longer
holds a direct pointer to the NVIC device; it is a pointer to
the ARMv7M container object. Rename it 'armv7m' accordingly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/stm32f205_soc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index e6bd73a..6e1260d 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -85,7 +85,7 @@ static void stm32f205_soc_initfn(Object *obj)
 static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
 {
     STM32F205State *s = STM32F205_SOC(dev_soc);
-    DeviceState *dev, *nvic;
+    DeviceState *dev, *armv7m;
     SysBusDevice *busdev;
     Error *err = NULL;
     int i;
@@ -113,9 +113,9 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
     vmstate_register_ram_global(sram);
     memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
 
-    nvic = DEVICE(&s->armv7m);
-    qdev_prop_set_uint32(nvic, "num-irq", 96);
-    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
+    armv7m = DEVICE(&s->armv7m);
+    qdev_prop_set_uint32(armv7m, "num-irq", 96);
+    qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
     object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
                                      "memory", &error_abort);
     object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
@@ -133,7 +133,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
     }
     busdev = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(busdev, 0, 0x40013800);
-    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, 71));
+    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71));
 
     /* Attach UART (uses USART registers) and USART controllers */
     for (i = 0; i < STM_NUM_USARTS; i++) {
@@ -147,7 +147,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
         }
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_mmio_map(busdev, 0, usart_addr[i]);
-        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, usart_irq[i]));
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
     }
 
     /* Timer 2 to 5 */
@@ -161,7 +161,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
         }
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_mmio_map(busdev, 0, timer_addr[i]);
-        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, timer_irq[i]));
     }
 
     /* ADC 1 to 3 */
@@ -173,7 +173,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
     qdev_connect_gpio_out(DEVICE(s->adc_irqs), 0,
-                          qdev_get_gpio_in(nvic, ADC_IRQ));
+                          qdev_get_gpio_in(armv7m, ADC_IRQ));
 
     for (i = 0; i < STM_NUM_ADCS; i++) {
         dev = DEVICE(&(s->adc[i]));
@@ -198,7 +198,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
         }
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_mmio_map(busdev, 0, spi_addr[i]);
-        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, spi_irq[i]));
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, spi_irq[i]));
     }
 }
 
-- 
2.7.4

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/11] armv7m: Abstract out the "load kernel" code

[Philippe Mathieu-Daudé]

On 02/20/2017 12:35 PM, Peter Maydell wrote:
> Abstract the "load kernel" code out of armv7m_init() into its own
> function.  This includes the registration of the CPU reset function,
> to parallel how we handle this for A profile cores.
>
> We make the function public so that boards which choose to
> directly instantiate an ARMv7M device object can call it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/arm.h | 12 ++++++++++++
>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----
>  2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index c175c0e..a3f79d3 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -26,6 +26,18 @@ typedef enum {
>  /* armv7m.c */
>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>                        const char *kernel_filename, const char *cpu_model);
> +/**
> + * armv7m_load_kernel:
> + * @cpu: CPU
> + * @kernel_filename: file to load
> + * @mem_size: mem_size: maximum image size to load
> + *
> + * Load the guest image for an ARMv7M system. This must be called by
> + * any ARMv7M board, either directly or via armv7m_init(). (This is
> + * necessary to ensure that the CPU resets correctly on system reset,
> + * as well as for kernel loading.)
> + */
> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);
>
>  /*
>   * struct used as a parameter of the arm_load_kernel machine init
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 0c9ca7b..b2cc6e9 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>      ARMCPU *cpu;
>      CPUARMState *env;
>      DeviceState *nvic;
> -    int image_size;
> -    uint64_t entry;
> -    uint64_t lowaddr;
> -    int big_endian;
>
>      if (cpu_model == NULL) {
>  	cpu_model = "cortex-m3";
> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>      qdev_init_nofail(nvic);
>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);
> +    return nvic;
> +}
> +
> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
> +{
> +    int image_size;
> +    uint64_t entry;
> +    uint64_t lowaddr;
> +    int big_endian;
>
>  #ifdef TARGET_WORDS_BIGENDIAN
>      big_endian = 1;
> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>          }
>      }
>
> +    /* CPU objects (unlike devices) are not automatically reset on system
> +     * reset, so we must always register a handler to do so. Unlike
> +     * A-profile CPUs, we don't need to do anything special in the
> +     * handler to arrange that it starts correctly.
> +     * This is arguably the wrong place to do this, but it matches the
> +     * way A-profile does it. Note that this means that every M profile
> +     * board must call this function!
> +     */
>      qemu_register_reset(armv7m_reset, cpu);
> -    return nvic;
>  }
>
>  static Property bitband_properties[] = {
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 02/11] armv7m: Move NVICState struct definition into header

[Philippe Mathieu-Daudé]

On 02/20/2017 12:35 PM, Peter Maydell wrote:
> Move the NVICState struct definition into a header, so we can
> embed it into other QOM objects like SoCs.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/armv7m_nvic.h | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/intc/armv7m_nvic.c        | 49 +-------------------------------
>  2 files changed, 67 insertions(+), 48 deletions(-)
>  create mode 100644 include/hw/arm/armv7m_nvic.h
>
> diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
> new file mode 100644
> index 0000000..39b94ee
> --- /dev/null
> +++ b/include/hw/arm/armv7m_nvic.h
> @@ -0,0 +1,66 @@
> +/*
> + * ARMv7M NVIC object
> + *
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This code is licensed under the GPL version 2 or later.
> + */
> +
> +#ifndef HW_ARM_ARMV7M_NVIC_H
> +#define HW_ARM_ARMV7M_NVIC_H
> +
> +#include "target/arm/cpu.h"
> +#include "hw/sysbus.h"
> +
> +#define TYPE_NVIC "armv7m_nvic"
> +
> +#define NVIC(obj) \
> +    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
> +
> +/* Highest permitted number of exceptions (architectural limit) */
> +#define NVIC_MAX_VECTORS 512
> +
> +typedef struct VecInfo {
> +    /* Exception priorities can range from -3 to 255; only the unmodifiable
> +     * priority values for RESET, NMI and HardFault can be negative.
> +     */
> +    int16_t prio;
> +    uint8_t enabled;
> +    uint8_t pending;
> +    uint8_t active;
> +    uint8_t level; /* exceptions <=15 never set level */
> +} VecInfo;
> +
> +typedef struct NVICState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    ARMCPU *cpu;
> +
> +    VecInfo vectors[NVIC_MAX_VECTORS];
> +    uint32_t prigroup;
> +
> +    /* vectpending and exception_prio are both cached state that can
> +     * be recalculated from the vectors[] array and the prigroup field.
> +     */
> +    unsigned int vectpending; /* highest prio pending enabled exception */
> +    int exception_prio; /* group prio of the highest prio active exception */
> +
> +    struct {
> +        uint32_t control;
> +        uint32_t reload;
> +        int64_t tick;
> +        QEMUTimer *timer;
> +    } systick;
> +
> +    MemoryRegion sysregmem;
> +    MemoryRegion container;
> +
> +    uint32_t num_irq;
> +    qemu_irq excpout;
> +    qemu_irq sysresetreq;
> +} NVICState;
> +
> +#endif
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 76097b4..f2ada39 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -17,6 +17,7 @@
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "hw/arm/arm.h"
> +#include "hw/arm/armv7m_nvic.h"
>  #include "target/arm/cpu.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/log.h"
> @@ -47,7 +48,6 @@
>   * "exception" more or less interchangeably.
>   */
>  #define NVIC_FIRST_IRQ 16
> -#define NVIC_MAX_VECTORS 512
>  #define NVIC_MAX_IRQ (NVIC_MAX_VECTORS - NVIC_FIRST_IRQ)
>
>  /* Effective running priority of the CPU when no exception is active
> @@ -55,53 +55,6 @@
>   */
>  #define NVIC_NOEXC_PRIO 0x100
>
> -typedef struct VecInfo {
> -    /* Exception priorities can range from -3 to 255; only the unmodifiable
> -     * priority values for RESET, NMI and HardFault can be negative.
> -     */
> -    int16_t prio;
> -    uint8_t enabled;
> -    uint8_t pending;
> -    uint8_t active;
> -    uint8_t level; /* exceptions <=15 never set level */
> -} VecInfo;
> -
> -typedef struct NVICState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    ARMCPU *cpu;
> -
> -    VecInfo vectors[NVIC_MAX_VECTORS];
> -    uint32_t prigroup;
> -
> -    /* vectpending and exception_prio are both cached state that can
> -     * be recalculated from the vectors[] array and the prigroup field.
> -     */
> -    unsigned int vectpending; /* highest prio pending enabled exception */
> -    int exception_prio; /* group prio of the highest prio active exception */
> -
> -    struct {
> -        uint32_t control;
> -        uint32_t reload;
> -        int64_t tick;
> -        QEMUTimer *timer;
> -    } systick;
> -
> -    MemoryRegion sysregmem;
> -    MemoryRegion container;
> -
> -    uint32_t num_irq;
> -    qemu_irq excpout;
> -    qemu_irq sysresetreq;
> -} NVICState;
> -
> -#define TYPE_NVIC "armv7m_nvic"
> -
> -#define NVIC(obj) \
> -    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
> -
>  static const uint8_t nvic_id[] = {
>      0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
>  };
>

Re: [Qemu-devel] [PATCH 08/11] armv7m: Don't put core v7M devices under CONFIG_STELLARIS

[Philippe Mathieu-Daudé]

On 02/20/2017 12:36 PM, Peter Maydell wrote:
> The NVIC is a core v7M device that exists for all v7M CPUs;
> put it under a CONFIG_ARM_V7M rather than hiding it under
> CONFIG_STELLARIS.
>
> (We'll use CONFIG_ARM_V7M for the SysTick device too
> when we split it out of the NVIC.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/intc/Makefile.objs           | 2 +-
>  default-configs/arm-softmmu.mak | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 8948106..adedd0d 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -24,7 +24,7 @@ obj-$(CONFIG_APIC) += apic.o apic_common.o
>  obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
>  obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
>  obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
> -obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
> +obj-$(CONFIG_ARM_V7M) += armv7m_nvic.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
>  obj-$(CONFIG_GRLIB) += grlib_irqmp.o
>  obj-$(CONFIG_IOAPIC) += ioapic.o
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index fdf4089..1e3bd2b 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -42,6 +42,8 @@ CONFIG_ARM11MPCORE=y
>  CONFIG_A9MPCORE=y
>  CONFIG_A15MPCORE=y
>
> +CONFIG_ARM_V7M=y
> +
>  CONFIG_ARM_GIC=y
>  CONFIG_ARM_GIC_KVM=$(CONFIG_KVM)
>  CONFIG_ARM_TIMER=y
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 09/11] armv7m: Split systick out from NVIC

[Philippe Mathieu-Daudé]

Hi peter,

On 02/20/2017 12:36 PM, Peter Maydell wrote:
> The SysTick timer isn't really part of the NVIC proper;
> we just modelled it that way back when we couldn't
> easily have devices that only occupied a small chunk
> of a memory region. Split it out into its own device.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/timer/Makefile.objs            |   1 +
>  include/hw/arm/armv7m_nvic.h      |  10 +-
>  include/hw/timer/armv7m_systick.h |  34 ++++++
>  hw/intc/armv7m_nvic.c             | 160 ++++++-------------------
>  hw/timer/armv7m_systick.c         | 239 ++++++++++++++++++++++++++++++++++++++
>  hw/timer/trace-events             |   6 +
>  6 files changed, 317 insertions(+), 133 deletions(-)
>  create mode 100644 include/hw/timer/armv7m_systick.h
>  create mode 100644 hw/timer/armv7m_systick.c
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index fc99668..dd6f27e 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -1,5 +1,6 @@
>  common-obj-$(CONFIG_ARM_TIMER) += arm_timer.o
>  common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> +common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o
>  common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
>  common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
>  common-obj-$(CONFIG_DS1338) += ds1338.o
> diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
> index 39b94ee..1d145fb 100644
> --- a/include/hw/arm/armv7m_nvic.h
> +++ b/include/hw/arm/armv7m_nvic.h
> @@ -12,6 +12,7 @@
>
>  #include "target/arm/cpu.h"
>  #include "hw/sysbus.h"
> +#include "hw/timer/armv7m_systick.h"
>
>  #define TYPE_NVIC "armv7m_nvic"
>
> @@ -48,19 +49,14 @@ typedef struct NVICState {
>      unsigned int vectpending; /* highest prio pending enabled exception */
>      int exception_prio; /* group prio of the highest prio active exception */
>
> -    struct {
> -        uint32_t control;
> -        uint32_t reload;
> -        int64_t tick;
> -        QEMUTimer *timer;
> -    } systick;
> -
>      MemoryRegion sysregmem;
>      MemoryRegion container;
>
>      uint32_t num_irq;
>      qemu_irq excpout;
>      qemu_irq sysresetreq;
> +
> +    SysTickState systick;
>  } NVICState;
>
>  #endif
> diff --git a/include/hw/timer/armv7m_systick.h b/include/hw/timer/armv7m_systick.h
> new file mode 100644
> index 0000000..cca04de
> --- /dev/null
> +++ b/include/hw/timer/armv7m_systick.h
> @@ -0,0 +1,34 @@
> +/*
> + * ARMv7M SysTick timer
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Written by Paul Brook
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell
> + *
> + * This code is licensed under the GPL (version 2 or later).
> + */
> +
> +#ifndef HW_TIMER_ARMV7M_SYSTICK_H
> +#define HW_TIMER_ARMV7M_SYSTICK_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_SYSTICK "armv7m_systick"
> +
> +#define SYSTICK(obj) OBJECT_CHECK(SysTickState, (obj), TYPE_SYSTICK)
> +
> +typedef struct SysTickState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    uint32_t control;
> +    uint32_t reload;
> +    int64_t tick;
> +    QEMUTimer *timer;
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +} SysTickState;
> +
> +#endif
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index c814e16..32ffa0b 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -58,65 +58,6 @@ 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
> -
> -#define SYSTICK_ENABLE    (1 << 0)
> -#define SYSTICK_TICKINT   (1 << 1)
> -#define SYSTICK_CLKSOURCE (1 << 2)
> -#define SYSTICK_COUNTFLAG (1 << 16)
> -
> -int system_clock_scale;
> -
> -/* Conversion factor from qemu timer to SysTick frequencies.  */
> -static inline int64_t systick_scale(NVICState *s)
> -{
> -    if (s->systick.control & SYSTICK_CLKSOURCE)
> -        return system_clock_scale;
> -    else
> -        return 1000;
> -}
> -
> -static void systick_reload(NVICState *s, int reset)
> -{
> -    /* The Cortex-M3 Devices Generic User Guide says that "When the
> -     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
> -     * SYST RVR register and then counts down". So, we need to check the
> -     * ENABLE bit before reloading the value.
> -     */
> -    if ((s->systick.control & SYSTICK_ENABLE) == 0) {
> -        return;
> -    }
> -
> -    if (reset)
> -        s->systick.tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    s->systick.tick += (s->systick.reload + 1) * systick_scale(s);
> -    timer_mod(s->systick.timer, s->systick.tick);
> -}
> -
> -static void systick_timer_tick(void * opaque)
> -{
> -    NVICState *s = (NVICState *)opaque;
> -    s->systick.control |= SYSTICK_COUNTFLAG;
> -    if (s->systick.control & SYSTICK_TICKINT) {
> -        /* Trigger the interrupt.  */
> -        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
> -    }
> -    if (s->systick.reload == 0) {
> -        s->systick.control &= ~SYSTICK_ENABLE;
> -    } else {
> -        systick_reload(s, 0);
> -    }
> -}
> -
> -static void systick_reset(NVICState *s)
> -{
> -    s->systick.control = 0;
> -    s->systick.reload = 0;
> -    s->systick.tick = 0;
> -    timer_del(s->systick.timer);
> -}
> -
>  static int nvic_pending_prio(NVICState *s)
>  {
>      /* return the priority of the current pending interrupt,
> @@ -462,30 +403,6 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
>      switch (offset) {
>      case 4: /* Interrupt Control Type.  */
>          return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;
> -    case 0x10: /* SysTick Control and Status.  */
> -        val = s->systick.control;
> -        s->systick.control &= ~SYSTICK_COUNTFLAG;
> -        return val;
> -    case 0x14: /* SysTick Reload Value.  */
> -        return s->systick.reload;
> -    case 0x18: /* SysTick Current Value.  */
> -        {
> -            int64_t t;
> -            if ((s->systick.control & SYSTICK_ENABLE) == 0)
> -                return 0;
> -            t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -            if (t >= s->systick.tick)
> -                return 0;
> -            val = ((s->systick.tick - (t + 1)) / systick_scale(s)) + 1;
> -            /* The interrupt in triggered when the timer reaches zero.
> -               However the counter is not reloaded until the next clock
> -               tick.  This is a hack to return zero during the first tick.  */
> -            if (val > s->systick.reload)
> -                val = 0;
> -            return val;
> -        }
> -    case 0x1c: /* SysTick Calibration Value.  */
> -        return 10000;
>      case 0xd00: /* CPUID Base.  */
>          return cpu->midr;
>      case 0xd04: /* Interrupt Control State.  */
> @@ -620,40 +537,8 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
>  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
>  {
>      ARMCPU *cpu = s->cpu;
> -    uint32_t oldval;
> +
>      switch (offset) {
> -    case 0x10: /* SysTick Control and Status.  */
> -        oldval = s->systick.control;
> -        s->systick.control &= 0xfffffff8;
> -        s->systick.control |= value & 7;
> -        if ((oldval ^ value) & SYSTICK_ENABLE) {
> -            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -            if (value & SYSTICK_ENABLE) {
> -                if (s->systick.tick) {
> -                    s->systick.tick += now;
> -                    timer_mod(s->systick.timer, s->systick.tick);
> -                } else {
> -                    systick_reload(s, 1);
> -                }
> -            } else {
> -                timer_del(s->systick.timer);
> -                s->systick.tick -= now;
> -                if (s->systick.tick < 0)
> -                  s->systick.tick = 0;
> -            }
> -        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
> -            /* This is a hack. Force the timer to be reloaded
> -               when the reference clock is changed.  */
> -            systick_reload(s, 1);
> -        }
> -        break;
> -    case 0x14: /* SysTick Reload Value.  */
> -        s->systick.reload = value;
> -        break;
> -    case 0x18: /* SysTick Current Value.  Writes reload the timer.  */
> -        systick_reload(s, 1);
> -        s->systick.control &= ~SYSTICK_COUNTFLAG;
> -        break;
>      case 0xd04: /* Interrupt Control State.  */
>          if (value & (1 << 31)) {
>              armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI);
> @@ -952,16 +837,12 @@ static const VMStateDescription vmstate_VecInfo = {
>
>  static const VMStateDescription vmstate_nvic = {
>      .name = "armv7m_nvic",
> -    .version_id = 3,
> -    .minimum_version_id = 3,
> +    .version_id = 4,
> +    .minimum_version_id = 4,
>      .post_load = &nvic_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_STRUCT_ARRAY(vectors, NVICState, NVIC_MAX_VECTORS, 1,
>                               vmstate_VecInfo, VecInfo),
> -        VMSTATE_UINT32(systick.control, NVICState),
> -        VMSTATE_UINT32(systick.reload, NVICState),
> -        VMSTATE_INT64(systick.tick, NVICState),
> -        VMSTATE_TIMER_PTR(systick.timer, NVICState),
>          VMSTATE_UINT32(prigroup, NVICState),
>          VMSTATE_END_OF_LIST()
>      }
> @@ -999,13 +880,26 @@ static void armv7m_nvic_reset(DeviceState *dev)
>
>      s->exception_prio = NVIC_NOEXC_PRIO;
>      s->vectpending = 0;
> +}
>
> -    systick_reset(s);
> +static void nvic_systick_trigger(void *opaque, int n, int level)
> +{
> +    NVICState *s = opaque;
> +
> +    if (level) {
> +        /* SysTick just asked us to pend its exception.
> +         * (This is different from an external interrupt line's
> +         * behaviour.)
> +         */
> +        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
> +    }
>  }
>
>  static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>  {
>      NVICState *s = NVIC(dev);
> +    SysBusDevice *systick_sbd;
> +    Error *err = NULL;
>
>      s->cpu = ARM_CPU(qemu_get_cpu(0));
>      assert(s->cpu);
> @@ -1020,10 +914,19 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>      /* include space for internal exception vectors */
>      s->num_irq += NVIC_FIRST_IRQ;
>
> +    object_property_set_bool(OBJECT(&s->systick), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    systick_sbd = SYS_BUS_DEVICE(&s->systick);
> +    sysbus_connect_irq(systick_sbd, 0,
> +                       qdev_get_gpio_in_named(dev, "systick-trigger", 0));
> +
>      /* The NVIC and System Control Space (SCS) starts at 0xe000e000
>       * and looks like this:
>       *  0x004 - ICTR
> -     *  0x010 - 0x1c - systick
> +     *  0x010 - 0xff - systick
>       *  0x100..0x7ec - NVIC
>       *  0x7f0..0xcff - Reserved
>       *  0xd00..0xd3c - SCS registers
> @@ -1041,10 +944,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>      memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s,
>                            "nvic_sysregs", 0x1000);
>      memory_region_add_subregion(&s->container, 0, &s->sysregmem);
> +    memory_region_add_subregion_overlap(&s->container, 0x10,
> +                                        sysbus_mmio_get_region(systick_sbd, 0),
> +                                        1);
>
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> -
> -    s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
>  }
>
>  static void armv7m_nvic_instance_init(Object *obj)
> @@ -1059,8 +963,12 @@ static void armv7m_nvic_instance_init(Object *obj)
>      NVICState *nvic = NVIC(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> +    object_initialize(&nvic->systick, sizeof(nvic->systick), TYPE_SYSTICK);
> +    qdev_set_parent_bus(DEVICE(&nvic->systick), sysbus_get_default());
> +
>      sysbus_init_irq(sbd, &nvic->excpout);
>      qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1);
> +    qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", 1);
>  }
>
>  static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> new file mode 100644
> index 0000000..3fd5e72
> --- /dev/null
> +++ b/hw/timer/armv7m_systick.c
> @@ -0,0 +1,239 @@
> +/*
> + * ARMv7M SysTick timer
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Written by Paul Brook
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell
> + *
> + * This code is licensed under the GPL (version 2 or later).
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/timer/armv7m_systick.h"
> +#include "qemu-common.h"
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +
> +/* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
> +#define SYSTICK_SCALE 1000ULL
> +
> +#define SYSTICK_ENABLE    (1 << 0)
> +#define SYSTICK_TICKINT   (1 << 1)
> +#define SYSTICK_CLKSOURCE (1 << 2)
> +#define SYSTICK_COUNTFLAG (1 << 16)
> +
> +int system_clock_scale;
> +
> +/* Conversion factor from qemu timer to SysTick frequencies.  */
> +static inline int64_t systick_scale(SysTickState *s)
> +{
> +    if (s->control & SYSTICK_CLKSOURCE) {
> +        return system_clock_scale;
> +    } else {
> +        return 1000;

this magic seems to be SYSTICK_SCALE
(Paul Brook's commit 9ee6e8bb85)

> +    }
> +}
> +
> +static void systick_reload(SysTickState *s, int reset)
> +{
> +    /* The Cortex-M3 Devices Generic User Guide says that "When the
> +     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
> +     * SYST RVR register and then counts down". So, we need to check the
> +     * ENABLE bit before reloading the value.
> +     */
> +    trace_systick_reload();
> +
> +    if ((s->control & SYSTICK_ENABLE) == 0) {
> +        return;
> +    }
> +
> +    if (reset) {
> +        s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    }
> +    s->tick += (s->reload + 1) * systick_scale(s);
> +    timer_mod(s->timer, s->tick);
> +}
> +
> +static void systick_timer_tick(void *opaque)
> +{
> +    SysTickState *s = (SysTickState *)opaque;
> +
> +    trace_systick_timer_tick();
> +
> +    s->control |= SYSTICK_COUNTFLAG;
> +    if (s->control & SYSTICK_TICKINT) {
> +        /* Tell the NVIC to pend the SysTick exception */
> +        qemu_irq_pulse(s->irq);
> +    }
> +    if (s->reload == 0) {
> +        s->control &= ~SYSTICK_ENABLE;
> +    } else {
> +        systick_reload(s, 0);
> +    }
> +}
> +
> +static uint64_t systick_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    SysTickState *s = opaque;
> +    uint32_t val;
> +
> +    switch (addr) {
> +    case 0x0: /* SysTick Control and Status.  */
> +        val = s->control;
> +        s->control &= ~SYSTICK_COUNTFLAG;
> +        break;
> +    case 0x4: /* SysTick Reload Value.  */
> +        val = s->reload;
> +        break;
> +    case 0x8: /* SysTick Current Value.  */
> +    {
> +        int64_t t;
> +
> +        if ((s->control & SYSTICK_ENABLE) == 0) {
> +            val = 0;
> +            break;
> +        }
> +        t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        if (t >= s->tick) {
> +            val = 0;
> +            break;
> +        }
> +        val = ((s->tick - (t + 1)) / systick_scale(s)) + 1;
> +        /* The interrupt in triggered when the timer reaches zero.
> +           However the counter is not reloaded until the next clock
> +           tick.  This is a hack to return zero during the first tick.  */
> +        if (val > s->reload) {
> +            val = 0;
> +        }
> +        break;
> +    }
> +    case 0xc: /* SysTick Calibration Value.  */
> +        val = 10000;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SysTick: Bad read offset 0x%" HWADDR_PRIx "\n", addr);
> +        break;
> +    }
> +
> +    trace_systick_read(addr, val, size);
> +    return val;
> +}
> +
> +static void systick_write(void *opaque, hwaddr addr,
> +                          uint64_t value, unsigned size)
> +{
> +    SysTickState *s = opaque;
> +
> +    trace_systick_write(addr, value, size);
> +
> +    switch (addr) {
> +    case 0x0: /* SysTick Control and Status.  */
> +    {
> +        uint32_t oldval = s->control;
> +
> +        s->control &= 0xfffffff8;
> +        s->control |= value & 7;
> +        if ((oldval ^ value) & SYSTICK_ENABLE) {
> +            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +            if (value & SYSTICK_ENABLE) {
> +                if (s->tick) {
> +                    s->tick += now;
> +                    timer_mod(s->timer, s->tick);
> +                } else {
> +                    systick_reload(s, 1);
> +                }
> +            } else {
> +                timer_del(s->timer);
> +                s->tick -= now;
> +                if (s->tick < 0) {
> +                    s->tick = 0;
> +                }
> +            }
> +        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
> +            /* This is a hack. Force the timer to be reloaded
> +               when the reference clock is changed.  */
> +            systick_reload(s, 1);
> +        }
> +        break;
> +    }
> +    case 0x4: /* SysTick Reload Value.  */
> +        s->reload = value;
> +        break;
> +    case 0x8: /* SysTick Current Value.  Writes reload the timer.  */
> +        systick_reload(s, 1);
> +        s->control &= ~SYSTICK_COUNTFLAG;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SysTick: Bad write offset 0x%" HWADDR_PRIx "\n", addr);
> +    }
> +}
> +
> +static const MemoryRegionOps systick_ops = {
> +    .read = systick_read,
> +    .write = systick_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};
> +
> +static void systick_reset(DeviceState *dev)
> +{
> +    SysTickState *s = SYSTICK(dev);
> +
> +    s->control = 0;
> +    s->reload = 0;
> +    s->tick = 0;
> +    timer_del(s->timer);
> +}
> +
> +static void systick_instance_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    SysTickState *s = SYSTICK(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick", 0xe0);

It seems to me the correct size is 0x10. The manual describes the range 
0x20-0xFF as _reserved_. This _reserved_ description is at the end of 
the 'SysTick' chapter which seems weird to me... I rather think this 
range belong to the 'Interrupt Controller'@'System Control Space'.

> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
> +}
> +
> +static const VMStateDescription vmstate_systick = {
> +    .name = "armv7m_systick",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, SysTickState),
> +        VMSTATE_UINT32(reload, SysTickState),
> +        VMSTATE_INT64(tick, SysTickState),
> +        VMSTATE_TIMER_PTR(timer, SysTickState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void systick_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &vmstate_systick;
> +    dc->reset = systick_reset;
> +}
> +
> +static const TypeInfo armv7m_systick_info = {
> +    .name = TYPE_SYSTICK,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_init = systick_instance_init,
> +    .instance_size = sizeof(SysTickState),
> +    .class_init = systick_class_init,
> +};
> +
> +static void armv7m_systick_register_types(void)
> +{
> +    type_register_static(&armv7m_systick_info);
> +}
> +
> +type_init(armv7m_systick_register_types)
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index 3495c41..d17cfe6 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -49,3 +49,9 @@ aspeed_timer_ctrl_pulse_enable(uint8_t i, bool enable) "Timer %" PRIu8 ": %d"
>  aspeed_timer_set_ctrl2(uint32_t value) "Value: 0x%" PRIx32
>  aspeed_timer_set_value(int timer, int reg, uint32_t value) "Timer %d register %d: 0x%" PRIx32
>  aspeed_timer_read(uint64_t offset, unsigned size, uint64_t value) "From 0x%" PRIx64 ": of size %u: 0x%" PRIx64
> +
> +# hw/timer/armv7m_systick.c
> +systick_reload(void) "systick reload"
> +systick_timer_tick(void) "systick reload"
> +systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
> +systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 10/11] stm32f205: Create armv7m object without using armv7m_init()

[Philippe Mathieu-Daudé]

On 02/20/2017 12:36 PM, Peter Maydell wrote:
> Switch the stm32f205 SoC to create the armv7m object directly
> rather than via the armv7m_init() wrapper. This fits better
> with the SoC model's very QOMified design.
>
> In particular this means we can push loading the guest image
> out to the top level board code where it belongs, rather
> than the SoC object having a QOM property for the filename
> to load.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/stm32f205_soc.h |  4 +++-
>  hw/arm/netduino2.c             |  7 ++++---
>  hw/arm/stm32f205_soc.c         | 16 +++++++++++++---
>  3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 1332141..e2dce11 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -31,6 +31,7 @@
>  #include "hw/adc/stm32f2xx_adc.h"
>  #include "hw/or-irq.h"
>  #include "hw/ssi/stm32f2xx_spi.h"
> +#include "hw/arm/armv7m.h"
>
>  #define TYPE_STM32F205_SOC "stm32f205-soc"
>  #define STM32F205_SOC(obj) \
> @@ -51,9 +52,10 @@ typedef struct STM32F205State {
>      SysBusDevice parent_obj;
>      /*< public >*/
>
> -    char *kernel_filename;
>      char *cpu_model;
>
> +    ARMv7MState armv7m;
> +
>      STM32F2XXSyscfgState syscfg;
>      STM32F2XXUsartState usart[STM_NUM_USARTS];
>      STM32F2XXTimerState timer[STM_NUM_TIMERS];
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 23d7928..3cfe332 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -27,17 +27,18 @@
>  #include "hw/boards.h"
>  #include "qemu/error-report.h"
>  #include "hw/arm/stm32f205_soc.h"
> +#include "hw/arm/arm.h"
>
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
>      dev = qdev_create(NULL, TYPE_STM32F205_SOC);
> -    if (machine->kernel_filename) {
> -        qdev_prop_set_string(dev, "kernel-filename", machine->kernel_filename);
> -    }
>      qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
>      object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> +                       FLASH_SIZE);
>  }
>
>  static void netduino2_machine_init(MachineClass *mc)
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 38425bd..e6bd73a 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -49,6 +49,9 @@ static void stm32f205_soc_initfn(Object *obj)
>      STM32F205State *s = STM32F205_SOC(obj);
>      int i;
>
> +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
> +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
> +
>      object_initialize(&s->syscfg, sizeof(s->syscfg), TYPE_STM32F2XX_SYSCFG);
>      qdev_set_parent_bus(DEVICE(&s->syscfg), sysbus_get_default());
>
> @@ -110,8 +113,16 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      vmstate_register_ram_global(sram);
>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>
> -    nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> -                       s->kernel_filename, s->cpu_model);
> +    nvic = DEVICE(&s->armv7m);
> +    qdev_prop_set_uint32(nvic, "num-irq", 96);
> +    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
> +    object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
> +                                     "memory", &error_abort);
> +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>
>      /* System configuration controller */
>      dev = DEVICE(&s->syscfg);
> @@ -192,7 +203,6 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>  }
>
>  static Property stm32f205_soc_properties[] = {
> -    DEFINE_PROP_STRING("kernel-filename", STM32F205State, kernel_filename),
>      DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 11/11] stm32f205: Rename 'nvic' local to 'armv7m'

[Philippe Mathieu-Daudé]

On 02/20/2017 12:36 PM, Peter Maydell wrote:
> The local variable 'nvic' in stm32f205_soc_realize() no longer
> holds a direct pointer to the NVIC device; it is a pointer to
> the ARMv7M container object. Rename it 'armv7m' accordingly.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/stm32f205_soc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index e6bd73a..6e1260d 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -85,7 +85,7 @@ static void stm32f205_soc_initfn(Object *obj)
>  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>  {
>      STM32F205State *s = STM32F205_SOC(dev_soc);
> -    DeviceState *dev, *nvic;
> +    DeviceState *dev, *armv7m;
>      SysBusDevice *busdev;
>      Error *err = NULL;
>      int i;
> @@ -113,9 +113,9 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      vmstate_register_ram_global(sram);
>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>
> -    nvic = DEVICE(&s->armv7m);
> -    qdev_prop_set_uint32(nvic, "num-irq", 96);
> -    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
> +    armv7m = DEVICE(&s->armv7m);
> +    qdev_prop_set_uint32(armv7m, "num-irq", 96);
> +    qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
>      object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
>                                       "memory", &error_abort);
>      object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> @@ -133,7 +133,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      }
>      busdev = SYS_BUS_DEVICE(dev);
>      sysbus_mmio_map(busdev, 0, 0x40013800);
> -    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, 71));
> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71));
>
>      /* Attach UART (uses USART registers) and USART controllers */
>      for (i = 0; i < STM_NUM_USARTS; i++) {
> @@ -147,7 +147,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, usart_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, usart_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
>      }
>
>      /* Timer 2 to 5 */
> @@ -161,7 +161,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, timer_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, timer_irq[i]));
>      }
>
>      /* ADC 1 to 3 */
> @@ -173,7 +173,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          return;
>      }
>      qdev_connect_gpio_out(DEVICE(s->adc_irqs), 0,
> -                          qdev_get_gpio_in(nvic, ADC_IRQ));
> +                          qdev_get_gpio_in(armv7m, ADC_IRQ));
>
>      for (i = 0; i < STM_NUM_ADCS; i++) {
>          dev = DEVICE(&(s->adc[i]));
> @@ -198,7 +198,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, spi_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, spi_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, spi_irq[i]));
>      }
>  }
>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 09/11] armv7m: Split systick out from NVIC

[Peter Maydell]

On 20 February 2017 at 17:43, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/20/2017 12:36 PM, Peter Maydell wrote:
>> +/* Conversion factor from qemu timer to SysTick frequencies.  */
>> +static inline int64_t systick_scale(SysTickState *s)
>> +{
>> +    if (s->control & SYSTICK_CLKSOURCE) {
>> +        return system_clock_scale;
>> +    } else {
>> +        return 1000;
>
>
> this magic seems to be SYSTICK_SCALE
> (Paul Brook's commit 9ee6e8bb85)

This patch is just copying this function from one file to the other...
at some point we might want to try to improve the handling of
the clock scale factors but that leads into complication, because
the external-clocksource scale factor should really be board
level configurable. (Slightly confusingly the leg of this if/else
which is board-configurable is the one for the internal clock,
because we're modelling the ability of the stellaris board to
configure the PLLs so as to downclock the whole CPU, which
incidentally affects the timing of the internal systick clock.
Modelling this properly needs to wait on Fred Konrad's clocktree
patches getting in, I think.) For the moment I chose to just
leave the existing source alone, mostly.

>> +static void systick_instance_init(Object *obj)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> +    SysTickState *s = SYSTICK(obj);
>> +
>> +    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick",
>> 0xe0);
>
>
> It seems to me the correct size is 0x10. The manual describes the range
> 0x20-0xFF as _reserved_. This _reserved_ description is at the end of the
> 'SysTick' chapter which seems weird to me... I rather think this range
> belong to the 'Interrupt Controller'@'System Control Space'.

The table of the System Control Space (B3-3 in the v7M ARM ARM
rev E.b) defines the Systick space as 0xE000E010 - 0xE000E0FF,
which makes it 0xE0 bytes in size. The NVIC doesn't start
until 0xE000E100 in this table.

The table of the SysTick registers (B3-7) agrees, in that
it defines the registers as covering 0xE000E010 to 0xE000E0FF.
In the current architecture everything from 0x20..0xFF is
reserved, which just means there's nothing there, but if
there was ever a need for systick to get another register
for some reason then it would go into that space (and if
we then implemented the new register in QEMU we wouldn't need
to change the NVIC code, only the systick code).

I think the way this patch has modelled things matches the
architecture -- it's a guest error to access the gap between
0XE000E020 and 0xE000E0FF, but we should report it as an
attempt to access an invalid systick register rather than
an access to an invalid NVIC register.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH 10/11] stm32f205: Create armv7m object without using armv7m_init()

[Alistair Francis]

On Mon, Feb 20, 2017 at 9:45 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/20/2017 12:36 PM, Peter Maydell wrote:
>>
>> Switch the stm32f205 SoC to create the armv7m object directly
>> rather than via the armv7m_init() wrapper. This fits better
>> with the SoC model's very QOMified design.
>>
>> In particular this means we can push loading the guest image
>> out to the top level board code where it belongs, rather
>> than the SoC object having a QOM property for the filename
>> to load.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
>
>> ---
>>  include/hw/arm/stm32f205_soc.h |  4 +++-
>>  hw/arm/netduino2.c             |  7 ++++---
>>  hw/arm/stm32f205_soc.c         | 16 +++++++++++++---
>>  3 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/hw/arm/stm32f205_soc.h
>> b/include/hw/arm/stm32f205_soc.h
>> index 1332141..e2dce11 100644
>> --- a/include/hw/arm/stm32f205_soc.h
>> +++ b/include/hw/arm/stm32f205_soc.h
>> @@ -31,6 +31,7 @@
>>  #include "hw/adc/stm32f2xx_adc.h"
>>  #include "hw/or-irq.h"
>>  #include "hw/ssi/stm32f2xx_spi.h"
>> +#include "hw/arm/armv7m.h"
>>
>>  #define TYPE_STM32F205_SOC "stm32f205-soc"
>>  #define STM32F205_SOC(obj) \
>> @@ -51,9 +52,10 @@ typedef struct STM32F205State {
>>      SysBusDevice parent_obj;
>>      /*< public >*/
>>
>> -    char *kernel_filename;
>>      char *cpu_model;
>>
>> +    ARMv7MState armv7m;
>> +
>>      STM32F2XXSyscfgState syscfg;
>>      STM32F2XXUsartState usart[STM_NUM_USARTS];
>>      STM32F2XXTimerState timer[STM_NUM_TIMERS];
>> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
>> index 23d7928..3cfe332 100644
>> --- a/hw/arm/netduino2.c
>> +++ b/hw/arm/netduino2.c
>> @@ -27,17 +27,18 @@
>>  #include "hw/boards.h"
>>  #include "qemu/error-report.h"
>>  #include "hw/arm/stm32f205_soc.h"
>> +#include "hw/arm/arm.h"
>>
>>  static void netduino2_init(MachineState *machine)
>>  {
>>      DeviceState *dev;
>>
>>      dev = qdev_create(NULL, TYPE_STM32F205_SOC);
>> -    if (machine->kernel_filename) {
>> -        qdev_prop_set_string(dev, "kernel-filename",
>> machine->kernel_filename);
>> -    }
>>      qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
>>      object_property_set_bool(OBJECT(dev), true, "realized",
>> &error_fatal);
>> +
>> +    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
>> +                       FLASH_SIZE);
>>  }
>>
>>  static void netduino2_machine_init(MachineClass *mc)
>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> index 38425bd..e6bd73a 100644
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -49,6 +49,9 @@ static void stm32f205_soc_initfn(Object *obj)
>>      STM32F205State *s = STM32F205_SOC(obj);
>>      int i;
>>
>> +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
>> +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
>> +
>>      object_initialize(&s->syscfg, sizeof(s->syscfg),
>> TYPE_STM32F2XX_SYSCFG);
>>      qdev_set_parent_bus(DEVICE(&s->syscfg), sysbus_get_default());
>>
>> @@ -110,8 +113,16 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>      vmstate_register_ram_global(sram);
>>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>>
>> -    nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
>> -                       s->kernel_filename, s->cpu_model);
>> +    nvic = DEVICE(&s->armv7m);
>> +    qdev_prop_set_uint32(nvic, "num-irq", 96);
>> +    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
>> +    object_property_set_link(OBJECT(&s->armv7m),
>> OBJECT(get_system_memory()),
>> +                                     "memory", &error_abort);
>> +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
>> +    if (err != NULL) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>>
>>      /* System configuration controller */
>>      dev = DEVICE(&s->syscfg);
>> @@ -192,7 +203,6 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>  }
>>
>>  static Property stm32f205_soc_properties[] = {
>> -    DEFINE_PROP_STRING("kernel-filename", STM32F205State,
>> kernel_filename),
>>      DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 11/11] stm32f205: Rename 'nvic' local to 'armv7m'

[Alistair Francis]

On Mon, Feb 20, 2017 at 9:46 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/20/2017 12:36 PM, Peter Maydell wrote:
>>
>> The local variable 'nvic' in stm32f205_soc_realize() no longer
>> holds a direct pointer to the NVIC device; it is a pointer to
>> the ARMv7M container object. Rename it 'armv7m' accordingly.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
>
>> ---
>>  hw/arm/stm32f205_soc.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> index e6bd73a..6e1260d 100644
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -85,7 +85,7 @@ static void stm32f205_soc_initfn(Object *obj)
>>  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>  {
>>      STM32F205State *s = STM32F205_SOC(dev_soc);
>> -    DeviceState *dev, *nvic;
>> +    DeviceState *dev, *armv7m;
>>      SysBusDevice *busdev;
>>      Error *err = NULL;
>>      int i;
>> @@ -113,9 +113,9 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>      vmstate_register_ram_global(sram);
>>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>>
>> -    nvic = DEVICE(&s->armv7m);
>> -    qdev_prop_set_uint32(nvic, "num-irq", 96);
>> -    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
>> +    armv7m = DEVICE(&s->armv7m);
>> +    qdev_prop_set_uint32(armv7m, "num-irq", 96);
>> +    qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
>>      object_property_set_link(OBJECT(&s->armv7m),
>> OBJECT(get_system_memory()),
>>                                       "memory", &error_abort);
>>      object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
>> @@ -133,7 +133,7 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>      }
>>      busdev = SYS_BUS_DEVICE(dev);
>>      sysbus_mmio_map(busdev, 0, 0x40013800);
>> -    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, 71));
>> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71));
>>
>>      /* Attach UART (uses USART registers) and USART controllers */
>>      for (i = 0; i < STM_NUM_USARTS; i++) {
>> @@ -147,7 +147,7 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>          }
>>          busdev = SYS_BUS_DEVICE(dev);
>>          sysbus_mmio_map(busdev, 0, usart_addr[i]);
>> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic,
>> usart_irq[i]));
>> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m,
>> usart_irq[i]));
>>      }
>>
>>      /* Timer 2 to 5 */
>> @@ -161,7 +161,7 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>          }
>>          busdev = SYS_BUS_DEVICE(dev);
>>          sysbus_mmio_map(busdev, 0, timer_addr[i]);
>> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic,
>> timer_irq[i]));
>> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m,
>> timer_irq[i]));
>>      }
>>
>>      /* ADC 1 to 3 */
>> @@ -173,7 +173,7 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>          return;
>>      }
>>      qdev_connect_gpio_out(DEVICE(s->adc_irqs), 0,
>> -                          qdev_get_gpio_in(nvic, ADC_IRQ));
>> +                          qdev_get_gpio_in(armv7m, ADC_IRQ));
>>
>>      for (i = 0; i < STM_NUM_ADCS; i++) {
>>          dev = DEVICE(&(s->adc[i]));
>> @@ -198,7 +198,7 @@ static void stm32f205_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>>          }
>>          busdev = SYS_BUS_DEVICE(dev);
>>          sysbus_mmio_map(busdev, 0, spi_addr[i]);
>> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic,
>> spi_irq[i]));
>> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m,
>> spi_irq[i]));
>>      }
>>  }
>>
>>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/11] armv7m: Abstract out the "load kernel" code

[Alistair Francis]

On Mon, Feb 20, 2017 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/20/2017 12:35 PM, Peter Maydell wrote:
>>
>> Abstract the "load kernel" code out of armv7m_init() into its own
>> function.  This includes the registration of the CPU reset function,
>> to parallel how we handle this for A profile cores.
>>
>> We make the function public so that boards which choose to
>> directly instantiate an ARMv7M device object can call it.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
>
>> ---
>>  include/hw/arm/arm.h | 12 ++++++++++++
>>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----
>>  2 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
>> index c175c0e..a3f79d3 100644
>> --- a/include/hw/arm/arm.h
>> +++ b/include/hw/arm/arm.h
>> @@ -26,6 +26,18 @@ typedef enum {
>>  /* armv7m.c */
>>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int
>> num_irq,
>>                        const char *kernel_filename, const char
>> *cpu_model);
>> +/**
>> + * armv7m_load_kernel:
>> + * @cpu: CPU
>> + * @kernel_filename: file to load
>> + * @mem_size: mem_size: maximum image size to load
>> + *
>> + * Load the guest image for an ARMv7M system. This must be called by
>> + * any ARMv7M board, either directly or via armv7m_init(). (This is
>> + * necessary to ensure that the CPU resets correctly on system reset,
>> + * as well as for kernel loading.)
>> + */
>> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int
>> mem_size);
>>
>>  /*
>>   * struct used as a parameter of the arm_load_kernel machine init
>> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
>> index 0c9ca7b..b2cc6e9 100644
>> --- a/hw/arm/armv7m.c
>> +++ b/hw/arm/armv7m.c
>> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,
>> int mem_size, int num_irq,
>>      ARMCPU *cpu;
>>      CPUARMState *env;
>>      DeviceState *nvic;
>> -    int image_size;
>> -    uint64_t entry;
>> -    uint64_t lowaddr;
>> -    int big_endian;
>>
>>      if (cpu_model == NULL) {
>>         cpu_model = "cortex-m3";
>> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,
>> int mem_size, int num_irq,
>>      qdev_init_nofail(nvic);
>>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
>>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
>> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);
>> +    return nvic;
>> +}
>> +
>> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int
>> mem_size)
>> +{
>> +    int image_size;
>> +    uint64_t entry;
>> +    uint64_t lowaddr;
>> +    int big_endian;
>>
>>  #ifdef TARGET_WORDS_BIGENDIAN
>>      big_endian = 1;
>> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,
>> int mem_size, int num_irq,
>>          }
>>      }
>>
>> +    /* CPU objects (unlike devices) are not automatically reset on system
>> +     * reset, so we must always register a handler to do so. Unlike
>> +     * A-profile CPUs, we don't need to do anything special in the
>> +     * handler to arrange that it starts correctly.
>> +     * This is arguably the wrong place to do this, but it matches the
>> +     * way A-profile does it. Note that this means that every M profile
>> +     * board must call this function!
>> +     */
>>      qemu_register_reset(armv7m_reset, cpu);
>> -    return nvic;
>>  }
>>
>>  static Property bitband_properties[] = {
>>
>

Re: [Qemu-devel] [PATCH 04/11] armv7m: Use QOMified armv7m object in armv7m_init()

[Alistair Francis]

On Mon, Feb 20, 2017 at 7:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Make the legacy armv7m_init() function use the newly QOMified
> armv7m object rather than doing everything by hand.
>
> We can return the armv7m object rather than the NVIC from
> armv7m_init() because its interface to the rest of the
> board (GPIOs, etc) is identical.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  hw/arm/armv7m.c | 49 ++++++++++++-------------------------------------
>  1 file changed, 12 insertions(+), 37 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 56d02d4..36f213c 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -131,21 +131,6 @@ static void bitband_init(Object *obj)
>      sysbus_init_mmio(dev, &s->iomem);
>  }
>
> -static void armv7m_bitband_init(void)
> -{
> -    DeviceState *dev;
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x20000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x22000000);
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x40000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x42000000);
> -}
> -
>  /* Board init.  */
>
>  static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> @@ -283,35 +268,25 @@ static void armv7m_reset(void *opaque)
>
>  /* Init CPU and memory for a v7-M based board.
>     mem_size is in bytes.
> -   Returns the NVIC array.  */
> +   Returns the ARMv7M device.  */
>
>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>                        const char *kernel_filename, const char *cpu_model)
>  {
> -    ARMCPU *cpu;
> -    CPUARMState *env;
> -    DeviceState *nvic;
> +    DeviceState *armv7m;
>
>      if (cpu_model == NULL) {
> -       cpu_model = "cortex-m3";
> +        cpu_model = "cortex-m3";
>      }
> -    cpu = cpu_arm_init(cpu_model);
> -    if (cpu == NULL) {
> -        fprintf(stderr, "Unable to find CPU definition\n");
> -        exit(1);
> -    }
> -    env = &cpu->env;
> -
> -    armv7m_bitband_init();
> -
> -    nvic = qdev_create(NULL, "armv7m_nvic");
> -    qdev_prop_set_uint32(nvic, "num-irq", num_irq);
> -    env->nvic = nvic;
> -    qdev_init_nofail(nvic);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
> -                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
> -    armv7m_load_kernel(cpu, kernel_filename, mem_size);
> -    return nvic;
> +
> +    armv7m = qdev_create(NULL, "armv7m");
> +    qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
> +    qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
> +    /* This will exit with an error if the user passed us a bad cpu_model */
> +    qdev_init_nofail(armv7m);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), kernel_filename, mem_size);
> +    return armv7m;
>  }
>
>  void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
> --
> 2.7.4
>

Re: [Qemu-devel] [PATCH 09/11] armv7m: Split systick out from NVIC

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The SysTick timer isn't really part of the NVIC proper;
> we just modelled it that way back when we couldn't
> easily have devices that only occupied a small chunk
> of a memory region. Split it out into its own device.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/timer/Makefile.objs            |   1 +
>  include/hw/arm/armv7m_nvic.h      |  10 +-
>  include/hw/timer/armv7m_systick.h |  34 ++++++
>  hw/intc/armv7m_nvic.c             | 160 ++++++-------------------
>  hw/timer/armv7m_systick.c         | 239 ++++++++++++++++++++++++++++++++++++++
>  hw/timer/trace-events             |   6 +
>  6 files changed, 317 insertions(+), 133 deletions(-)
>  create mode 100644 include/hw/timer/armv7m_systick.h
>  create mode 100644 hw/timer/armv7m_systick.c
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index fc99668..dd6f27e 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -1,5 +1,6 @@
>  common-obj-$(CONFIG_ARM_TIMER) += arm_timer.o
>  common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> +common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o
>  common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
>  common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
>  common-obj-$(CONFIG_DS1338) += ds1338.o
> diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
> index 39b94ee..1d145fb 100644
> --- a/include/hw/arm/armv7m_nvic.h
> +++ b/include/hw/arm/armv7m_nvic.h
> @@ -12,6 +12,7 @@
>
>  #include "target/arm/cpu.h"
>  #include "hw/sysbus.h"
> +#include "hw/timer/armv7m_systick.h"
>
>  #define TYPE_NVIC "armv7m_nvic"
>
> @@ -48,19 +49,14 @@ typedef struct NVICState {
>      unsigned int vectpending; /* highest prio pending enabled exception */
>      int exception_prio; /* group prio of the highest prio active exception */
>
> -    struct {
> -        uint32_t control;
> -        uint32_t reload;
> -        int64_t tick;
> -        QEMUTimer *timer;
> -    } systick;
> -
>      MemoryRegion sysregmem;
>      MemoryRegion container;
>
>      uint32_t num_irq;
>      qemu_irq excpout;
>      qemu_irq sysresetreq;
> +
> +    SysTickState systick;
>  } NVICState;
>
>  #endif
> diff --git a/include/hw/timer/armv7m_systick.h b/include/hw/timer/armv7m_systick.h
> new file mode 100644
> index 0000000..cca04de
> --- /dev/null
> +++ b/include/hw/timer/armv7m_systick.h
> @@ -0,0 +1,34 @@
> +/*
> + * ARMv7M SysTick timer
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Written by Paul Brook
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell
> + *
> + * This code is licensed under the GPL (version 2 or later).
> + */
> +
> +#ifndef HW_TIMER_ARMV7M_SYSTICK_H
> +#define HW_TIMER_ARMV7M_SYSTICK_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_SYSTICK "armv7m_systick"
> +
> +#define SYSTICK(obj) OBJECT_CHECK(SysTickState, (obj), TYPE_SYSTICK)
> +
> +typedef struct SysTickState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    uint32_t control;
> +    uint32_t reload;
> +    int64_t tick;
> +    QEMUTimer *timer;
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +} SysTickState;
> +
> +#endif
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index c814e16..32ffa0b 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -58,65 +58,6 @@ 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
> -
> -#define SYSTICK_ENABLE    (1 << 0)
> -#define SYSTICK_TICKINT   (1 << 1)
> -#define SYSTICK_CLKSOURCE (1 << 2)
> -#define SYSTICK_COUNTFLAG (1 << 16)
> -
> -int system_clock_scale;
> -
> -/* Conversion factor from qemu timer to SysTick frequencies.  */
> -static inline int64_t systick_scale(NVICState *s)
> -{
> -    if (s->systick.control & SYSTICK_CLKSOURCE)
> -        return system_clock_scale;
> -    else
> -        return 1000;
> -}
> -
> -static void systick_reload(NVICState *s, int reset)
> -{
> -    /* The Cortex-M3 Devices Generic User Guide says that "When the
> -     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
> -     * SYST RVR register and then counts down". So, we need to check the
> -     * ENABLE bit before reloading the value.
> -     */
> -    if ((s->systick.control & SYSTICK_ENABLE) == 0) {
> -        return;
> -    }
> -
> -    if (reset)
> -        s->systick.tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    s->systick.tick += (s->systick.reload + 1) * systick_scale(s);
> -    timer_mod(s->systick.timer, s->systick.tick);
> -}
> -
> -static void systick_timer_tick(void * opaque)
> -{
> -    NVICState *s = (NVICState *)opaque;
> -    s->systick.control |= SYSTICK_COUNTFLAG;
> -    if (s->systick.control & SYSTICK_TICKINT) {
> -        /* Trigger the interrupt.  */
> -        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
> -    }
> -    if (s->systick.reload == 0) {
> -        s->systick.control &= ~SYSTICK_ENABLE;
> -    } else {
> -        systick_reload(s, 0);
> -    }
> -}
> -
> -static void systick_reset(NVICState *s)
> -{
> -    s->systick.control = 0;
> -    s->systick.reload = 0;
> -    s->systick.tick = 0;
> -    timer_del(s->systick.timer);
> -}
> -
>  static int nvic_pending_prio(NVICState *s)
>  {
>      /* return the priority of the current pending interrupt,
> @@ -462,30 +403,6 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
>      switch (offset) {
>      case 4: /* Interrupt Control Type.  */
>          return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;
> -    case 0x10: /* SysTick Control and Status.  */
> -        val = s->systick.control;
> -        s->systick.control &= ~SYSTICK_COUNTFLAG;
> -        return val;
> -    case 0x14: /* SysTick Reload Value.  */
> -        return s->systick.reload;
> -    case 0x18: /* SysTick Current Value.  */
> -        {
> -            int64_t t;
> -            if ((s->systick.control & SYSTICK_ENABLE) == 0)
> -                return 0;
> -            t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -            if (t >= s->systick.tick)
> -                return 0;
> -            val = ((s->systick.tick - (t + 1)) / systick_scale(s)) + 1;
> -            /* The interrupt in triggered when the timer reaches zero.
> -               However the counter is not reloaded until the next clock
> -               tick.  This is a hack to return zero during the first tick.  */
> -            if (val > s->systick.reload)
> -                val = 0;
> -            return val;
> -        }
> -    case 0x1c: /* SysTick Calibration Value.  */
> -        return 10000;
>      case 0xd00: /* CPUID Base.  */
>          return cpu->midr;
>      case 0xd04: /* Interrupt Control State.  */
> @@ -620,40 +537,8 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
>  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
>  {
>      ARMCPU *cpu = s->cpu;
> -    uint32_t oldval;
> +
>      switch (offset) {
> -    case 0x10: /* SysTick Control and Status.  */
> -        oldval = s->systick.control;
> -        s->systick.control &= 0xfffffff8;
> -        s->systick.control |= value & 7;
> -        if ((oldval ^ value) & SYSTICK_ENABLE) {
> -            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -            if (value & SYSTICK_ENABLE) {
> -                if (s->systick.tick) {
> -                    s->systick.tick += now;
> -                    timer_mod(s->systick.timer, s->systick.tick);
> -                } else {
> -                    systick_reload(s, 1);
> -                }
> -            } else {
> -                timer_del(s->systick.timer);
> -                s->systick.tick -= now;
> -                if (s->systick.tick < 0)
> -                  s->systick.tick = 0;
> -            }
> -        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
> -            /* This is a hack. Force the timer to be reloaded
> -               when the reference clock is changed.  */
> -            systick_reload(s, 1);
> -        }
> -        break;
> -    case 0x14: /* SysTick Reload Value.  */
> -        s->systick.reload = value;
> -        break;
> -    case 0x18: /* SysTick Current Value.  Writes reload the timer.  */
> -        systick_reload(s, 1);
> -        s->systick.control &= ~SYSTICK_COUNTFLAG;
> -        break;
>      case 0xd04: /* Interrupt Control State.  */
>          if (value & (1 << 31)) {
>              armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI);
> @@ -952,16 +837,12 @@ static const VMStateDescription vmstate_VecInfo = {
>
>  static const VMStateDescription vmstate_nvic = {
>      .name = "armv7m_nvic",
> -    .version_id = 3,
> -    .minimum_version_id = 3,
> +    .version_id = 4,
> +    .minimum_version_id = 4,
>      .post_load = &nvic_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_STRUCT_ARRAY(vectors, NVICState, NVIC_MAX_VECTORS, 1,
>                               vmstate_VecInfo, VecInfo),
> -        VMSTATE_UINT32(systick.control, NVICState),
> -        VMSTATE_UINT32(systick.reload, NVICState),
> -        VMSTATE_INT64(systick.tick, NVICState),
> -        VMSTATE_TIMER_PTR(systick.timer, NVICState),
>          VMSTATE_UINT32(prigroup, NVICState),
>          VMSTATE_END_OF_LIST()
>      }
> @@ -999,13 +880,26 @@ static void armv7m_nvic_reset(DeviceState *dev)
>
>      s->exception_prio = NVIC_NOEXC_PRIO;
>      s->vectpending = 0;
> +}
>
> -    systick_reset(s);
> +static void nvic_systick_trigger(void *opaque, int n, int level)
> +{
> +    NVICState *s = opaque;
> +
> +    if (level) {
> +        /* SysTick just asked us to pend its exception.
> +         * (This is different from an external interrupt line's
> +         * behaviour.)
> +         */
> +        armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
> +    }
>  }
>
>  static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>  {
>      NVICState *s = NVIC(dev);
> +    SysBusDevice *systick_sbd;
> +    Error *err = NULL;
>
>      s->cpu = ARM_CPU(qemu_get_cpu(0));
>      assert(s->cpu);
> @@ -1020,10 +914,19 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>      /* include space for internal exception vectors */
>      s->num_irq += NVIC_FIRST_IRQ;
>
> +    object_property_set_bool(OBJECT(&s->systick), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    systick_sbd = SYS_BUS_DEVICE(&s->systick);
> +    sysbus_connect_irq(systick_sbd, 0,
> +                       qdev_get_gpio_in_named(dev, "systick-trigger", 0));
> +
>      /* The NVIC and System Control Space (SCS) starts at 0xe000e000
>       * and looks like this:
>       *  0x004 - ICTR
> -     *  0x010 - 0x1c - systick
> +     *  0x010 - 0xff - systick
>       *  0x100..0x7ec - NVIC
>       *  0x7f0..0xcff - Reserved
>       *  0xd00..0xd3c - SCS registers
> @@ -1041,10 +944,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>      memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s,
>                            "nvic_sysregs", 0x1000);
>      memory_region_add_subregion(&s->container, 0, &s->sysregmem);
> +    memory_region_add_subregion_overlap(&s->container, 0x10,
> +                                        sysbus_mmio_get_region(systick_sbd, 0),
> +                                        1);
>
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> -
> -    s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
>  }
>
>  static void armv7m_nvic_instance_init(Object *obj)
> @@ -1059,8 +963,12 @@ static void armv7m_nvic_instance_init(Object *obj)
>      NVICState *nvic = NVIC(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> +    object_initialize(&nvic->systick, sizeof(nvic->systick), TYPE_SYSTICK);
> +    qdev_set_parent_bus(DEVICE(&nvic->systick), sysbus_get_default());
> +
>      sysbus_init_irq(sbd, &nvic->excpout);
>      qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1);
> +    qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", 1);
>  }
>
>  static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> new file mode 100644
> index 0000000..3fd5e72
> --- /dev/null
> +++ b/hw/timer/armv7m_systick.c
> @@ -0,0 +1,239 @@
> +/*
> + * ARMv7M SysTick timer
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Written by Paul Brook
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell
> + *
> + * This code is licensed under the GPL (version 2 or later).
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/timer/armv7m_systick.h"
> +#include "qemu-common.h"
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +
> +/* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
> +#define SYSTICK_SCALE 1000ULL
> +
> +#define SYSTICK_ENABLE    (1 << 0)
> +#define SYSTICK_TICKINT   (1 << 1)
> +#define SYSTICK_CLKSOURCE (1 << 2)
> +#define SYSTICK_COUNTFLAG (1 << 16)
> +
> +int system_clock_scale;
> +
> +/* Conversion factor from qemu timer to SysTick frequencies.  */
> +static inline int64_t systick_scale(SysTickState *s)
> +{
> +    if (s->control & SYSTICK_CLKSOURCE) {
> +        return system_clock_scale;
> +    } else {
> +        return 1000;
> +    }
> +}
> +
> +static void systick_reload(SysTickState *s, int reset)
> +{
> +    /* The Cortex-M3 Devices Generic User Guide says that "When the
> +     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
> +     * SYST RVR register and then counts down". So, we need to check the
> +     * ENABLE bit before reloading the value.
> +     */
> +    trace_systick_reload();
> +
> +    if ((s->control & SYSTICK_ENABLE) == 0) {
> +        return;
> +    }
> +
> +    if (reset) {
> +        s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    }
> +    s->tick += (s->reload + 1) * systick_scale(s);
> +    timer_mod(s->timer, s->tick);
> +}
> +
> +static void systick_timer_tick(void *opaque)
> +{
> +    SysTickState *s = (SysTickState *)opaque;
> +
> +    trace_systick_timer_tick();
> +
> +    s->control |= SYSTICK_COUNTFLAG;
> +    if (s->control & SYSTICK_TICKINT) {
> +        /* Tell the NVIC to pend the SysTick exception */
> +        qemu_irq_pulse(s->irq);
> +    }
> +    if (s->reload == 0) {
> +        s->control &= ~SYSTICK_ENABLE;
> +    } else {
> +        systick_reload(s, 0);
> +    }
> +}
> +
> +static uint64_t systick_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    SysTickState *s = opaque;
> +    uint32_t val;
> +
> +    switch (addr) {
> +    case 0x0: /* SysTick Control and Status.  */
> +        val = s->control;
> +        s->control &= ~SYSTICK_COUNTFLAG;
> +        break;
> +    case 0x4: /* SysTick Reload Value.  */
> +        val = s->reload;
> +        break;
> +    case 0x8: /* SysTick Current Value.  */
> +    {
> +        int64_t t;
> +
> +        if ((s->control & SYSTICK_ENABLE) == 0) {
> +            val = 0;
> +            break;
> +        }
> +        t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        if (t >= s->tick) {
> +            val = 0;
> +            break;
> +        }
> +        val = ((s->tick - (t + 1)) / systick_scale(s)) + 1;
> +        /* The interrupt in triggered when the timer reaches zero.
> +           However the counter is not reloaded until the next clock
> +           tick.  This is a hack to return zero during the first tick.  */
> +        if (val > s->reload) {
> +            val = 0;
> +        }
> +        break;
> +    }
> +    case 0xc: /* SysTick Calibration Value.  */
> +        val = 10000;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SysTick: Bad read offset 0x%" HWADDR_PRIx "\n", addr);
> +        break;
> +    }
> +
> +    trace_systick_read(addr, val, size);

I'm getting the compiler complain:

hw/timer/armv7m_systick.c: In function ‘systick_read’:
hw/timer/armv7m_systick.c:81:14: error: ‘val’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     uint32_t val;
              ^
cc1: all warnings being treated as errors

I suspect the trace function could pick up garbage if we take the
GUEST_ERROR leg and default out....


> +    return val;
> +}
> +
> +static void systick_write(void *opaque, hwaddr addr,
> +                          uint64_t value, unsigned size)
> +{
> +    SysTickState *s = opaque;
> +
> +    trace_systick_write(addr, value, size);
> +
> +    switch (addr) {
> +    case 0x0: /* SysTick Control and Status.  */
> +    {
> +        uint32_t oldval = s->control;
> +
> +        s->control &= 0xfffffff8;
> +        s->control |= value & 7;
> +        if ((oldval ^ value) & SYSTICK_ENABLE) {
> +            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +            if (value & SYSTICK_ENABLE) {
> +                if (s->tick) {
> +                    s->tick += now;
> +                    timer_mod(s->timer, s->tick);
> +                } else {
> +                    systick_reload(s, 1);
> +                }
> +            } else {
> +                timer_del(s->timer);
> +                s->tick -= now;
> +                if (s->tick < 0) {
> +                    s->tick = 0;
> +                }
> +            }
> +        } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
> +            /* This is a hack. Force the timer to be reloaded
> +               when the reference clock is changed.  */
> +            systick_reload(s, 1);
> +        }
> +        break;
> +    }
> +    case 0x4: /* SysTick Reload Value.  */
> +        s->reload = value;
> +        break;
> +    case 0x8: /* SysTick Current Value.  Writes reload the timer.  */
> +        systick_reload(s, 1);
> +        s->control &= ~SYSTICK_COUNTFLAG;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SysTick: Bad write offset 0x%" HWADDR_PRIx "\n", addr);
> +    }
> +}
> +
> +static const MemoryRegionOps systick_ops = {
> +    .read = systick_read,
> +    .write = systick_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};
> +
> +static void systick_reset(DeviceState *dev)
> +{
> +    SysTickState *s = SYSTICK(dev);
> +
> +    s->control = 0;
> +    s->reload = 0;
> +    s->tick = 0;
> +    timer_del(s->timer);
> +}
> +
> +static void systick_instance_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    SysTickState *s = SYSTICK(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick", 0xe0);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
> +}
> +
> +static const VMStateDescription vmstate_systick = {
> +    .name = "armv7m_systick",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, SysTickState),
> +        VMSTATE_UINT32(reload, SysTickState),
> +        VMSTATE_INT64(tick, SysTickState),
> +        VMSTATE_TIMER_PTR(timer, SysTickState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void systick_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &vmstate_systick;
> +    dc->reset = systick_reset;
> +}
> +
> +static const TypeInfo armv7m_systick_info = {
> +    .name = TYPE_SYSTICK,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_init = systick_instance_init,
> +    .instance_size = sizeof(SysTickState),
> +    .class_init = systick_class_init,
> +};
> +
> +static void armv7m_systick_register_types(void)
> +{
> +    type_register_static(&armv7m_systick_info);
> +}
> +
> +type_init(armv7m_systick_register_types)
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index 3495c41..d17cfe6 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -49,3 +49,9 @@ aspeed_timer_ctrl_pulse_enable(uint8_t i, bool enable) "Timer %" PRIu8 ": %d"
>  aspeed_timer_set_ctrl2(uint32_t value) "Value: 0x%" PRIx32
>  aspeed_timer_set_value(int timer, int reg, uint32_t value) "Timer %d register %d: 0x%" PRIx32
>  aspeed_timer_read(uint64_t offset, unsigned size, uint64_t value) "From 0x%" PRIx64 ": of size %u: 0x%" PRIx64
> +
> +# hw/timer/armv7m_systick.c
> +systick_reload(void) "systick reload"
> +systick_timer_tick(void) "systick reload"
> +systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
> +systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"


--
Alex Bennée

Re: [Qemu-devel] [PATCH 09/11] armv7m: Split systick out from NVIC

[Peter Maydell]

On 24 February 2017 at 14:29, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> The SysTick timer isn't really part of the NVIC proper;
>> we just modelled it that way back when we couldn't
>> easily have devices that only occupied a small chunk
>> of a memory region. Split it out into its own device.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "SysTick: Bad read offset 0x%" HWADDR_PRIx "\n", addr);
>> +        break;
>> +    }
>> +
>> +    trace_systick_read(addr, val, size);
>
> I'm getting the compiler complain:
>
> hw/timer/armv7m_systick.c: In function ‘systick_read’:
> hw/timer/armv7m_systick.c:81:14: error: ‘val’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      uint32_t val;
>               ^
> cc1: all warnings being treated as errors
>
> I suspect the trace function could pick up garbage if we take the
> GUEST_ERROR leg and default out....

Indeed, missing 'val = 0;' in the default case.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 03/11] armv7m: QOMify the armv7m container

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Create a proper QOM object for the armv7m container, which
> holds the CPU, the NVIC and the bitband regions.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armv7m.h |  51 ++++++++++++++++++
>  hw/arm/armv7m.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 179 insertions(+), 12 deletions(-)
>  create mode 100644 include/hw/arm/armv7m.h
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> new file mode 100644
> index 0000000..193ad71
> --- /dev/null
> +++ b/include/hw/arm/armv7m.h
> @@ -0,0 +1,51 @@
> +/*
> + * ARMv7M CPU object
> + *
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This code is licensed under the GPL version 2 or later.
> + */
> +
> +#ifndef HW_ARM_ARMV7M_H
> +#define HW_ARM_ARMV7M_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/armv7m_nvic.h"
> +
> +#define TYPE_BITBAND "ARM,bitband-memory"
> +#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t base;
> +} BitBandState;
> +
> +#define TYPE_ARMV7M "armv7m"
> +#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
> +
> +#define ARMV7M_NUM_BITBANDS 2
> +
> +/* ARMv7M container object.
> + * + Unnamed GPIO input lines: external IRQ lines for the NVIC
> + * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
> + * + Property "cpu-model": CPU model to instantiate
> + * + Property "num-irq": number of external IRQ lines
> + */
> +typedef struct ARMv7MState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +    NVICState nvic;
> +    BitBandState bitband[ARMV7M_NUM_BITBANDS];
> +    ARMCPU *cpu;
> +
> +    /* Properties */
> +    char *cpu_model;
> +} ARMv7MState;
> +
> +#endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index b2cc6e9..56d02d4 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -8,6 +8,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "hw/arm/armv7m.h"
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> @@ -120,18 +121,6 @@ static const MemoryRegionOps bitband_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -#define TYPE_BITBAND "ARM,bitband-memory"
> -#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> -
> -typedef struct {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    MemoryRegion iomem;
> -    uint32_t base;
> -} BitBandState;
> -
>  static void bitband_init(Object *obj)
>  {
>      BitBandState *s = BITBAND(obj);
> @@ -159,6 +148,132 @@ static void armv7m_bitband_init(void)
>
>  /* Board init.  */
>
> +static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x20000000, 0x40000000
> +};
> +
> +static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x22000000, 0x42000000
> +};
> +
> +static void armv7m_instance_init(Object *obj)
> +{
> +    ARMv7MState *s = ARMV7M(obj);
> +    int i;
> +
> +    /* Can't init the cpu here, we don't yet know which model to use */
> +
> +    object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
> +    qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
> +    object_property_add_alias(obj, "num-irq",
> +                              OBJECT(&s->nvic), "num-irq", &error_abort);
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        object_initialize(&s->bitband[i], sizeof(s->bitband[i]), TYPE_BITBAND);
> +        qdev_set_parent_bus(DEVICE(&s->bitband[i]), sysbus_get_default());
> +    }
> +}
> +
> +static void armv7m_realize(DeviceState *dev, Error **errp)
> +{
> +    /* here realize our children */

This comment is a little zen-like. Does this mean the child devices are
realised at this point when the armv7m realise code is run?

> +    ARMv7MState *s = ARMV7M(dev);
> +    Error *err = NULL;
> +    int i;
> +    char **cpustr;
> +    ObjectClass *oc;
> +    const char *typename;
> +    CPUClass *cc;
> +
> +    cpustr = g_strsplit(s->cpu_model, ",", 2);
> +
> +    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> +    if (!oc) {
> +        error_setg(errp, "Unknown CPU model %s", cpustr[0]);
> +        g_strfreev(cpustr);
> +        return;
> +    }
> +
> +    cc = CPU_CLASS(oc);
> +    typename = object_class_get_name(oc);
> +    cc->parse_features(typename, cpustr[1], &err);
> +    g_strfreev(cpustr);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    s->cpu = ARM_CPU(object_new(typename));
> +    if (!s->cpu) {
> +        error_setg(errp, "Unknown CPU model %s", s->cpu_model);
> +        return;
> +    }
> +
> +    object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Note that we must realize the NVIC after the CPU */
> +    object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Alias the NVIC's input and output GPIOs as our own so the board
> +     * code can wire them up. (We do this in realize because the
> +     * NVIC doesn't create the input GPIO array until realize.)
> +     */
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> +
> +    /* Wire the NVIC up to the CPU */
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
> +                       qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
> +    s->cpu->env.nvic = &s->nvic;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        Object *obj = OBJECT(&s->bitband[i]);
> +        SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> +
> +        object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        object_property_set_bool(obj, true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
> +    }
> +}
> +
> +static Property armv7m_properties[] = {
> +    DEFINE_PROP_STRING("cpu-model", ARMv7MState, cpu_model),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void armv7m_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = armv7m_realize;
> +    dc->props = armv7m_properties;
> +}
> +
> +static const TypeInfo armv7m_info = {
> +    .name = TYPE_ARMV7M,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(ARMv7MState),
> +    .instance_init = armv7m_instance_init,
> +    .class_init = armv7m_class_init,
> +};
> +
>  static void armv7m_reset(void *opaque)
>  {
>      ARMCPU *cpu = opaque;
> @@ -264,6 +379,7 @@ static const TypeInfo bitband_info = {
>  static void armv7m_register_types(void)
>  {
>      type_register_static(&bitband_info);
> +    type_register_static(&armv7m_info);
>  }
>
>  type_init(armv7m_register_types)

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée

Re: [Qemu-devel] [PATCH 01/11] armv7m: Abstract out the "load kernel" code

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Abstract the "load kernel" code out of armv7m_init() into its own
> function.  This includes the registration of the CPU reset function,
> to parallel how we handle this for A profile cores.
>
> We make the function public so that boards which choose to
> directly instantiate an ARMv7M device object can call it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/arm.h | 12 ++++++++++++
>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----
>  2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index c175c0e..a3f79d3 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -26,6 +26,18 @@ typedef enum {
>  /* armv7m.c */
>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>                        const char *kernel_filename, const char *cpu_model);
> +/**
> + * armv7m_load_kernel:
> + * @cpu: CPU
> + * @kernel_filename: file to load
> + * @mem_size: mem_size: maximum image size to load
> + *
> + * Load the guest image for an ARMv7M system. This must be called by
> + * any ARMv7M board, either directly or via armv7m_init(). (This is
> + * necessary to ensure that the CPU resets correctly on system reset,
> + * as well as for kernel loading.)
> + */
> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);
>
>  /*
>   * struct used as a parameter of the arm_load_kernel machine init
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 0c9ca7b..b2cc6e9 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>      ARMCPU *cpu;
>      CPUARMState *env;
>      DeviceState *nvic;
> -    int image_size;
> -    uint64_t entry;
> -    uint64_t lowaddr;
> -    int big_endian;
>
>      if (cpu_model == NULL) {
>  	cpu_model = "cortex-m3";
> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>      qdev_init_nofail(nvic);
>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);
> +    return nvic;
> +}
> +
> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
> +{
> +    int image_size;
> +    uint64_t entry;
> +    uint64_t lowaddr;
> +    int big_endian;
>
>  #ifdef TARGET_WORDS_BIGENDIAN
>      big_endian = 1;
> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>          }
>      }
>
> +    /* CPU objects (unlike devices) are not automatically reset on system
> +     * reset, so we must always register a handler to do so. Unlike
> +     * A-profile CPUs, we don't need to do anything special in the
> +     * handler to arrange that it starts correctly.
> +     * This is arguably the wrong place to do this, but it matches the
> +     * way A-profile does it. Note that this means that every M profile
> +     * board must call this function!
> +     */
>      qemu_register_reset(armv7m_reset, cpu);
> -    return nvic;
>  }
>
>  static Property bitband_properties[] = {


--
Alex Bennée

Re: [Qemu-devel] [PATCH 02/11] armv7m: Move NVICState struct definition into header

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Move the NVICState struct definition into a header, so we can
> embed it into other QOM objects like SoCs.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/armv7m_nvic.h | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/intc/armv7m_nvic.c        | 49 +-------------------------------
>  2 files changed, 67 insertions(+), 48 deletions(-)
>  create mode 100644 include/hw/arm/armv7m_nvic.h
>
> diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h
> new file mode 100644
> index 0000000..39b94ee
> --- /dev/null
> +++ b/include/hw/arm/armv7m_nvic.h
> @@ -0,0 +1,66 @@
> +/*
> + * ARMv7M NVIC object
> + *
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This code is licensed under the GPL version 2 or later.
> + */
> +
> +#ifndef HW_ARM_ARMV7M_NVIC_H
> +#define HW_ARM_ARMV7M_NVIC_H
> +
> +#include "target/arm/cpu.h"
> +#include "hw/sysbus.h"
> +
> +#define TYPE_NVIC "armv7m_nvic"
> +
> +#define NVIC(obj) \
> +    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
> +
> +/* Highest permitted number of exceptions (architectural limit) */
> +#define NVIC_MAX_VECTORS 512
> +
> +typedef struct VecInfo {
> +    /* Exception priorities can range from -3 to 255; only the unmodifiable
> +     * priority values for RESET, NMI and HardFault can be negative.
> +     */
> +    int16_t prio;
> +    uint8_t enabled;
> +    uint8_t pending;
> +    uint8_t active;
> +    uint8_t level; /* exceptions <=15 never set level */
> +} VecInfo;
> +
> +typedef struct NVICState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    ARMCPU *cpu;
> +
> +    VecInfo vectors[NVIC_MAX_VECTORS];
> +    uint32_t prigroup;
> +
> +    /* vectpending and exception_prio are both cached state that can
> +     * be recalculated from the vectors[] array and the prigroup field.
> +     */
> +    unsigned int vectpending; /* highest prio pending enabled exception */
> +    int exception_prio; /* group prio of the highest prio active exception */
> +
> +    struct {
> +        uint32_t control;
> +        uint32_t reload;
> +        int64_t tick;
> +        QEMUTimer *timer;
> +    } systick;
> +
> +    MemoryRegion sysregmem;
> +    MemoryRegion container;
> +
> +    uint32_t num_irq;
> +    qemu_irq excpout;
> +    qemu_irq sysresetreq;
> +} NVICState;
> +
> +#endif
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 76097b4..f2ada39 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -17,6 +17,7 @@
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "hw/arm/arm.h"
> +#include "hw/arm/armv7m_nvic.h"
>  #include "target/arm/cpu.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/log.h"
> @@ -47,7 +48,6 @@
>   * "exception" more or less interchangeably.
>   */
>  #define NVIC_FIRST_IRQ 16
> -#define NVIC_MAX_VECTORS 512
>  #define NVIC_MAX_IRQ (NVIC_MAX_VECTORS - NVIC_FIRST_IRQ)
>
>  /* Effective running priority of the CPU when no exception is active
> @@ -55,53 +55,6 @@
>   */
>  #define NVIC_NOEXC_PRIO 0x100
>
> -typedef struct VecInfo {
> -    /* Exception priorities can range from -3 to 255; only the unmodifiable
> -     * priority values for RESET, NMI and HardFault can be negative.
> -     */
> -    int16_t prio;
> -    uint8_t enabled;
> -    uint8_t pending;
> -    uint8_t active;
> -    uint8_t level; /* exceptions <=15 never set level */
> -} VecInfo;
> -
> -typedef struct NVICState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    ARMCPU *cpu;
> -
> -    VecInfo vectors[NVIC_MAX_VECTORS];
> -    uint32_t prigroup;
> -
> -    /* vectpending and exception_prio are both cached state that can
> -     * be recalculated from the vectors[] array and the prigroup field.
> -     */
> -    unsigned int vectpending; /* highest prio pending enabled exception */
> -    int exception_prio; /* group prio of the highest prio active exception */
> -
> -    struct {
> -        uint32_t control;
> -        uint32_t reload;
> -        int64_t tick;
> -        QEMUTimer *timer;
> -    } systick;
> -
> -    MemoryRegion sysregmem;
> -    MemoryRegion container;
> -
> -    uint32_t num_irq;
> -    qemu_irq excpout;
> -    qemu_irq sysresetreq;
> -} NVICState;
> -
> -#define TYPE_NVIC "armv7m_nvic"
> -
> -#define NVIC(obj) \
> -    OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
> -
>  static const uint8_t nvic_id[] = {
>      0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
>  };


--
Alex Bennée

Re: [Qemu-devel] [PATCH 04/11] armv7m: Use QOMified armv7m object in armv7m_init()

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Make the legacy armv7m_init() function use the newly QOMified
> armv7m object rather than doing everything by hand.
>
> We can return the armv7m object rather than the NVIC from
> armv7m_init() because its interface to the rest of the
> board (GPIOs, etc) is identical.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/arm/armv7m.c | 49 ++++++++++++-------------------------------------
>  1 file changed, 12 insertions(+), 37 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 56d02d4..36f213c 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -131,21 +131,6 @@ static void bitband_init(Object *obj)
>      sysbus_init_mmio(dev, &s->iomem);
>  }
>
> -static void armv7m_bitband_init(void)
> -{
> -    DeviceState *dev;
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x20000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x22000000);
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x40000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x42000000);
> -}
> -
>  /* Board init.  */
>
>  static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> @@ -283,35 +268,25 @@ static void armv7m_reset(void *opaque)
>
>  /* Init CPU and memory for a v7-M based board.
>     mem_size is in bytes.
> -   Returns the NVIC array.  */
> +   Returns the ARMv7M device.  */
>
>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>                        const char *kernel_filename, const char *cpu_model)
>  {
> -    ARMCPU *cpu;
> -    CPUARMState *env;
> -    DeviceState *nvic;
> +    DeviceState *armv7m;
>
>      if (cpu_model == NULL) {
> -	cpu_model = "cortex-m3";
> +        cpu_model = "cortex-m3";
>      }
> -    cpu = cpu_arm_init(cpu_model);
> -    if (cpu == NULL) {
> -        fprintf(stderr, "Unable to find CPU definition\n");
> -        exit(1);
> -    }
> -    env = &cpu->env;
> -
> -    armv7m_bitband_init();
> -
> -    nvic = qdev_create(NULL, "armv7m_nvic");
> -    qdev_prop_set_uint32(nvic, "num-irq", num_irq);
> -    env->nvic = nvic;
> -    qdev_init_nofail(nvic);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
> -                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
> -    armv7m_load_kernel(cpu, kernel_filename, mem_size);
> -    return nvic;
> +
> +    armv7m = qdev_create(NULL, "armv7m");
> +    qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
> +    qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
> +    /* This will exit with an error if the user passed us a bad cpu_model */
> +    qdev_init_nofail(armv7m);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), kernel_filename, mem_size);
> +    return armv7m;
>  }
>
>  void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 05/11] armv7m: Make ARMv7M object take memory region link

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Make the ARMv7M object take a memory region link which it uses
> to wire up the bitband rather than having them always put
> themselves in the system address space.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/armv7m.h | 10 ++++++++++
>  hw/arm/armv7m.c         | 23 ++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index 193ad71..3333c91 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -35,6 +35,9 @@ typedef struct {
>   * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
>   * + Property "cpu-model": CPU model to instantiate
>   * + Property "num-irq": number of external IRQ lines
> + * + Property "memory": MemoryRegion defining the physical address space
> + *   that CPU accesses see. (The NVIC, bitbanding and other CPU-internal
> + *   devices will be automatically layered on top of this view.)
>   */
>  typedef struct ARMv7MState {
>      /*< private >*/
> @@ -44,8 +47,15 @@ typedef struct ARMv7MState {
>      BitBandState bitband[ARMV7M_NUM_BITBANDS];
>      ARMCPU *cpu;
>
> +    /* MemoryRegion we pass to the CPU, with our devices layered on
> +     * top of the ones the board provides in board_memory.
> +     */
> +    MemoryRegion container;
> +
>      /* Properties */
>      char *cpu_model;
> +    /* MemoryRegion the board provides to us (with its devices, RAM, etc) */
> +    MemoryRegion *board_memory;
>  } ARMv7MState;
>
>  #endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 36f213c..638c597 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -18,6 +18,7 @@
>  #include "elf.h"
>  #include "sysemu/qtest.h"
>  #include "qemu/error-report.h"
> +#include "exec/address-spaces.h"
>
>  /* Bitbanded IO.  Each word corresponds to a single bit.  */
>
> @@ -148,6 +149,14 @@ static void armv7m_instance_init(Object *obj)
>
>      /* Can't init the cpu here, we don't yet know which model to use */
>
> +    object_property_add_link(obj, "memory",
> +                             TYPE_MEMORY_REGION,
> +                             (Object **)&s->board_memory,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
> +                             &error_abort);
> +    memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX);
> +
>      object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
>      qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
>      object_property_add_alias(obj, "num-irq",
> @@ -170,6 +179,13 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>      const char *typename;
>      CPUClass *cc;
>
> +    if (!s->board_memory) {
> +        error_setg(errp, "memory property was not set");
> +        return;
> +    }
> +
> +    memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
> +
>      cpustr = g_strsplit(s->cpu_model, ",", 2);
>
>      oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> @@ -194,6 +210,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
> +                             &error_abort);
>      object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
> @@ -234,7 +252,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>
> -        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
> +        memory_region_add_subregion(&s->container, bitband_output_addr[i],
> +                                    sysbus_mmio_get_region(sbd, 0));
>      }
>  }
>
> @@ -282,6 +301,8 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>      armv7m = qdev_create(NULL, "armv7m");
>      qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
>      qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
> +    object_property_set_link(OBJECT(armv7m), OBJECT(get_system_memory()),
> +                                     "memory", &error_abort);
>      /* This will exit with an error if the user passed us a bad cpu_model */
>      qdev_init_nofail(armv7m);


--
Alex Bennée

Re: [Qemu-devel] [PATCH 06/11] armv7m: Make NVIC expose a memory region rather than mapping itself

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Make the NVIC device expose a memory region for its users
> to map, rather than mapping itself into the system memory
> space on realize, and get the one user (the ARMv7M object)
> to do this.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/arm/armv7m.c       | 7 ++++++-
>  hw/intc/armv7m_nvic.c | 7 ++-----
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 638c597..fb21f74 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -172,6 +172,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>  {
>      /* here realize our children */
>      ARMv7MState *s = ARMV7M(dev);
> +    SysBusDevice *sbd;
>      Error *err = NULL;
>      int i;
>      char **cpustr;
> @@ -233,10 +234,14 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>      qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
>
>      /* Wire the NVIC up to the CPU */
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
> +    sbd = SYS_BUS_DEVICE(&s->nvic);
> +    sysbus_connect_irq(sbd, 0,
>                         qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
>      s->cpu->env.nvic = &s->nvic;
>
> +    memory_region_add_subregion(&s->container, 0xe000e000,
> +                                sysbus_mmio_get_region(sbd, 0));
> +
>      for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
>          Object *obj = OBJECT(&s->bitband[i]);
>          SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index f2ada39..c814e16 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -19,7 +19,6 @@
>  #include "hw/arm/arm.h"
>  #include "hw/arm/armv7m_nvic.h"
>  #include "target/arm/cpu.h"
> -#include "exec/address-spaces.h"
>  #include "qemu/log.h"
>  #include "trace.h"
>
> @@ -1043,10 +1042,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>                            "nvic_sysregs", 0x1000);
>      memory_region_add_subregion(&s->container, 0, &s->sysregmem);
>
> -    /* 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);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> +
>      s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH 03/11] armv7m: QOMify the armv7m container

[Peter Maydell]

On 28 February 2017 at 13:56, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>> +static void armv7m_realize(DeviceState *dev, Error **errp)
>> +{
>> +    /* here realize our children */
>
> This comment is a little zen-like. Does this mean the child devices are
> realised at this point when the armv7m realise code is run?

Yeah, it just means "we init child devices in init, and realize
them in realize", but it's a bit cryptic. I have a feeling it
was a placeholder comment I put in when I was writing this
as a "put this code here" note. I'll delete it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 07/11] armv7m: Make bitband device take the address space to access

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Instead of the bitband device doing a cpu_physical_memory_read/write,
> make it take a MemoryRegion which specifies where it should be
> accessing, and use address_space_read/write to access the
> corresponding AddressSpace.
>
> Since this entails pretty much a rewrite, convert away from
> old_mmio in the process.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/armv7m.h |   2 +
>  hw/arm/armv7m.c         | 166 +++++++++++++++++++++++-------------------------
>  2 files changed, 81 insertions(+), 87 deletions(-)
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index 3333c91..a9b3f2a 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -21,8 +21,10 @@ typedef struct {
>      SysBusDevice parent_obj;
>      /*< public >*/
>
> +    AddressSpace *source_as;
>      MemoryRegion iomem;
>      uint32_t base;
> +    MemoryRegion *source_memory;
>  } BitBandState;
>
>  #define TYPE_ARMV7M "armv7m"
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index fb21f74..4481106 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -23,103 +23,73 @@
>  /* Bitbanded IO.  Each word corresponds to a single bit.  */
>
>  /* Get the byte address of the real memory for a bitband access.  */
> -static inline uint32_t bitband_addr(void * opaque, uint32_t addr)
> +static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
>  {
> -    uint32_t res;
> -
> -    res = *(uint32_t *)opaque;
> -    res |= (addr & 0x1ffffff) >> 5;
> -    return res;
> -
> -}
> -
> -static uint32_t bitband_readb(void *opaque, hwaddr offset)
> -{
> -    uint8_t v;
> -    cpu_physical_memory_read(bitband_addr(opaque, offset), &v, 1);
> -    return (v & (1 << ((offset >> 2) & 7))) != 0;
> -}
> -
> -static void bitband_writeb(void *opaque, hwaddr offset,
> -                           uint32_t value)
> -{
> -    uint32_t addr;
> -    uint8_t mask;
> -    uint8_t v;
> -    addr = bitband_addr(opaque, offset);
> -    mask = (1 << ((offset >> 2) & 7));
> -    cpu_physical_memory_read(addr, &v, 1);
> -    if (value & 1)
> -        v |= mask;
> -    else
> -        v &= ~mask;
> -    cpu_physical_memory_write(addr, &v, 1);
> -}
> -
> -static uint32_t bitband_readw(void *opaque, hwaddr offset)
> -{
> -    uint32_t addr;
> -    uint16_t mask;
> -    uint16_t v;
> -    addr = bitband_addr(opaque, offset) & ~1;
> -    mask = (1 << ((offset >> 2) & 15));
> -    mask = tswap16(mask);
> -    cpu_physical_memory_read(addr, &v, 2);
> -    return (v & mask) != 0;
> -}
> -
> -static void bitband_writew(void *opaque, hwaddr offset,
> -                           uint32_t value)
> -{
> -    uint32_t addr;
> -    uint16_t mask;
> -    uint16_t v;
> -    addr = bitband_addr(opaque, offset) & ~1;
> -    mask = (1 << ((offset >> 2) & 15));
> -    mask = tswap16(mask);
> -    cpu_physical_memory_read(addr, &v, 2);
> -    if (value & 1)
> -        v |= mask;
> -    else
> -        v &= ~mask;
> -    cpu_physical_memory_write(addr, &v, 2);
> +    return s->base | (offset & 0x1ffffff) >> 5;
>  }
>
> -static uint32_t bitband_readl(void *opaque, hwaddr offset)
> +static MemTxResult bitband_read(void *opaque, hwaddr offset,
> +                                uint64_t *data, unsigned size, MemTxAttrs attrs)
>  {
> -    uint32_t addr;
> -    uint32_t mask;
> -    uint32_t v;
> -    addr = bitband_addr(opaque, offset) & ~3;
> -    mask = (1 << ((offset >> 2) & 31));
> -    mask = tswap32(mask);
> -    cpu_physical_memory_read(addr, &v, 4);
> -    return (v & mask) != 0;
> +    BitBandState *s = opaque;
> +    uint8_t buf[4];
> +    MemTxResult res;
> +    int bitpos, bit;
> +    hwaddr addr;
> +
> +    assert(size <= 4);
> +
> +    /* Find address in underlying memory and round down to multiple of size */
> +    addr = bitband_addr(s, offset) & (-size);
> +    res = address_space_read(s->source_as, addr, attrs, buf, size);
> +    if (res) {
> +        return res;
> +    }
> +    /* Bit position in the N bytes read... */
> +    bitpos = (offset >> 2) & ((size * 8) - 1);
> +    /* ...converted to byte in buffer and bit in byte */
> +    bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
> +    *data = bit;
> +    return MEMTX_OK;
>  }
>
> -static void bitband_writel(void *opaque, hwaddr offset,
> -                           uint32_t value)
> +static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
> +                                 unsigned size, MemTxAttrs attrs)
>  {
> -    uint32_t addr;
> -    uint32_t mask;
> -    uint32_t v;
> -    addr = bitband_addr(opaque, offset) & ~3;
> -    mask = (1 << ((offset >> 2) & 31));
> -    mask = tswap32(mask);
> -    cpu_physical_memory_read(addr, &v, 4);
> -    if (value & 1)
> -        v |= mask;
> -    else
> -        v &= ~mask;
> -    cpu_physical_memory_write(addr, &v, 4);
> +    BitBandState *s = opaque;
> +    uint8_t buf[4];
> +    MemTxResult res;
> +    int bitpos, bit;
> +    hwaddr addr;
> +
> +    assert(size <= 4);
> +
> +    /* Find address in underlying memory and round down to multiple of size */
> +    addr = bitband_addr(s, offset) & (-size);
> +    res = address_space_read(s->source_as, addr, attrs, buf, size);
> +    if (res) {
> +        return res;
> +    }
> +    /* Bit position in the N bytes read... */
> +    bitpos = (offset >> 2) & ((size * 8) - 1);
> +    /* ...converted to byte in buffer and bit in byte */
> +    bit = 1 << (bitpos & 7);
> +    if (value & 1) {
> +        buf[bitpos >> 3] |= bit;
> +    } else {
> +        buf[bitpos >> 3] &= ~bit;
> +    }
> +    return address_space_write(s->source_as, addr, attrs, buf, size);
>  }
>
>  static const MemoryRegionOps bitband_ops = {
> -    .old_mmio = {
> -        .read = { bitband_readb, bitband_readw, bitband_readl, },
> -        .write = { bitband_writeb, bitband_writew, bitband_writel, },
> -    },
> +    .read_with_attrs = bitband_read,
> +    .write_with_attrs = bitband_write,
>      .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl.min_access_size = 1,
> +    .impl.max_access_size = 4,
> +    .valid.min_access_size = 1,
> +    .valid.max_access_size = 4,
>  };
>
>  static void bitband_init(Object *obj)
> @@ -127,11 +97,30 @@ static void bitband_init(Object *obj)
>      BitBandState *s = BITBAND(obj);
>      SysBusDevice *dev = SYS_BUS_DEVICE(obj);
>
> -    memory_region_init_io(&s->iomem, obj, &bitband_ops, &s->base,
> +    object_property_add_link(obj, "source-memory",
> +                             TYPE_MEMORY_REGION,
> +                             (Object **)&s->source_memory,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
> +                             &error_abort);
> +    memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
>                            "bitband", 0x02000000);
>      sysbus_init_mmio(dev, &s->iomem);
>  }
>
> +static void bitband_realize(DeviceState *dev, Error **errp)
> +{
> +    BitBandState *s = BITBAND(dev);
> +
> +    if (!s->source_memory) {
> +        error_setg(errp, "source-memory property not set");
> +        return;
> +    }
> +
> +    s->source_as = address_space_init_shareable(s->source_memory,
> +                                                "bitband-source");
> +}
> +
>  /* Board init.  */
>
>  static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> @@ -251,6 +240,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>              error_propagate(errp, err);
>              return;
>          }
> +        object_property_set_link(obj, OBJECT(s->board_memory),
> +                                 "source-memory", &error_abort);
>          object_property_set_bool(obj, true, "realized", &err);
>          if (err != NULL) {
>              error_propagate(errp, err);
> @@ -366,6 +357,7 @@ static void bitband_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
> +    dc->realize = bitband_realize;
>      dc->props = bitband_properties;
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH 08/11] armv7m: Don't put core v7M devices under CONFIG_STELLARIS

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The NVIC is a core v7M device that exists for all v7M CPUs;
> put it under a CONFIG_ARM_V7M rather than hiding it under
> CONFIG_STELLARIS.
>
> (We'll use CONFIG_ARM_V7M for the SysTick device too
> when we split it out of the NVIC.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/intc/Makefile.objs           | 2 +-
>  default-configs/arm-softmmu.mak | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 8948106..adedd0d 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -24,7 +24,7 @@ obj-$(CONFIG_APIC) += apic.o apic_common.o
>  obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
>  obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
>  obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_its_kvm.o
> -obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
> +obj-$(CONFIG_ARM_V7M) += armv7m_nvic.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
>  obj-$(CONFIG_GRLIB) += grlib_irqmp.o
>  obj-$(CONFIG_IOAPIC) += ioapic.o
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index fdf4089..1e3bd2b 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -42,6 +42,8 @@ CONFIG_ARM11MPCORE=y
>  CONFIG_A9MPCORE=y
>  CONFIG_A15MPCORE=y
>
> +CONFIG_ARM_V7M=y
> +
>  CONFIG_ARM_GIC=y
>  CONFIG_ARM_GIC_KVM=$(CONFIG_KVM)
>  CONFIG_ARM_TIMER=y


--
Alex Bennée

Re: [Qemu-devel] [PATCH 10/11] stm32f205: Create armv7m object without using armv7m_init()

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Switch the stm32f205 SoC to create the armv7m object directly
> rather than via the armv7m_init() wrapper. This fits better
> with the SoC model's very QOMified design.
>
> In particular this means we can push loading the guest image
> out to the top level board code where it belongs, rather
> than the SoC object having a QOM property for the filename
> to load.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/stm32f205_soc.h |  4 +++-
>  hw/arm/netduino2.c             |  7 ++++---
>  hw/arm/stm32f205_soc.c         | 16 +++++++++++++---
>  3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 1332141..e2dce11 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -31,6 +31,7 @@
>  #include "hw/adc/stm32f2xx_adc.h"
>  #include "hw/or-irq.h"
>  #include "hw/ssi/stm32f2xx_spi.h"
> +#include "hw/arm/armv7m.h"
>
>  #define TYPE_STM32F205_SOC "stm32f205-soc"
>  #define STM32F205_SOC(obj) \
> @@ -51,9 +52,10 @@ typedef struct STM32F205State {
>      SysBusDevice parent_obj;
>      /*< public >*/
>
> -    char *kernel_filename;
>      char *cpu_model;
>
> +    ARMv7MState armv7m;
> +
>      STM32F2XXSyscfgState syscfg;
>      STM32F2XXUsartState usart[STM_NUM_USARTS];
>      STM32F2XXTimerState timer[STM_NUM_TIMERS];
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 23d7928..3cfe332 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -27,17 +27,18 @@
>  #include "hw/boards.h"
>  #include "qemu/error-report.h"
>  #include "hw/arm/stm32f205_soc.h"
> +#include "hw/arm/arm.h"
>
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
>      dev = qdev_create(NULL, TYPE_STM32F205_SOC);
> -    if (machine->kernel_filename) {
> -        qdev_prop_set_string(dev, "kernel-filename", machine->kernel_filename);
> -    }
>      qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
>      object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> +                       FLASH_SIZE);
>  }
>
>  static void netduino2_machine_init(MachineClass *mc)
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 38425bd..e6bd73a 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -49,6 +49,9 @@ static void stm32f205_soc_initfn(Object *obj)
>      STM32F205State *s = STM32F205_SOC(obj);
>      int i;
>
> +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
> +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
> +
>      object_initialize(&s->syscfg, sizeof(s->syscfg), TYPE_STM32F2XX_SYSCFG);
>      qdev_set_parent_bus(DEVICE(&s->syscfg), sysbus_get_default());
>
> @@ -110,8 +113,16 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      vmstate_register_ram_global(sram);
>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>
> -    nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> -                       s->kernel_filename, s->cpu_model);
> +    nvic = DEVICE(&s->armv7m);
> +    qdev_prop_set_uint32(nvic, "num-irq", 96);
> +    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
> +    object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
> +                                     "memory", &error_abort);
> +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>
>      /* System configuration controller */
>      dev = DEVICE(&s->syscfg);
> @@ -192,7 +203,6 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>  }
>
>  static Property stm32f205_soc_properties[] = {
> -    DEFINE_PROP_STRING("kernel-filename", STM32F205State, kernel_filename),
>      DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
>      DEFINE_PROP_END_OF_LIST(),
>  };


--
Alex Bennée

Re: [Qemu-devel] [PATCH 11/11] stm32f205: Rename 'nvic' local to 'armv7m'

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The local variable 'nvic' in stm32f205_soc_realize() no longer
> holds a direct pointer to the NVIC device; it is a pointer to
> the ARMv7M container object. Rename it 'armv7m' accordingly.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/arm/stm32f205_soc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index e6bd73a..6e1260d 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -85,7 +85,7 @@ static void stm32f205_soc_initfn(Object *obj)
>  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>  {
>      STM32F205State *s = STM32F205_SOC(dev_soc);
> -    DeviceState *dev, *nvic;
> +    DeviceState *dev, *armv7m;
>      SysBusDevice *busdev;
>      Error *err = NULL;
>      int i;
> @@ -113,9 +113,9 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      vmstate_register_ram_global(sram);
>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>
> -    nvic = DEVICE(&s->armv7m);
> -    qdev_prop_set_uint32(nvic, "num-irq", 96);
> -    qdev_prop_set_string(nvic, "cpu-model", s->cpu_model);
> +    armv7m = DEVICE(&s->armv7m);
> +    qdev_prop_set_uint32(armv7m, "num-irq", 96);
> +    qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
>      object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
>                                       "memory", &error_abort);
>      object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> @@ -133,7 +133,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>      }
>      busdev = SYS_BUS_DEVICE(dev);
>      sysbus_mmio_map(busdev, 0, 0x40013800);
> -    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, 71));
> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71));
>
>      /* Attach UART (uses USART registers) and USART controllers */
>      for (i = 0; i < STM_NUM_USARTS; i++) {
> @@ -147,7 +147,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, usart_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, usart_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
>      }
>
>      /* Timer 2 to 5 */
> @@ -161,7 +161,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, timer_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, timer_irq[i]));
>      }
>
>      /* ADC 1 to 3 */
> @@ -173,7 +173,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          return;
>      }
>      qdev_connect_gpio_out(DEVICE(s->adc_irqs), 0,
> -                          qdev_get_gpio_in(nvic, ADC_IRQ));
> +                          qdev_get_gpio_in(armv7m, ADC_IRQ));
>
>      for (i = 0; i < STM_NUM_ADCS; i++) {
>          dev = DEVICE(&(s->adc[i]));
> @@ -198,7 +198,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>          busdev = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(busdev, 0, spi_addr[i]);
> -        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, spi_irq[i]));
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, spi_irq[i]));
>      }
>  }


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH 03/11] armv7m: QOMify the armv7m container

[Philippe Mathieu-Daudé]

On 02/20/2017 12:35 PM, Peter Maydell wrote:
> Create a proper QOM object for the armv7m container, which
> holds the CPU, the NVIC and the bitband regions.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/armv7m.h |  51 ++++++++++++++++++
>  hw/arm/armv7m.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 179 insertions(+), 12 deletions(-)
>  create mode 100644 include/hw/arm/armv7m.h
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> new file mode 100644
> index 0000000..193ad71
> --- /dev/null
> +++ b/include/hw/arm/armv7m.h
> @@ -0,0 +1,51 @@
> +/*
> + * ARMv7M CPU object
> + *
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This code is licensed under the GPL version 2 or later.
> + */
> +
> +#ifndef HW_ARM_ARMV7M_H
> +#define HW_ARM_ARMV7M_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/armv7m_nvic.h"
> +
> +#define TYPE_BITBAND "ARM,bitband-memory"
> +#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t base;
> +} BitBandState;
> +
> +#define TYPE_ARMV7M "armv7m"
> +#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
> +
> +#define ARMV7M_NUM_BITBANDS 2
> +
> +/* ARMv7M container object.
> + * + Unnamed GPIO input lines: external IRQ lines for the NVIC
> + * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
> + * + Property "cpu-model": CPU model to instantiate
> + * + Property "num-irq": number of external IRQ lines
> + */
> +typedef struct ARMv7MState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +    NVICState nvic;
> +    BitBandState bitband[ARMV7M_NUM_BITBANDS];
> +    ARMCPU *cpu;
> +
> +    /* Properties */
> +    char *cpu_model;
> +} ARMv7MState;
> +
> +#endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index b2cc6e9..56d02d4 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -8,6 +8,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "hw/arm/armv7m.h"
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> @@ -120,18 +121,6 @@ static const MemoryRegionOps bitband_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -#define TYPE_BITBAND "ARM,bitband-memory"
> -#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> -
> -typedef struct {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    MemoryRegion iomem;
> -    uint32_t base;
> -} BitBandState;
> -
>  static void bitband_init(Object *obj)
>  {
>      BitBandState *s = BITBAND(obj);
> @@ -159,6 +148,132 @@ static void armv7m_bitband_init(void)
>
>  /* Board init.  */
>
> +static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x20000000, 0x40000000
> +};
> +
> +static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x22000000, 0x42000000
> +};
> +
> +static void armv7m_instance_init(Object *obj)
> +{
> +    ARMv7MState *s = ARMV7M(obj);
> +    int i;
> +
> +    /* Can't init the cpu here, we don't yet know which model to use */
> +
> +    object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
> +    qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
> +    object_property_add_alias(obj, "num-irq",
> +                              OBJECT(&s->nvic), "num-irq", &error_abort);
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        object_initialize(&s->bitband[i], sizeof(s->bitband[i]), TYPE_BITBAND);
> +        qdev_set_parent_bus(DEVICE(&s->bitband[i]), sysbus_get_default());
> +    }
> +}
> +
> +static void armv7m_realize(DeviceState *dev, Error **errp)
> +{
> +    /* here realize our children */
> +    ARMv7MState *s = ARMV7M(dev);
> +    Error *err = NULL;
> +    int i;
> +    char **cpustr;
> +    ObjectClass *oc;
> +    const char *typename;
> +    CPUClass *cc;
> +
> +    cpustr = g_strsplit(s->cpu_model, ",", 2);
> +
> +    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> +    if (!oc) {
> +        error_setg(errp, "Unknown CPU model %s", cpustr[0]);
> +        g_strfreev(cpustr);
> +        return;
> +    }
> +
> +    cc = CPU_CLASS(oc);
> +    typename = object_class_get_name(oc);
> +    cc->parse_features(typename, cpustr[1], &err);
> +    g_strfreev(cpustr);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    s->cpu = ARM_CPU(object_new(typename));
> +    if (!s->cpu) {
> +        error_setg(errp, "Unknown CPU model %s", s->cpu_model);
> +        return;
> +    }
> +
> +    object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Note that we must realize the NVIC after the CPU */
> +    object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Alias the NVIC's input and output GPIOs as our own so the board
> +     * code can wire them up. (We do this in realize because the
> +     * NVIC doesn't create the input GPIO array until realize.)
> +     */
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> +
> +    /* Wire the NVIC up to the CPU */
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
> +                       qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
> +    s->cpu->env.nvic = &s->nvic;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        Object *obj = OBJECT(&s->bitband[i]);
> +        SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> +
> +        object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        object_property_set_bool(obj, true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
> +    }
> +}
> +
> +static Property armv7m_properties[] = {
> +    DEFINE_PROP_STRING("cpu-model", ARMv7MState, cpu_model),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void armv7m_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = armv7m_realize;
> +    dc->props = armv7m_properties;
> +}
> +
> +static const TypeInfo armv7m_info = {
> +    .name = TYPE_ARMV7M,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(ARMv7MState),
> +    .instance_init = armv7m_instance_init,
> +    .class_init = armv7m_class_init,
> +};
> +
>  static void armv7m_reset(void *opaque)
>  {
>      ARMCPU *cpu = opaque;
> @@ -264,6 +379,7 @@ static const TypeInfo bitband_info = {
>  static void armv7m_register_types(void)
>  {
>      type_register_static(&bitband_info);
> +    type_register_static(&armv7m_info);
>  }
>
>  type_init(armv7m_register_types)
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 04/11] armv7m: Use QOMified armv7m object in armv7m_init()

[Philippe Mathieu-Daudé]

On 02/20/2017 12:35 PM, Peter Maydell wrote:
> Make the legacy armv7m_init() function use the newly QOMified
> armv7m object rather than doing everything by hand.
>
> We can return the armv7m object rather than the NVIC from
> armv7m_init() because its interface to the rest of the
> board (GPIOs, etc) is identical.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/armv7m.c | 49 ++++++++++++-------------------------------------
>  1 file changed, 12 insertions(+), 37 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 56d02d4..36f213c 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -131,21 +131,6 @@ static void bitband_init(Object *obj)
>      sysbus_init_mmio(dev, &s->iomem);
>  }
>
> -static void armv7m_bitband_init(void)
> -{
> -    DeviceState *dev;
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x20000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x22000000);
> -
> -    dev = qdev_create(NULL, TYPE_BITBAND);
> -    qdev_prop_set_uint32(dev, "base", 0x40000000);
> -    qdev_init_nofail(dev);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x42000000);
> -}
> -
>  /* Board init.  */
>
>  static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> @@ -283,35 +268,25 @@ static void armv7m_reset(void *opaque)
>
>  /* Init CPU and memory for a v7-M based board.
>     mem_size is in bytes.
> -   Returns the NVIC array.  */
> +   Returns the ARMv7M device.  */
>
>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>                        const char *kernel_filename, const char *cpu_model)
>  {
> -    ARMCPU *cpu;
> -    CPUARMState *env;
> -    DeviceState *nvic;
> +    DeviceState *armv7m;
>
>      if (cpu_model == NULL) {
> -	cpu_model = "cortex-m3";
> +        cpu_model = "cortex-m3";
>      }
> -    cpu = cpu_arm_init(cpu_model);
> -    if (cpu == NULL) {
> -        fprintf(stderr, "Unable to find CPU definition\n");
> -        exit(1);
> -    }
> -    env = &cpu->env;
> -
> -    armv7m_bitband_init();
> -
> -    nvic = qdev_create(NULL, "armv7m_nvic");
> -    qdev_prop_set_uint32(nvic, "num-irq", num_irq);
> -    env->nvic = nvic;
> -    qdev_init_nofail(nvic);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
> -                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
> -    armv7m_load_kernel(cpu, kernel_filename, mem_size);
> -    return nvic;
> +
> +    armv7m = qdev_create(NULL, "armv7m");
> +    qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
> +    qdev_prop_set_string(armv7m, "cpu-model", cpu_model);
> +    /* This will exit with an error if the user passed us a bad cpu_model */
> +    qdev_init_nofail(armv7m);
> +
> +    armv7m_load_kernel(ARM_CPU(first_cpu), kernel_filename, mem_size);
> +    return armv7m;
>  }
>
>  void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
>

Re: [Qemu-devel] [PATCH 06/11] armv7m: Make NVIC expose a memory region rather than mapping itself

[Philippe Mathieu-Daudé]

On 02/20/2017 12:36 PM, Peter Maydell wrote:
> Make the NVIC device expose a memory region for its users
> to map, rather than mapping itself into the system memory
> space on realize, and get the one user (the ARMv7M object)
> to do this.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/armv7m.c       | 7 ++++++-
>  hw/intc/armv7m_nvic.c | 7 ++-----
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 638c597..fb21f74 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -172,6 +172,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>  {
>      /* here realize our children */
>      ARMv7MState *s = ARMV7M(dev);
> +    SysBusDevice *sbd;
>      Error *err = NULL;
>      int i;
>      char **cpustr;
> @@ -233,10 +234,14 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>      qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
>
>      /* Wire the NVIC up to the CPU */
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
> +    sbd = SYS_BUS_DEVICE(&s->nvic);
> +    sysbus_connect_irq(sbd, 0,
>                         qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
>      s->cpu->env.nvic = &s->nvic;
>
> +    memory_region_add_subregion(&s->container, 0xe000e000,
> +                                sysbus_mmio_get_region(sbd, 0));
> +
>      for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
>          Object *obj = OBJECT(&s->bitband[i]);
>          SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index f2ada39..c814e16 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -19,7 +19,6 @@
>  #include "hw/arm/arm.h"
>  #include "hw/arm/armv7m_nvic.h"
>  #include "target/arm/cpu.h"
> -#include "exec/address-spaces.h"
>  #include "qemu/log.h"
>  #include "trace.h"
>
> @@ -1043,10 +1042,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>                            "nvic_sysregs", 0x1000);
>      memory_region_add_subregion(&s->container, 0, &s->sysregmem);
>
> -    /* 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);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> +
>      s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
>  }
>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 03/11] armv7m: QOMify the armv7m container

[Philippe Mathieu-Daudé]

Hi Peter,



On Mon, Feb 20, 2017 at 12:35 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> Create a proper QOM object for the armv7m container, which
> holds the CPU, the NVIC and the bitband regions.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armv7m.h |  51 ++++++++++++++++++
>  hw/arm/armv7m.c         | 140 ++++++++++++++++++++++++++++++
> +++++++++++++-----
>  2 files changed, 179 insertions(+), 12 deletions(-)
>  create mode 100644 include/hw/arm/armv7m.h
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> new file mode 100644
> index 0000000..193ad71
> --- /dev/null
> +++ b/include/hw/arm/armv7m.h
> @@ -0,0 +1,51 @@
> +/*
> + * ARMv7M CPU object
> + *
> + * Copyright (c) 2017 Linaro Ltd
> + * Written by Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This code is licensed under the GPL version 2 or later.
> + */
> +
> +#ifndef HW_ARM_ARMV7M_H
> +#define HW_ARM_ARMV7M_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/armv7m_nvic.h"
> +
> +#define TYPE_BITBAND "ARM,bitband-memory"
> +#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t base;
> +} BitBandState;
> +
> +#define TYPE_ARMV7M "armv7m"
> +#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
> +
> +#define ARMV7M_NUM_BITBANDS 2
> +
> +/* ARMv7M container object.
> + * + Unnamed GPIO input lines: external IRQ lines for the NVIC
> + * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
> + * + Property "cpu-model": CPU model to instantiate
> + * + Property "num-irq": number of external IRQ lines
> + */
> +typedef struct ARMv7MState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +    NVICState nvic;
> +    BitBandState bitband[ARMV7M_NUM_BITBANDS];
> +    ARMCPU *cpu;
> +
> +    /* Properties */
> +    char *cpu_model;
> +} ARMv7MState;
> +
> +#endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index b2cc6e9..56d02d4 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -8,6 +8,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "hw/arm/armv7m.h"
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> @@ -120,18 +121,6 @@ static const MemoryRegionOps bitband_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -#define TYPE_BITBAND "ARM,bitband-memory"
> -#define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
> -
> -typedef struct {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    MemoryRegion iomem;
> -    uint32_t base;
> -} BitBandState;
> -
>  static void bitband_init(Object *obj)
>  {
>      BitBandState *s = BITBAND(obj);
> @@ -159,6 +148,132 @@ static void armv7m_bitband_init(void)
>
>  /* Board init.  */
>
> +static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x20000000, 0x40000000
> +};
>

What do you think about adding a

#define ARMV7M_BITBAND_[REGION/IOMEM]_SIZE (1ULL << 25) or 0x2000000

and use it at least with memory_region_init_io() and eventually to get the
output base addr?


> +
> +static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
> +    0x22000000, 0x42000000
> +};
> +
> +static void armv7m_instance_init(Object *obj)
> +{
> +    ARMv7MState *s = ARMV7M(obj);
> +    int i;
> +
> +    /* Can't init the cpu here, we don't yet know which model to use */
> +
> +    object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic");
> +    qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
> +    object_property_add_alias(obj, "num-irq",
> +                              OBJECT(&s->nvic), "num-irq", &error_abort);
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        object_initialize(&s->bitband[i], sizeof(s->bitband[i]),
> TYPE_BITBAND);
> +        qdev_set_parent_bus(DEVICE(&s->bitband[i]),
> sysbus_get_default());
> +    }
> +}
> +
> +static void armv7m_realize(DeviceState *dev, Error **errp)
> +{
> +    /* here realize our children */
> +    ARMv7MState *s = ARMV7M(dev);
> +    Error *err = NULL;
> +    int i;
> +    char **cpustr;
> +    ObjectClass *oc;
> +    const char *typename;
> +    CPUClass *cc;
> +
> +    cpustr = g_strsplit(s->cpu_model, ",", 2);
> +
> +    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> +    if (!oc) {
> +        error_setg(errp, "Unknown CPU model %s", cpustr[0]);
> +        g_strfreev(cpustr);
> +        return;
> +    }
> +
> +    cc = CPU_CLASS(oc);
> +    typename = object_class_get_name(oc);
> +    cc->parse_features(typename, cpustr[1], &err);
> +    g_strfreev(cpustr);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    s->cpu = ARM_CPU(object_new(typename));
> +    if (!s->cpu) {
> +        error_setg(errp, "Unknown CPU model %s", s->cpu_model);
> +        return;
> +    }
> +
> +    object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Note that we must realize the NVIC after the CPU */
> +    object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    /* Alias the NVIC's input and output GPIOs as our own so the board
> +     * code can wire them up. (We do this in realize because the
> +     * NVIC doesn't create the input GPIO array until realize.)
> +     */
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> +    qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> +
> +    /* Wire the NVIC up to the CPU */
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->nvic), 0,
> +                       qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
> +    s->cpu->env.nvic = &s->nvic;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +        Object *obj = OBJECT(&s->bitband[i]);
> +        SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> +
> +        object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        object_property_set_bool(obj, true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sysbus_mmio_map(sbd, 0, bitband_output_addr[i]);
> +    }
> +}
> +
> +static Property armv7m_properties[] = {
> +    DEFINE_PROP_STRING("cpu-model", ARMv7MState, cpu_model),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void armv7m_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = armv7m_realize;
> +    dc->props = armv7m_properties;
> +}
> +
> +static const TypeInfo armv7m_info = {
> +    .name = TYPE_ARMV7M,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(ARMv7MState),
> +    .instance_init = armv7m_instance_init,
> +    .class_init = armv7m_class_init,
> +};
> +
>  static void armv7m_reset(void *opaque)
>  {
>      ARMCPU *cpu = opaque;
> @@ -264,6 +379,7 @@ static const TypeInfo bitband_info = {
>  static void armv7m_register_types(void)
>  {
>      type_register_static(&bitband_info);
> +    type_register_static(&armv7m_info);
>  }
>
>  type_init(armv7m_register_types)
> --
> 2.7.4
>
>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 09/11] armv7m: Split systick out from NVIC

[Philippe Mathieu-Daudé]

On 02/20/2017 03:51 PM, Peter Maydell wrote:
> On 20 February 2017 at 17:43, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> On 02/20/2017 12:36 PM, Peter Maydell wrote:
>>> +/* Conversion factor from qemu timer to SysTick frequencies.  */
>>> +static inline int64_t systick_scale(SysTickState *s)
>>> +{
>>> +    if (s->control & SYSTICK_CLKSOURCE) {
>>> +        return system_clock_scale;
>>> +    } else {
>>> +        return 1000;
>>
>>
>> this magic seems to be SYSTICK_SCALE
>> (Paul Brook's commit 9ee6e8bb85)
>
> This patch is just copying this function from one file to the other...
> at some point we might want to try to improve the handling of
> the clock scale factors but that leads into complication, because
> the external-clocksource scale factor should really be board
> level configurable. (Slightly confusingly the leg of this if/else
> which is board-configurable is the one for the internal clock,
> because we're modelling the ability of the stellaris board to
> configure the PLLs so as to downclock the whole CPU, which
> incidentally affects the timing of the internal systick clock.
> Modelling this properly needs to wait on Fred Konrad's clocktree
> patches getting in, I think.) For the moment I chose to just
> leave the existing source alone, mostly.

Fine!

I didn't know about those clocktree patches, I'll eventually use them 
for MIPS.

>>> +static void systick_instance_init(Object *obj)
>>> +{
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>> +    SysTickState *s = SYSTICK(obj);
>>> +
>>> +    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick",
>>> 0xe0);
>>
>>
>> It seems to me the correct size is 0x10. The manual describes the range
>> 0x20-0xFF as _reserved_. This _reserved_ description is at the end of the
>> 'SysTick' chapter which seems weird to me... I rather think this range
>> belong to the 'Interrupt Controller'@'System Control Space'.
>
> The table of the System Control Space (B3-3 in the v7M ARM ARM
> rev E.b) defines the Systick space as 0xE000E010 - 0xE000E0FF,
> which makes it 0xE0 bytes in size. The NVIC doesn't start
> until 0xE000E100 in this table.

I still think it's weird from an electronic design point of view, but 
since I don't have access to ARM internals I'll believe the datasheet 
blindly :S

> The table of the SysTick registers (B3-7) agrees, in that
> it defines the registers as covering 0xE000E010 to 0xE000E0FF.
> In the current architecture everything from 0x20..0xFF is
> reserved, which just means there's nothing there, but if
> there was ever a need for systick to get another register
> for some reason then it would go into that space (and if
> we then implemented the new register in QEMU we wouldn't need
> to change the NVIC code, only the systick code).
>
> I think the way this patch has modelled things matches the
> architecture -- it's a guest error to access the gap between
> 0XE000E020 and 0xE000E0FF, but we should report it as an
> attempt to access an invalid systick register rather than
> an access to an invalid NVIC register.

I agree.

> thanks
> -- PMM
>

Regards,

Phil.

Re: [Qemu-devel] [Qemu-arm] [PATCH 03/11] armv7m: QOMify the armv7m container

[Peter Maydell]

On 17 April 2017 at 04:59, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On Mon, Feb 20, 2017 at 12:35 PM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>> +static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
>> +    0x20000000, 0x40000000
>> +};
>
>
> What do you think about adding a
>
> #define ARMV7M_BITBAND_[REGION/IOMEM]_SIZE (1ULL << 25) or 0x2000000
>
> and use it at least with memory_region_init_io() and eventually to get the
> output base addr?

I don't think the size of the bitbanding region and the output base
address are inherently related: it's just a coincidence that the
output-region base address is at input-region-base + output-region-size.

I don't particularly object to a constant for the region size,
but I don't currently have any plans to come back to this particular
bit of code for a while (this patch is already in master).

thanks
-- PMM