[Qemu-devel] [PATCH 0/8] ARMSSE: Implement MHUs and dual-core capability

[Qemu-devel] [PATCH 0/8] ARMSSE: Implement MHUs and dual-core capability

[Peter Maydell]

This patchset adds support for running dual-core guest images on
the boards using the SSE-200 (mps2-tz, musca-a, musca-b1):
 * it implements the Message Handling Units (MHUs), which dual-core
   guests typically use to signal interrupts to each other
 * it implements the handling in the sysctl register block of the
   CPUWAIT register, which is how a guest powers up the second core
 * it adds support for the INITSVTOR[01] registers in the sysctl
   block, which guests typically use to make the second core start
   with a different exception vector table from the first one

The support for the MHUs has not yet landed in the Zephyr RTOS,
but this has been tested with the code and test application
in this pending pull request:
https://github.com/zephyrproject-rtos/zephyr/pull/12722

$ qemu-system-arm -M musca-a -kernel zephyr.elf \
  -device loader,file=zephyr-ns.elf \
  -serial null -serial stdio -display none -d unimp,guest_errors
pl011_read: Bad offset 0x44
pl011_read: Bad offset 0x44
***** Booting Zephyr OS zephyr-v1.13.0-3378-g1a0632c8b0 *****
IPM MHU sample on musca_a
CPU 0, get MHU0 success!
pl011_read: Bad offset 0x44
pl011_read: Bad offset 0x44
***** Booting Zephyr OS zephyr-v1.13.0-3378-g1a0632c8b0 *****
IPM MHU sample on musca_a_nonsecure
CPU 1, get MHU0 success!
MHU ISR on CPU 0
MHU ISR on CPU 1
MHU Test Done.

(The pl011 messages are a Zephyr bug which I have reported to them.)


Based-on: 20190214125107.22178-1-peter.maydell@linaro.org
("Add model of the Arm Musca devboards")

thanks
-- PMM

Peter Maydell (8):
  hw/misc/armsse-mhu.c: Model the SSE-200 Message Handling Unit
  hw/arm/armsse: Wire up the MHUs
  target/arm/cpu: Allow init-svtor property to be set after realize
  target/arm/arm-powerctl: Add new arm_set_cpu_on_and_reset()
  hw/misc/iotkit-sysctl: Correct typo in INITSVTOR0 register name
  hw/arm/iotkit-sysctl: Add SSE-200 registers
  hw/arm/iotkit-sysctl: Implement CPUWAIT and INITSVTOR*
  hw/arm/armsse: Unify init-svtor and cpuwait handling

 hw/misc/Makefile.objs           |   1 +
 include/hw/arm/armsse.h         |   3 +-
 include/hw/misc/armsse-mhu.h    |  44 +++++
 include/hw/misc/iotkit-sysctl.h |  25 ++-
 target/arm/arm-powerctl.h       |  16 ++
 hw/arm/armsse.c                 |  91 +++++++---
 hw/misc/armsse-mhu.c            | 198 +++++++++++++++++++++
 hw/misc/iotkit-sysctl.c         | 294 ++++++++++++++++++++++++++++++--
 target/arm/arm-powerctl.c       |  56 ++++++
 target/arm/cpu.c                |  29 +++-
 MAINTAINERS                     |   2 +
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/trace-events            |   4 +
 13 files changed, 714 insertions(+), 50 deletions(-)
 create mode 100644 include/hw/misc/armsse-mhu.h
 create mode 100644 hw/misc/armsse-mhu.c

-- 
2.20.1

[Qemu-devel] [PATCH 1/8] hw/misc/armsse-mhu.c: Model the SSE-200 Message Handling Unit

[Peter Maydell]

Implement a model of the Message Handling Unit (MHU) found in
the Arm SSE-200. This is a simple device which just contains
some registers which allow the two cores of the SSE-200
to raise interrupts on each other.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/Makefile.objs           |   1 +
 include/hw/misc/armsse-mhu.h    |  44 +++++++
 hw/misc/armsse-mhu.c            | 198 ++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   2 +
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/trace-events            |   4 +
 6 files changed, 250 insertions(+)
 create mode 100644 include/hw/misc/armsse-mhu.h
 create mode 100644 hw/misc/armsse-mhu.c

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 74c91d250c8..c71e07ae35d 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -70,6 +70,7 @@ obj-$(CONFIG_IOTKIT_SECCTL) += iotkit-secctl.o
 obj-$(CONFIG_IOTKIT_SYSCTL) += iotkit-sysctl.o
 obj-$(CONFIG_IOTKIT_SYSINFO) += iotkit-sysinfo.o
 obj-$(CONFIG_ARMSSE_CPUID) += armsse-cpuid.o
+obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_AUX) += auxbus.o
diff --git a/include/hw/misc/armsse-mhu.h b/include/hw/misc/armsse-mhu.h
new file mode 100644
index 00000000000..e57eafc2521
--- /dev/null
+++ b/include/hw/misc/armsse-mhu.h
@@ -0,0 +1,44 @@
+/*
+ * ARM SSE-200 Message Handling Unit (MHU)
+ *
+ * Copyright (c) 2019 Linaro Limited
+ * Written by Peter Maydell
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 or
+ *  (at your option) any later version.
+ */
+
+/*
+ * This is a model of the Message Handling Unit (MHU) which is part of the
+ * Arm SSE-200 and documented in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.101104_0100_00_en/corelink_sse200_subsystem_for_embedded_technical_reference_manual_101104_0100_00_en.pdf
+ *
+ * QEMU interface:
+ *  + sysbus MMIO region 0: the system information register bank
+ *  + sysbus IRQ 0: interrupt for CPU 0
+ *  + sysbus IRQ 1: interrupt for CPU 1
+ */
+
+#ifndef HW_MISC_SSE_MHU_H
+#define HW_MISC_SSE_MHU_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ARMSSE_MHU "armsse-mhu"
+#define ARMSSE_MHU(obj) OBJECT_CHECK(ARMSSEMHU, (obj), TYPE_ARMSSE_MHU)
+
+typedef struct ARMSSEMHU {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion iomem;
+    qemu_irq cpu0irq;
+    qemu_irq cpu1irq;
+
+    uint32_t cpu0intr;
+    uint32_t cpu1intr;
+} ARMSSEMHU;
+
+#endif
diff --git a/hw/misc/armsse-mhu.c b/hw/misc/armsse-mhu.c
new file mode 100644
index 00000000000..9ebca32e9aa
--- /dev/null
+++ b/hw/misc/armsse-mhu.c
@@ -0,0 +1,198 @@
+/*
+ * ARM SSE-200 Message Handling Unit (MHU)
+ *
+ * Copyright (c) 2019 Linaro Limited
+ * Written by Peter Maydell
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 or
+ *  (at your option) any later version.
+ */
+
+/*
+ * This is a model of the Message Handling Unit (MHU) which is part of the
+ * Arm SSE-200 and documented in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.101104_0100_00_en/corelink_sse200_subsystem_for_embedded_technical_reference_manual_101104_0100_00_en.pdf
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "qapi/error.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "hw/registerfields.h"
+#include "hw/misc/armsse-mhu.h"
+
+REG32(CPU0INTR_STAT, 0x0)
+REG32(CPU0INTR_SET, 0x4)
+REG32(CPU0INTR_CLR, 0x8)
+REG32(CPU1INTR_STAT, 0x10)
+REG32(CPU1INTR_SET, 0x14)
+REG32(CPU1INTR_CLR, 0x18)
+REG32(PID4, 0xfd0)
+REG32(PID5, 0xfd4)
+REG32(PID6, 0xfd8)
+REG32(PID7, 0xfdc)
+REG32(PID0, 0xfe0)
+REG32(PID1, 0xfe4)
+REG32(PID2, 0xfe8)
+REG32(PID3, 0xfec)
+REG32(CID0, 0xff0)
+REG32(CID1, 0xff4)
+REG32(CID2, 0xff8)
+REG32(CID3, 0xffc)
+
+/* Valid bits in the interrupt registers. If any are set the IRQ is raised */
+#define INTR_MASK 0xf
+
+/* PID/CID values */
+static const int armsse_mhu_id[] = {
+    0x04, 0x00, 0x00, 0x00, /* PID4..PID7 */
+    0x56, 0xb8, 0x0b, 0x00, /* PID0..PID3 */
+    0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */
+};
+
+static void armsse_mhu_update(ARMSSEMHU *s)
+{
+    qemu_set_irq(s->cpu0irq, s->cpu0intr != 0);
+    qemu_set_irq(s->cpu1irq, s->cpu1intr != 0);
+}
+
+static uint64_t armsse_mhu_read(void *opaque, hwaddr offset, unsigned size)
+{
+    ARMSSEMHU *s = ARMSSE_MHU(opaque);
+    uint64_t r;
+
+    switch (offset) {
+    case A_CPU0INTR_STAT:
+        r = s->cpu0intr;
+        break;
+
+    case A_CPU1INTR_STAT:
+        r = s->cpu1intr;
+        break;
+
+    case A_PID4 ... A_CID3:
+        r = armsse_mhu_id[(offset - A_PID4) / 4];
+        break;
+
+    case A_CPU0INTR_SET:
+    case A_CPU0INTR_CLR:
+    case A_CPU1INTR_SET:
+    case A_CPU1INTR_CLR:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SSE MHU: read of write-only register at offset 0x%x\n",
+                      (int)offset);
+        r = 0;
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SSE MHU read: bad offset 0x%x\n", (int)offset);
+        r = 0;
+        break;
+    }
+    trace_armsse_mhu_read(offset, r, size);
+    return r;
+}
+
+static void armsse_mhu_write(void *opaque, hwaddr offset,
+                             uint64_t value, unsigned size)
+{
+    ARMSSEMHU *s = ARMSSE_MHU(opaque);
+
+    trace_armsse_mhu_write(offset, value, size);
+
+    switch (offset) {
+    case A_CPU0INTR_SET:
+        s->cpu0intr |= (value & INTR_MASK);
+        break;
+    case A_CPU0INTR_CLR:
+        s->cpu0intr &= ~(value & INTR_MASK);
+        break;
+    case A_CPU1INTR_SET:
+        s->cpu1intr |= (value & INTR_MASK);
+        break;
+    case A_CPU1INTR_CLR:
+        s->cpu1intr &= ~(value & INTR_MASK);
+        break;
+
+    case A_CPU0INTR_STAT:
+    case A_CPU1INTR_STAT:
+    case A_PID4 ... A_CID3:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SSE MHU: write to read-only register at offset 0x%x\n",
+                      (int)offset);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SSE MHU write: bad offset 0x%x\n", (int)offset);
+        break;
+    }
+
+    armsse_mhu_update(s);
+}
+
+static const MemoryRegionOps armsse_mhu_ops = {
+    .read = armsse_mhu_read,
+    .write = armsse_mhu_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static void armsse_mhu_reset(DeviceState *dev)
+{
+    ARMSSEMHU *s = ARMSSE_MHU(dev);
+
+    s->cpu0intr = 0;
+    s->cpu1intr = 0;
+}
+
+static const VMStateDescription armsse_mhu_vmstate = {
+    .name = "armsse-mhu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(cpu0intr, ARMSSEMHU),
+        VMSTATE_UINT32(cpu1intr, ARMSSEMHU),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void armsse_mhu_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    ARMSSEMHU *s = ARMSSE_MHU(obj);
+
+    memory_region_init_io(&s->iomem, obj, &armsse_mhu_ops,
+                          s, "armsse-mhu", 0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->cpu0irq);
+    sysbus_init_irq(sbd, &s->cpu1irq);
+}
+
+static void armsse_mhu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = armsse_mhu_reset;
+    dc->vmsd = &armsse_mhu_vmstate;
+}
+
+static const TypeInfo armsse_mhu_info = {
+    .name = TYPE_ARMSSE_MHU,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ARMSSEMHU),
+    .instance_init = armsse_mhu_init,
+    .class_init = armsse_mhu_class_init,
+};
+
+static void armsse_mhu_register_types(void)
+{
+    type_register_static(&armsse_mhu_info);
+}
+
+type_init(armsse_mhu_register_types);
diff --git a/MAINTAINERS b/MAINTAINERS
index 770b2eaaf91..f198d510e89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -633,6 +633,8 @@ F: hw/misc/iotkit-sysinfo.c
 F: include/hw/misc/iotkit-sysinfo.h
 F: hw/misc/armsse-cpuid.c
 F: include/hw/misc/armsse-cpuid.h
+F: hw/misc/armsse-mhu.c
+F: include/hw/misc/armsse-mhu.h
 
 Musca
 M: Peter Maydell <peter.maydell@linaro.org>
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 87ad2674946..bd6943b691a 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -119,6 +119,7 @@ CONFIG_IOTKIT_SECCTL=y
 CONFIG_IOTKIT_SYSCTL=y
 CONFIG_IOTKIT_SYSINFO=y
 CONFIG_ARMSSE_CPUID=y
+CONFIG_ARMSSE_MHU=y
 
 CONFIG_VERSATILE=y
 CONFIG_VERSATILE_PCI=y
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index b0701bddd3c..c1795bb54b8 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -136,3 +136,7 @@ iotkit_sysctl_reset(void) "IoTKit SysCtl: reset"
 # hw/misc/armsse-cpuid.c
 armsse_cpuid_read(uint64_t offset, uint64_t data, unsigned size) "SSE-200 CPU_IDENTITY read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 armsse_cpuid_write(uint64_t offset, uint64_t data, unsigned size) "SSE-200 CPU_IDENTITY write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+
+# hw/misc/armsse-mhu.c
+armsse_mhu_read(uint64_t offset, uint64_t data, unsigned size) "SSE-200 MHU read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+armsse_mhu_write(uint64_t offset, uint64_t data, unsigned size) "SSE-200 MHU write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
-- 
2.20.1

[Qemu-devel] [PATCH 2/8] hw/arm/armsse: Wire up the MHUs

[Peter Maydell]

Create and connect the MHUs in the SSE-200.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armsse.h |  3 ++-
 hw/arm/armsse.c         | 40 ++++++++++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
index 7ef871c7dfe..81e082cccf8 100644
--- a/include/hw/arm/armsse.h
+++ b/include/hw/arm/armsse.h
@@ -95,6 +95,7 @@
 #include "hw/misc/iotkit-sysctl.h"
 #include "hw/misc/iotkit-sysinfo.h"
 #include "hw/misc/armsse-cpuid.h"
+#include "hw/misc/armsse-mhu.h"
 #include "hw/misc/unimp.h"
 #include "hw/or-irq.h"
 #include "hw/core/split-irq.h"
@@ -166,7 +167,7 @@ typedef struct ARMSSE {
     IoTKitSysCtl sysctl;
     IoTKitSysCtl sysinfo;
 
-    UnimplementedDeviceState mhu[2];
+    ARMSSEMHU mhu[2];
     UnimplementedDeviceState ppu[NUM_PPUS];
     UnimplementedDeviceState cachectrl[SSE_MAX_CPUS];
     UnimplementedDeviceState cpusecctrl[SSE_MAX_CPUS];
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 129e7ea7fe0..5768d6fbf8b 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -282,9 +282,9 @@ static void armsse_init(Object *obj)
                           sizeof(s->sysinfo), TYPE_IOTKIT_SYSINFO);
     if (info->has_mhus) {
         sysbus_init_child_obj(obj, "mhu0", &s->mhu[0], sizeof(s->mhu[0]),
-                              TYPE_UNIMPLEMENTED_DEVICE);
+                              TYPE_ARMSSE_MHU);
         sysbus_init_child_obj(obj, "mhu1", &s->mhu[1], sizeof(s->mhu[1]),
-                              TYPE_UNIMPLEMENTED_DEVICE);
+                              TYPE_ARMSSE_MHU);
     }
     if (info->has_ppus) {
         for (i = 0; i < info->num_cpus; i++) {
@@ -766,22 +766,28 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     }
 
     if (info->has_mhus) {
-        for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {
-            char *name;
-            char *port;
+        /*
+         * An SSE-200 with only one CPU should have only one MHU created,
+         * with the region where the second MHU usually is being RAZ/WI.
+         * We don't implement that SSE-200 config; if we want to support
+         * it then this code needs to be enhanced to handle creating the
+         * RAZ/WI region instead of the second MHU.
+         */
+        assert(info->num_cpus > 1);
+
+        for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {
+            char *port;
+            int cpunum;
+            SysBusDevice *mhu_sbd = SYS_BUS_DEVICE(&s->mhu[i]);
 
-            name = g_strdup_printf("MHU%d", i);
-            qdev_prop_set_string(DEVICE(&s->mhu[i]), "name", name);
-            qdev_prop_set_uint64(DEVICE(&s->mhu[i]), "size", 0x1000);
             object_property_set_bool(OBJECT(&s->mhu[i]), true,
                                      "realized", &err);
-            g_free(name);
             if (err) {
                 error_propagate(errp, err);
                 return;
             }
             port = g_strdup_printf("port[%d]", i + 3);
-            mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mhu[i]), 0);
+            mr = sysbus_mmio_get_region(mhu_sbd, 0);
             object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
                                      port, &err);
             g_free(port);
@@ -789,6 +795,20 @@ static void armsse_realize(DeviceState *dev, Error **errp)
                 error_propagate(errp, err);
                 return;
             }
+
+            /*
+             * Each MHU has an irq line for each CPU:
+             *  MHU 0 irq line 0 -> CPU 0 IRQ 6
+             *  MHU 0 irq line 1 -> CPU 1 IRQ 6
+             *  MHU 1 irq line 0 -> CPU 0 IRQ 7
+             *  MHU 1 irq line 1 -> CPU 1 IRQ 7
+             */
+            for (cpunum = 0; cpunum < info->num_cpus; cpunum++) {
+                DeviceState *cpudev = DEVICE(&s->armv7m[cpunum]);
+
+                sysbus_connect_irq(mhu_sbd, cpunum,
+                                   qdev_get_gpio_in(cpudev, 6 + i));
+            }
         }
     }
 
-- 
2.20.1

[Qemu-devel] [PATCH 3/8] target/arm/cpu: Allow init-svtor property to be set after realize

[Peter Maydell]

Make the M-profile "init-svtor" property be settable after realize.
This matches the hardware, where this is a config signal which
is sampled on CPU reset and can thus be changed between one
reset and another. To do this we have to change the API we
use to add the property.

(We will need this capability for the SSE-200.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index edf6e0e1f1c..a418ac5cc32 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -22,6 +22,7 @@
 #include "target/arm/idau.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "cpu.h"
 #include "internals.h"
 #include "qemu-common.h"
@@ -771,9 +772,21 @@ static Property arm_cpu_pmsav7_dregion_property =
                                            pmsav7_dregion,
                                            qdev_prop_uint32, uint32_t);
 
-/* M profile: initial value of the Secure VTOR */
-static Property arm_cpu_initsvtor_property =
-            DEFINE_PROP_UINT32("init-svtor", ARMCPU, init_svtor, 0);
+static void arm_get_init_svtor(Object *obj, Visitor *v, const char *name,
+                               void *opaque, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    visit_type_uint32(v, name, &cpu->init_svtor, errp);
+}
+
+static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
+                               void *opaque, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    visit_type_uint32(v, name, &cpu->init_svtor, errp);
+}
 
 void arm_cpu_post_init(Object *obj)
 {
@@ -845,8 +858,14 @@ void arm_cpu_post_init(Object *obj)
                                  qdev_prop_allow_set_link_before_realize,
                                  OBJ_PROP_LINK_STRONG,
                                  &error_abort);
-        qdev_property_add_static(DEVICE(obj), &arm_cpu_initsvtor_property,
-                                 &error_abort);
+        /*
+         * M profile: initial value of the Secure VTOR. We can't just use
+         * a simple DEFINE_PROP_UINT32 for this because we want to permit
+         * the property to be set after realize.
+         */
+        object_property_add(obj, "init-svtor", "uint32",
+                            arm_get_init_svtor, arm_set_init_svtor,
+                            NULL, NULL, &error_abort);
     }
 
     qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
-- 
2.20.1

[Qemu-devel] [PATCH 4/8] target/arm/arm-powerctl: Add new arm_set_cpu_on_and_reset()

[Peter Maydell]

Currently the Arm arm-powerctl.h APIs allow:
 * arm_set_cpu_on(), which powers on a CPU and sets its
   initial PC and other startup state
 * arm_reset_cpu(), which resets a CPU which is already on
   (and fails if the CPU is powered off)

but there is no way to say "power on a CPU as if it had
just come out of reset and don't do anything else to it".

Add a new function arm_set_cpu_on_and_reset(), which does this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/arm-powerctl.h | 16 +++++++++++
 target/arm/arm-powerctl.c | 56 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/target/arm/arm-powerctl.h b/target/arm/arm-powerctl.h
index 04353923c06..37c8a04f0a9 100644
--- a/target/arm/arm-powerctl.h
+++ b/target/arm/arm-powerctl.h
@@ -74,4 +74,20 @@ int arm_set_cpu_off(uint64_t cpuid);
  */
 int arm_reset_cpu(uint64_t cpuid);
 
+/*
+ * arm_set_cpu_on_and_reset:
+ * @cpuid: the id of the CPU we want to star
+ *
+ * Start the cpu designated by @cpuid and put it through its normal
+ * CPU reset process. The CPU will start in the way it is architected
+ * to start after a power-on reset.
+ *
+ * Returns: QEMU_ARM_POWERCTL_RET_SUCCESS on success.
+ * QEMU_ARM_POWERCTL_INVALID_PARAM if there is no CPU with that ID.
+ * QEMU_ARM_POWERCTL_ALREADY_ON if the CPU is already on.
+ * QEMU_ARM_POWERCTL_ON_PENDING if the CPU is already partway through
+ * powering on.
+ */
+int arm_set_cpu_on_and_reset(uint64_t cpuid);
+
 #endif
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index f9de5164e55..f77a950db67 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -228,6 +228,62 @@ int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id,
     return QEMU_ARM_POWERCTL_RET_SUCCESS;
 }
 
+static void arm_set_cpu_on_and_reset_async_work(CPUState *target_cpu_state,
+                                                run_on_cpu_data data)
+{
+    ARMCPU *target_cpu = ARM_CPU(target_cpu_state);
+
+    /* Initialize the cpu we are turning on */
+    cpu_reset(target_cpu_state);
+    target_cpu_state->halted = 0;
+
+    /* Finally set the power status */
+    assert(qemu_mutex_iothread_locked());
+    target_cpu->power_state = PSCI_ON;
+}
+
+int arm_set_cpu_on_and_reset(uint64_t cpuid)
+{
+    CPUState *target_cpu_state;
+    ARMCPU *target_cpu;
+
+    assert(qemu_mutex_iothread_locked());
+
+    /* Retrieve the cpu we are powering up */
+    target_cpu_state = arm_get_cpu_by_id(cpuid);
+    if (!target_cpu_state) {
+        /* The cpu was not found */
+        return QEMU_ARM_POWERCTL_INVALID_PARAM;
+    }
+
+    target_cpu = ARM_CPU(target_cpu_state);
+    if (target_cpu->power_state == PSCI_ON) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "[ARM]%s: CPU %" PRId64 " is already on\n",
+                      __func__, cpuid);
+        return QEMU_ARM_POWERCTL_ALREADY_ON;
+    }
+
+    /*
+     * If another CPU has powered the target on we are in the state
+     * ON_PENDING and additional attempts to power on the CPU should
+     * fail (see 6.6 Implementation CPU_ON/CPU_OFF races in the PSCI
+     * spec)
+     */
+    if (target_cpu->power_state == PSCI_ON_PENDING) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "[ARM]%s: CPU %" PRId64 " is already powering on\n",
+                      __func__, cpuid);
+        return QEMU_ARM_POWERCTL_ON_PENDING;
+    }
+
+    async_run_on_cpu(target_cpu_state, arm_set_cpu_on_and_reset_async_work,
+                     RUN_ON_CPU_NULL);
+
+    /* We are good to go */
+    return QEMU_ARM_POWERCTL_RET_SUCCESS;
+}
+
 static void arm_set_cpu_off_async_work(CPUState *target_cpu_state,
                                        run_on_cpu_data data)
 {
-- 
2.20.1

[Qemu-devel] [PATCH 5/8] hw/misc/iotkit-sysctl: Correct typo in INITSVTOR0 register name

[Peter Maydell]

The iotkit-sysctl device has a register it names INITSVRTOR0.
This is actually a typo present in the IoTKit documentation
and also in part of the SSE-200 documentation:  it should be
INITSVTOR0 because it is specifying the initial value of the
Secure VTOR register in the CPU. Correct the typo.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/iotkit-sysctl.h |  2 +-
 hw/misc/iotkit-sysctl.c         | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/misc/iotkit-sysctl.h b/include/hw/misc/iotkit-sysctl.h
index e36613cb5ee..17a145517a4 100644
--- a/include/hw/misc/iotkit-sysctl.h
+++ b/include/hw/misc/iotkit-sysctl.h
@@ -41,7 +41,7 @@ typedef struct IoTKitSysCtl {
     uint32_t reset_syndrome;
     uint32_t reset_mask;
     uint32_t gretreg;
-    uint32_t initsvrtor0;
+    uint32_t initsvtor0;
     uint32_t cpuwait;
     uint32_t wicctrl;
 } IoTKitSysCtl;
diff --git a/hw/misc/iotkit-sysctl.c b/hw/misc/iotkit-sysctl.c
index a21d8bd6789..8c85aea9309 100644
--- a/hw/misc/iotkit-sysctl.c
+++ b/hw/misc/iotkit-sysctl.c
@@ -33,7 +33,7 @@ REG32(RESET_MASK, 0x104)
 REG32(SWRESET, 0x108)
     FIELD(SWRESET, SWRESETREQ, 9, 1)
 REG32(GRETREG, 0x10c)
-REG32(INITSVRTOR0, 0x110)
+REG32(INITSVTOR0, 0x110)
 REG32(CPUWAIT, 0x118)
 REG32(BUSWAIT, 0x11c)
 REG32(WICCTRL, 0x120)
@@ -76,8 +76,8 @@ static uint64_t iotkit_sysctl_read(void *opaque, hwaddr offset,
     case A_GRETREG:
         r = s->gretreg;
         break;
-    case A_INITSVRTOR0:
-        r = s->initsvrtor0;
+    case A_INITSVTOR0:
+        r = s->initsvtor0;
         break;
     case A_CPUWAIT:
         r = s->cpuwait;
@@ -145,9 +145,9 @@ static void iotkit_sysctl_write(void *opaque, hwaddr offset,
          */
         s->gretreg = value;
         break;
-    case A_INITSVRTOR0:
-        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl INITSVRTOR0 unimplemented\n");
-        s->initsvrtor0 = value;
+    case A_INITSVTOR0:
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl INITSVTOR0 unimplemented\n");
+        s->initsvtor0 = value;
         break;
     case A_CPUWAIT:
         qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl CPUWAIT unimplemented\n");
@@ -206,7 +206,7 @@ static void iotkit_sysctl_reset(DeviceState *dev)
     s->reset_syndrome = 1;
     s->reset_mask = 0;
     s->gretreg = 0;
-    s->initsvrtor0 = 0x10000000;
+    s->initsvtor0 = 0x10000000;
     s->cpuwait = 0;
     s->wicctrl = 0;
 }
@@ -230,7 +230,7 @@ static const VMStateDescription iotkit_sysctl_vmstate = {
         VMSTATE_UINT32(reset_syndrome, IoTKitSysCtl),
         VMSTATE_UINT32(reset_mask, IoTKitSysCtl),
         VMSTATE_UINT32(gretreg, IoTKitSysCtl),
-        VMSTATE_UINT32(initsvrtor0, IoTKitSysCtl),
+        VMSTATE_UINT32(initsvtor0, IoTKitSysCtl),
         VMSTATE_UINT32(cpuwait, IoTKitSysCtl),
         VMSTATE_UINT32(wicctrl, IoTKitSysCtl),
         VMSTATE_END_OF_LIST()
-- 
2.20.1

[Qemu-devel] [PATCH 6/8] hw/arm/iotkit-sysctl: Add SSE-200 registers

[Peter Maydell]

The SYSCTL block in the SSE-200 has some extra registers that
are not present in the IoTKit version. Add these registers
(as reads-as-written stubs), enabled by a new QOM property.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/iotkit-sysctl.h |  20 +++
 hw/arm/armsse.c                 |   2 +
 hw/misc/iotkit-sysctl.c         | 245 +++++++++++++++++++++++++++++++-
 3 files changed, 262 insertions(+), 5 deletions(-)

diff --git a/include/hw/misc/iotkit-sysctl.h b/include/hw/misc/iotkit-sysctl.h
index 17a145517a4..9c2f23ecd28 100644
--- a/include/hw/misc/iotkit-sysctl.h
+++ b/include/hw/misc/iotkit-sysctl.h
@@ -17,6 +17,9 @@
  * "system control register" blocks.
  *
  * QEMU interface:
+ *  + QOM property "SYS_VERSION": value of the SYS_VERSION register of the
+ *    system information block of the SSE
+ *    (used to identify whether to provide SSE-200-only registers)
  *  + sysbus MMIO region 0: the system information register bank
  *  + sysbus MMIO region 1: the system control register bank
  */
@@ -44,6 +47,23 @@ typedef struct IoTKitSysCtl {
     uint32_t initsvtor0;
     uint32_t cpuwait;
     uint32_t wicctrl;
+    uint32_t scsecctrl;
+    uint32_t fclk_div;
+    uint32_t sysclk_div;
+    uint32_t clock_force;
+    uint32_t initsvtor1;
+    uint32_t nmi_enable;
+    uint32_t ewctrl;
+    uint32_t pdcm_pd_sys_sense;
+    uint32_t pdcm_pd_sram0_sense;
+    uint32_t pdcm_pd_sram1_sense;
+    uint32_t pdcm_pd_sram2_sense;
+    uint32_t pdcm_pd_sram3_sense;
+
+    /* Properties */
+    uint32_t sys_version;
+
+    bool is_sse200;
 } IoTKitSysCtl;
 
 #endif
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 5768d6fbf8b..7c946564ebc 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -997,6 +997,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     /* System information registers */
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysinfo), 0, 0x40020000);
     /* System control registers */
+    object_property_set_int(OBJECT(&s->sysctl), info->sys_version,
+                            "SYS_VERSION", &err);
     object_property_set_bool(OBJECT(&s->sysctl), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/misc/iotkit-sysctl.c b/hw/misc/iotkit-sysctl.c
index 8c85aea9309..05606017fc2 100644
--- a/hw/misc/iotkit-sysctl.c
+++ b/hw/misc/iotkit-sysctl.c
@@ -17,6 +17,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
 #include "qemu/log.h"
 #include "trace.h"
 #include "qapi/error.h"
@@ -28,15 +29,26 @@
 REG32(SECDBGSTAT, 0x0)
 REG32(SECDBGSET, 0x4)
 REG32(SECDBGCLR, 0x8)
+REG32(SCSECCTRL, 0xc)
+REG32(FCLK_DIV, 0x10)
+REG32(SYSCLK_DIV, 0x14)
+REG32(CLOCK_FORCE, 0x18)
 REG32(RESET_SYNDROME, 0x100)
 REG32(RESET_MASK, 0x104)
 REG32(SWRESET, 0x108)
     FIELD(SWRESET, SWRESETREQ, 9, 1)
 REG32(GRETREG, 0x10c)
 REG32(INITSVTOR0, 0x110)
+REG32(INITSVTOR1, 0x114)
 REG32(CPUWAIT, 0x118)
-REG32(BUSWAIT, 0x11c)
+REG32(NMI_ENABLE, 0x11c) /* BUSWAIT in IoTKit */
 REG32(WICCTRL, 0x120)
+REG32(EWCTRL, 0x124)
+REG32(PDCM_PD_SYS_SENSE, 0x200)
+REG32(PDCM_PD_SRAM0_SENSE, 0x20c)
+REG32(PDCM_PD_SRAM1_SENSE, 0x210)
+REG32(PDCM_PD_SRAM2_SENSE, 0x214)
+REG32(PDCM_PD_SRAM3_SENSE, 0x218)
 REG32(PID4, 0xfd0)
 REG32(PID5, 0xfd4)
 REG32(PID6, 0xfd8)
@@ -67,6 +79,30 @@ static uint64_t iotkit_sysctl_read(void *opaque, hwaddr offset,
     case A_SECDBGSTAT:
         r = s->secure_debug;
         break;
+    case A_SCSECCTRL:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->scsecctrl;
+        break;
+    case A_FCLK_DIV:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->fclk_div;
+        break;
+    case A_SYSCLK_DIV:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->sysclk_div;
+        break;
+    case A_CLOCK_FORCE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->clock_force;
+        break;
     case A_RESET_SYNDROME:
         r = s->reset_syndrome;
         break;
@@ -79,16 +115,62 @@ static uint64_t iotkit_sysctl_read(void *opaque, hwaddr offset,
     case A_INITSVTOR0:
         r = s->initsvtor0;
         break;
+    case A_INITSVTOR1:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->initsvtor1;
+        break;
     case A_CPUWAIT:
         r = s->cpuwait;
         break;
-    case A_BUSWAIT:
-        /* In IoTKit BUSWAIT is reserved, R/O, zero */
-        r = 0;
+    case A_NMI_ENABLE:
+        /* In IoTKit this is named BUSWAIT but is marked reserved, R/O, zero */
+        if (!s->is_sse200) {
+            r = 0;
+            break;
+        }
+        r = s->nmi_enable;
         break;
     case A_WICCTRL:
         r = s->wicctrl;
         break;
+    case A_EWCTRL:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->ewctrl;
+        break;
+    case A_PDCM_PD_SYS_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->pdcm_pd_sys_sense;
+        break;
+    case A_PDCM_PD_SRAM0_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->pdcm_pd_sram0_sense;
+        break;
+    case A_PDCM_PD_SRAM1_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->pdcm_pd_sram1_sense;
+        break;
+    case A_PDCM_PD_SRAM2_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->pdcm_pd_sram2_sense;
+        break;
+    case A_PDCM_PD_SRAM3_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        r = s->pdcm_pd_sram3_sense;
+        break;
     case A_PID4 ... A_CID3:
         r = sysctl_id[(offset - A_PID4) / 4];
         break;
@@ -101,6 +183,7 @@ static uint64_t iotkit_sysctl_read(void *opaque, hwaddr offset,
         r = 0;
         break;
     default:
+    bad_offset:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "IoTKit SysCtl read: bad offset %x\n", (int)offset);
         r = 0;
@@ -172,14 +255,105 @@ static void iotkit_sysctl_write(void *opaque, hwaddr offset,
             qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
-    case A_BUSWAIT:        /* In IoTKit BUSWAIT is reserved, R/O, zero */
+    case A_SCSECCTRL:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl SCSECCTRL unimplemented\n");
+        s->scsecctrl = value;
+        break;
+    case A_FCLK_DIV:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl FCLK_DIV unimplemented\n");
+        s->fclk_div = value;
+        break;
+    case A_SYSCLK_DIV:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl SYSCLK_DIV unimplemented\n");
+        s->sysclk_div = value;
+        break;
+    case A_CLOCK_FORCE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl CLOCK_FORCE unimplemented\n");
+        s->clock_force = value;
+        break;
+    case A_INITSVTOR1:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl INITSVTOR1 unimplemented\n");
+        s->initsvtor1 = value;
+        break;
+    case A_EWCTRL:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl EWCTRL unimplemented\n");
+        s->ewctrl = value;
+        break;
+    case A_PDCM_PD_SYS_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP,
+                      "IoTKit SysCtl PDCM_PD_SYS_SENSE unimplemented\n");
+        s->pdcm_pd_sys_sense = value;
+        break;
+    case A_PDCM_PD_SRAM0_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP,
+                      "IoTKit SysCtl PDCM_PD_SRAM0_SENSE unimplemented\n");
+        s->pdcm_pd_sram0_sense = value;
+        break;
+    case A_PDCM_PD_SRAM1_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP,
+                      "IoTKit SysCtl PDCM_PD_SRAM1_SENSE unimplemented\n");
+        s->pdcm_pd_sram1_sense = value;
+        break;
+    case A_PDCM_PD_SRAM2_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP,
+                      "IoTKit SysCtl PDCM_PD_SRAM2_SENSE unimplemented\n");
+        s->pdcm_pd_sram2_sense = value;
+        break;
+    case A_PDCM_PD_SRAM3_SENSE:
+        if (!s->is_sse200) {
+            goto bad_offset;
+        }
+        qemu_log_mask(LOG_UNIMP,
+                      "IoTKit SysCtl PDCM_PD_SRAM3_SENSE unimplemented\n");
+        s->pdcm_pd_sram3_sense = value;
+        break;
+    case A_NMI_ENABLE:
+        /* In IoTKit this is BUSWAIT: reserved, R/O, zero */
+        if (!s->is_sse200) {
+            goto ro_offset;
+        }
+        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl NMI_ENABLE unimplemented\n");
+        s->nmi_enable = value;
+        break;
     case A_SECDBGSTAT:
     case A_PID4 ... A_CID3:
+    ro_offset:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "IoTKit SysCtl write: write of RO offset %x\n",
                       (int)offset);
         break;
     default:
+    bad_offset:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "IoTKit SysCtl write: bad offset %x\n", (int)offset);
         break;
@@ -207,8 +381,20 @@ static void iotkit_sysctl_reset(DeviceState *dev)
     s->reset_mask = 0;
     s->gretreg = 0;
     s->initsvtor0 = 0x10000000;
+    s->initsvtor1 = 0x10000000;
     s->cpuwait = 0;
     s->wicctrl = 0;
+    s->scsecctrl = 0;
+    s->fclk_div = 0;
+    s->sysclk_div = 0;
+    s->clock_force = 0;
+    s->nmi_enable = 0;
+    s->ewctrl = 0;
+    s->pdcm_pd_sys_sense = 0x7f;
+    s->pdcm_pd_sram0_sense = 0;
+    s->pdcm_pd_sram1_sense = 0;
+    s->pdcm_pd_sram2_sense = 0;
+    s->pdcm_pd_sram3_sense = 0;
 }
 
 static void iotkit_sysctl_init(Object *obj)
@@ -221,6 +407,44 @@ static void iotkit_sysctl_init(Object *obj)
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
+static void iotkit_sysctl_realize(DeviceState *dev, Error **errp)
+{
+    IoTKitSysCtl *s = IOTKIT_SYSCTL(dev);
+
+    /* The top 4 bits of the SYS_VERSION register tell us if we're an SSE-200 */
+    if (extract32(s->sys_version, 28, 4) == 2) {
+        s->is_sse200 = true;
+    }
+}
+
+static bool sse200_needed(void *opaque)
+{
+    IoTKitSysCtl *s = IOTKIT_SYSCTL(opaque);
+
+    return s->is_sse200;
+}
+
+static const VMStateDescription iotkit_sysctl_sse200_vmstate = {
+    .name = "iotkit-sysctl/sse-200",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = sse200_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(scsecctrl, IoTKitSysCtl),
+        VMSTATE_UINT32(fclk_div, IoTKitSysCtl),
+        VMSTATE_UINT32(sysclk_div, IoTKitSysCtl),
+        VMSTATE_UINT32(clock_force, IoTKitSysCtl),
+        VMSTATE_UINT32(initsvtor1, IoTKitSysCtl),
+        VMSTATE_UINT32(nmi_enable, IoTKitSysCtl),
+        VMSTATE_UINT32(pdcm_pd_sys_sense, IoTKitSysCtl),
+        VMSTATE_UINT32(pdcm_pd_sram0_sense, IoTKitSysCtl),
+        VMSTATE_UINT32(pdcm_pd_sram1_sense, IoTKitSysCtl),
+        VMSTATE_UINT32(pdcm_pd_sram2_sense, IoTKitSysCtl),
+        VMSTATE_UINT32(pdcm_pd_sram3_sense, IoTKitSysCtl),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription iotkit_sysctl_vmstate = {
     .name = "iotkit-sysctl",
     .version_id = 1,
@@ -234,15 +458,26 @@ static const VMStateDescription iotkit_sysctl_vmstate = {
         VMSTATE_UINT32(cpuwait, IoTKitSysCtl),
         VMSTATE_UINT32(wicctrl, IoTKitSysCtl),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription*[]) {
+        &iotkit_sysctl_sse200_vmstate,
+        NULL
     }
 };
 
+static Property iotkit_sysctl_props[] = {
+    DEFINE_PROP_UINT32("SYS_VERSION", IoTKitSysCtl, sys_version, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void iotkit_sysctl_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &iotkit_sysctl_vmstate;
     dc->reset = iotkit_sysctl_reset;
+    dc->props = iotkit_sysctl_props;
+    dc->realize = iotkit_sysctl_realize;
 }
 
 static const TypeInfo iotkit_sysctl_info = {
-- 
2.20.1

[Qemu-devel] [PATCH 7/8] hw/arm/iotkit-sysctl: Implement CPUWAIT and INITSVTOR*

[Peter Maydell]

The CPUWAIT register acts as a sort of power-control: if a bit
in it is 1 then the CPU will have been forced into waiting
when the system was reset (which in QEMU we model as the
CPU starting powered off). Writing a 0 to the register will
allow the CPU to boot (for QEMU, we model this as powering
it on). Note that writing 0 to the register does not power
off a CPU.

For this to work correctly we need to also honour the
INITSVTOR* registers, which let the guest control where the
CPU will load its SP and PC from when it comes out of reset.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/iotkit-sysctl.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/hw/misc/iotkit-sysctl.c b/hw/misc/iotkit-sysctl.c
index 05606017fc2..e333c8169a3 100644
--- a/hw/misc/iotkit-sysctl.c
+++ b/hw/misc/iotkit-sysctl.c
@@ -25,6 +25,8 @@
 #include "hw/sysbus.h"
 #include "hw/registerfields.h"
 #include "hw/misc/iotkit-sysctl.h"
+#include "target/arm/arm-powerctl.h"
+#include "target/arm/cpu.h"
 
 REG32(SECDBGSTAT, 0x0)
 REG32(SECDBGSET, 0x4)
@@ -69,6 +71,21 @@ static const int sysctl_id[] = {
     0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */
 };
 
+/*
+ * Set the initial secure vector table offset address for the core.
+ * This will take effect when the CPU next resets.
+ */
+static void set_init_vtor(uint64_t cpuid, uint32_t vtor)
+{
+    Object *cpuobj = OBJECT(arm_get_cpu_by_id(cpuid));
+
+    if (cpuobj) {
+        if (object_property_find(cpuobj, "init-svtor", NULL)) {
+            object_property_set_uint(cpuobj, vtor, "init-svtor", &error_abort);
+        }
+    }
+}
+
 static uint64_t iotkit_sysctl_read(void *opaque, hwaddr offset,
                                     unsigned size)
 {
@@ -229,11 +246,18 @@ static void iotkit_sysctl_write(void *opaque, hwaddr offset,
         s->gretreg = value;
         break;
     case A_INITSVTOR0:
-        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl INITSVTOR0 unimplemented\n");
         s->initsvtor0 = value;
+        set_init_vtor(0, s->initsvtor0);
         break;
     case A_CPUWAIT:
-        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl CPUWAIT unimplemented\n");
+        if ((s->cpuwait & 1) && !(value & 1)) {
+            /* Powering up CPU 0 */
+            arm_set_cpu_on_and_reset(0);
+        }
+        if ((s->cpuwait & 2) && !(value & 2)) {
+            /* Powering up CPU 1 */
+            arm_set_cpu_on_and_reset(1);
+        }
         s->cpuwait = value;
         break;
     case A_WICCTRL:
@@ -287,8 +311,8 @@ static void iotkit_sysctl_write(void *opaque, hwaddr offset,
         if (!s->is_sse200) {
             goto bad_offset;
         }
-        qemu_log_mask(LOG_UNIMP, "IoTKit SysCtl INITSVTOR1 unimplemented\n");
         s->initsvtor1 = value;
+        set_init_vtor(1, s->initsvtor1);
         break;
     case A_EWCTRL:
         if (!s->is_sse200) {
@@ -382,7 +406,16 @@ static void iotkit_sysctl_reset(DeviceState *dev)
     s->gretreg = 0;
     s->initsvtor0 = 0x10000000;
     s->initsvtor1 = 0x10000000;
-    s->cpuwait = 0;
+    if (s->is_sse200) {
+        /*
+         * CPU 0 starts on, CPU 1 starts off. In real hardware this is
+         * configurable by the SoC integrator as a verilog parameter.
+         */
+        s->cpuwait = 2;
+    } else {
+        /* CPU 0 starts on */
+        s->cpuwait = 0;
+    }
     s->wicctrl = 0;
     s->scsecctrl = 0;
     s->fclk_div = 0;
-- 
2.20.1

[Qemu-devel] [PATCH 8/8] hw/arm/armsse: Unify init-svtor and cpuwait handling

[Peter Maydell]

At the moment the handling of init-svtor and cpuwait initial
values is split between armsse.c and iotkit-sysctl.c:
the code in armsse.c sets the initial state of the CPU
object by setting the init-svtor and start-powered-off
properties, but the iotkit-sysctl.c code has its own
code setting the reset values of its registers (which are
then used when updating the CPU when the guest makes
runtime changes).

Clean this up by making the armsse.c code set properties on the
iotkit-sysctl object to define the initial values of the
registers, so they always match the initial CPU state,
and update the comments in armsse.c accordingly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/iotkit-sysctl.h |  3 ++
 hw/arm/armsse.c                 | 49 +++++++++++++++++++++------------
 hw/misc/iotkit-sysctl.c         | 20 ++++++--------
 3 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/include/hw/misc/iotkit-sysctl.h b/include/hw/misc/iotkit-sysctl.h
index 9c2f23ecd28..601c8ecc0d0 100644
--- a/include/hw/misc/iotkit-sysctl.h
+++ b/include/hw/misc/iotkit-sysctl.h
@@ -62,6 +62,9 @@ typedef struct IoTKitSysCtl {
 
     /* Properties */
     uint32_t sys_version;
+    uint32_t cpuwait_rst;
+    uint32_t initsvtor0_rst;
+    uint32_t initsvtor1_rst;
 
     bool is_sse200;
 } IoTKitSysCtl;
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 7c946564ebc..027e113f2ee 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -11,6 +11,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/bitops.h"
 #include "qapi/error.h"
 #include "trace.h"
 #include "hw/sysbus.h"
@@ -29,6 +30,7 @@ struct ARMSSEInfo {
     int sram_banks;
     int num_cpus;
     uint32_t sys_version;
+    uint32_t cpuwait_rst;
     SysConfigFormat sys_config_format;
     bool has_mhus;
     bool has_ppus;
@@ -43,6 +45,7 @@ static const ARMSSEInfo armsse_variants[] = {
         .sram_banks = 1,
         .num_cpus = 1,
         .sys_version = 0x41743,
+        .cpuwait_rst = 0,
         .sys_config_format = IoTKitFormat,
         .has_mhus = false,
         .has_ppus = false,
@@ -55,6 +58,7 @@ static const ARMSSEInfo armsse_variants[] = {
         .sram_banks = 4,
         .num_cpus = 2,
         .sys_version = 0x22041743,
+        .cpuwait_rst = 2,
         .sys_config_format = SSE200Format,
         .has_mhus = true,
         .has_ppus = true,
@@ -495,30 +499,33 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 
         qdev_prop_set_uint32(cpudev, "num-irq", s->exp_numirq + 32);
         /*
-         * In real hardware the initial Secure VTOR is set from the INITSVTOR0
-         * register in the IoT Kit System Control Register block, and the
-         * initial value of that is in turn specifiable by the FPGA that
-         * instantiates the IoT Kit. In QEMU we don't implement this wrinkle,
-         * and simply set the CPU's init-svtor to the IoT Kit default value.
-         * In SSE-200 the situation is similar, except that the default value
-         * is a reset-time signal input. Typically a board using the SSE-200
-         * will have a system control processor whose boot firmware initializes
-         * the INITSVTOR* registers before powering up the CPUs in any case,
-         * so the hardware's default value doesn't matter. QEMU doesn't emulate
+         * In real hardware the initial Secure VTOR is set from the INITSVTOR*
+         * registers in the IoT Kit System Control Register block. In QEMU
+         * we set the initial value here, and also the reset value of the
+         * sysctl register, from this object's QOM init-svtor property.
+         * If the guest changes the INITSVTOR* registers at runtime then the
+         * code in iotkit-sysctl.c will update the CPU init-svtor property
+         * (which will then take effect on the next CPU warm-reset).
+         *
+         * Note that typically a board using the SSE-200 will have a system
+         * control processor whose boot firmware initializes the INITSVTOR*
+         * registers before powering up the CPUs. QEMU doesn't emulate
          * the control processor, so instead we behave in the way that the
-         * firmware does. The initial value is configurable by the board code
-         * to match whatever its firmware does.
+         * firmware does: the initial value should be set by the board code
+         * (using the init-svtor property on the ARMSSE object) to match
+         * whatever its firmware does.
          */
         qdev_prop_set_uint32(cpudev, "init-svtor", s->init_svtor);
         /*
-         * Start all CPUs except CPU0 powered down. In real hardware it is
-         * a configurable property of the SSE-200 which CPUs start powered up
-         * (via the CPUWAIT0_RST and CPUWAIT1_RST parameters), but since all
-         * the boards we care about start CPU0 and leave CPU1 powered off,
-         * we hard-code that for now. We can add QOM properties for this
+         * CPUs start powered down if the corresponding bit in the CPUWAIT
+         * register is 1. In real hardware the CPUWAIT register reset value is
+         * a configurable property of the SSE-200 (via the CPUWAIT0_RST and
+         * CPUWAIT1_RST parameters), but since all the boards we care about
+         * start CPU0 and leave CPU1 powered off, we hard-code that in
+         * info->cpuwait_rst for now. We can add QOM properties for this
          * later if necessary.
          */
-        if (i > 0) {
+        if (extract32(info->cpuwait_rst, i, 1)) {
             object_property_set_bool(cpuobj, true, "start-powered-off", &err);
             if (err) {
                 error_propagate(errp, err);
@@ -999,6 +1006,12 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     /* System control registers */
     object_property_set_int(OBJECT(&s->sysctl), info->sys_version,
                             "SYS_VERSION", &err);
+    object_property_set_int(OBJECT(&s->sysctl), info->cpuwait_rst,
+                            "CPUWAIT_RST", &err);
+    object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
+                            "INITSVTOR0_RST", &err);
+    object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
+                            "INITSVTOR1_RST", &err);
     object_property_set_bool(OBJECT(&s->sysctl), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/misc/iotkit-sysctl.c b/hw/misc/iotkit-sysctl.c
index e333c8169a3..54064a31ef0 100644
--- a/hw/misc/iotkit-sysctl.c
+++ b/hw/misc/iotkit-sysctl.c
@@ -404,18 +404,9 @@ static void iotkit_sysctl_reset(DeviceState *dev)
     s->reset_syndrome = 1;
     s->reset_mask = 0;
     s->gretreg = 0;
-    s->initsvtor0 = 0x10000000;
-    s->initsvtor1 = 0x10000000;
-    if (s->is_sse200) {
-        /*
-         * CPU 0 starts on, CPU 1 starts off. In real hardware this is
-         * configurable by the SoC integrator as a verilog parameter.
-         */
-        s->cpuwait = 2;
-    } else {
-        /* CPU 0 starts on */
-        s->cpuwait = 0;
-    }
+    s->initsvtor0 = s->initsvtor0_rst;
+    s->initsvtor1 = s->initsvtor1_rst;
+    s->cpuwait = s->cpuwait_rst;
     s->wicctrl = 0;
     s->scsecctrl = 0;
     s->fclk_div = 0;
@@ -500,6 +491,11 @@ static const VMStateDescription iotkit_sysctl_vmstate = {
 
 static Property iotkit_sysctl_props[] = {
     DEFINE_PROP_UINT32("SYS_VERSION", IoTKitSysCtl, sys_version, 0),
+    DEFINE_PROP_UINT32("CPUWAIT_RST", IoTKitSysCtl, cpuwait_rst, 0),
+    DEFINE_PROP_UINT32("INITSVTOR0_RST", IoTKitSysCtl, initsvtor0_rst,
+                       0x10000000),
+    DEFINE_PROP_UINT32("INITSVTOR1_RST", IoTKitSysCtl, initsvtor1_rst,
+                       0x10000000),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/8] hw/misc/armsse-mhu.c: Model the SSE-200 Message Handling Unit

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> Implement a model of the Message Handling Unit (MHU) found in
> the Arm SSE-200. This is a simple device which just contains
> some registers which allow the two cores of the SSE-200
> to raise interrupts on each other.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/misc/Makefile.objs           |   1 +
>  include/hw/misc/armsse-mhu.h    |  44 +++++++
>  hw/misc/armsse-mhu.c            | 198 ++++++++++++++++++++++++++++++++
>  MAINTAINERS                     |   2 +
>  default-configs/arm-softmmu.mak |   1 +
>  hw/misc/trace-events            |   4 +
>  6 files changed, 250 insertions(+)
>  create mode 100644 include/hw/misc/armsse-mhu.h
>  create mode 100644 hw/misc/armsse-mhu.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 2/8] hw/arm/armsse: Wire up the MHUs

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> Create and connect the MHUs in the SSE-200.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armsse.h |  3 ++-
>  hw/arm/armsse.c         | 40 ++++++++++++++++++++++++++++++----------
>  2 files changed, 32 insertions(+), 11 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> +        /*
> +         * An SSE-200 with only one CPU should have only one MHU created,
> +         * with the region where the second MHU usually is being RAZ/WI.
> +         * We don't implement that SSE-200 config; if we want to support
> +         * it then this code needs to be enhanced to handle creating the
> +         * RAZ/WI region instead of the second MHU.
> +         */
> +        assert(info->num_cpus > 1);
> +
> +        for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {

Nit: x > 1 is probably better as x == ARRAY_SIZE(s->mhu).


r~

Re: [Qemu-devel] [PATCH 3/8] target/arm/cpu: Allow init-svtor property to be set after realize

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> +static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
> +                               void *opaque, Error **errp)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    visit_type_uint32(v, name, &cpu->init_svtor, errp);
> +}

Since there's no validation needed, can this use object_property_add_uint32_ptr?

That said, this isn't wrong so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 4/8] target/arm/arm-powerctl: Add new arm_set_cpu_on_and_reset()

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> Currently the Arm arm-powerctl.h APIs allow:
>  * arm_set_cpu_on(), which powers on a CPU and sets its
>    initial PC and other startup state
>  * arm_reset_cpu(), which resets a CPU which is already on
>    (and fails if the CPU is powered off)
> 
> but there is no way to say "power on a CPU as if it had
> just come out of reset and don't do anything else to it".
> 
> Add a new function arm_set_cpu_on_and_reset(), which does this.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/arm-powerctl.h | 16 +++++++++++
>  target/arm/arm-powerctl.c | 56 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 5/8] hw/misc/iotkit-sysctl: Correct typo in INITSVTOR0 register name

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> The iotkit-sysctl device has a register it names INITSVRTOR0.
> This is actually a typo present in the IoTKit documentation
> and also in part of the SSE-200 documentation:  it should be
> INITSVTOR0 because it is specifying the initial value of the
> Secure VTOR register in the CPU. Correct the typo.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/misc/iotkit-sysctl.h |  2 +-
>  hw/misc/iotkit-sysctl.c         | 16 ++++++++--------
>  2 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 6/8] hw/arm/iotkit-sysctl: Add SSE-200 registers

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> The SYSCTL block in the SSE-200 has some extra registers that
> are not present in the IoTKit version. Add these registers
> (as reads-as-written stubs), enabled by a new QOM property.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/misc/iotkit-sysctl.h |  20 +++
>  hw/arm/armsse.c                 |   2 +
>  hw/misc/iotkit-sysctl.c         | 245 +++++++++++++++++++++++++++++++-
>  3 files changed, 262 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 7/8] hw/arm/iotkit-sysctl: Implement CPUWAIT and INITSVTOR*

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> The CPUWAIT register acts as a sort of power-control: if a bit
> in it is 1 then the CPU will have been forced into waiting
> when the system was reset (which in QEMU we model as the
> CPU starting powered off). Writing a 0 to the register will
> allow the CPU to boot (for QEMU, we model this as powering
> it on). Note that writing 0 to the register does not power
> off a CPU.
> 
> For this to work correctly we need to also honour the
> INITSVTOR* registers, which let the guest control where the
> CPU will load its SP and PC from when it comes out of reset.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/misc/iotkit-sysctl.c | 41 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 8/8] hw/arm/armsse: Unify init-svtor and cpuwait handling

[Richard Henderson]

On 2/19/19 4:58 AM, Peter Maydell wrote:
> At the moment the handling of init-svtor and cpuwait initial
> values is split between armsse.c and iotkit-sysctl.c:
> the code in armsse.c sets the initial state of the CPU
> object by setting the init-svtor and start-powered-off
> properties, but the iotkit-sysctl.c code has its own
> code setting the reset values of its registers (which are
> then used when updating the CPU when the guest makes
> runtime changes).
> 
> Clean this up by making the armsse.c code set properties on the
> iotkit-sysctl object to define the initial values of the
> registers, so they always match the initial CPU state,
> and update the comments in armsse.c accordingly.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/misc/iotkit-sysctl.h |  3 ++
>  hw/arm/armsse.c                 | 49 +++++++++++++++++++++------------
>  hw/misc/iotkit-sysctl.c         | 20 ++++++--------
>  3 files changed, 42 insertions(+), 30 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 3/8] target/arm/cpu: Allow init-svtor property to be set after realize

[Peter Maydell]

On Wed, 20 Feb 2019 at 18:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/19/19 4:58 AM, Peter Maydell wrote:
> > +static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
> > +                               void *opaque, Error **errp)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    visit_type_uint32(v, name, &cpu->init_svtor, errp);
> > +}
>
> Since there's no validation needed, can this use object_property_add_uint32_ptr?

I tried that first, but it doesn't let you set the property,
only read it...

thanks
-- PMM