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

target-arm queue:
 * Model the Arm "Musca" development boards: "musca-a" and "musca-b1"
 * Implement the ARMv8.3-JSConv extension
 * v8M MPU should use background region as default, not always
 * Stop unintentional sign extension in pmu_init

# gpg: Signature made Thu 21 Feb 2019 18:56:32 GMT
# gpg:                using RSA key E1A5C593CD419DE28E8315CF3C2525ED14360CDE
# gpg:                issuer "peter.maydell@linaro.org"
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>" [ultimate]
# gpg:                 aka "Peter Maydell <pmaydell@gmail.com>" [ultimate]
# gpg:                 aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>" [ultimate]
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83  15CF 3C25 25ED 1436 0CDE

* remotes/pmaydell/tags/pull-target-arm-20190221: (21 commits)
  hw/arm/armsse: Make 0x5... alias region work for per-CPU devices
  hw/arm/musca: Wire up PL011 UARTs
  hw/arm/musca: Wire up PL031 RTC
  hw/arm/musca: Add MPCs
  hw/arm/musca: Add PPCs
  hw/arm/musca.c: Implement models of the Musca-A and -B1 boards
  hw/arm/armsse: Allow boards to specify init-svtor
  hw/arm/armsse: Document SRAM_ADDR_WIDTH property in header comment
  hw/char/pl011: Use '0x' prefix when logging hex numbers
  hw/char/pl011: Support all interrupt lines
  hw/char/pl011: Allow use as an embedded-struct device
  hw/timer/pl031: Convert to using trace events
  hw/timer/pl031: Allow use as an embedded-struct device
  hw/misc/tz-ppc: Support having unused ports in the middle of the range
  target/arm: Implement ARMv8.3-JSConv
  target/arm: Rearrange Floating-point data-processing (2 regs)
  target/arm: Split out vfp_helper.c
  target/arm: Restructure disas_fp_int_conv
  target/arm: Stop unintentional sign extension in pmu_init
  target/arm: v8M MPU should use background region as default, not always
  ...

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

7 files changed, 813 insertions, 100 deletions

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fa40e8d641..fa57c7c770 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -35,6 +35,7 @@ obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
 obj-$(CONFIG_MPS2) += mps2.o
 obj-$(CONFIG_MPS2) += mps2-tz.o
 obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
+obj-$(CONFIG_MUSCA) += musca.o
 obj-$(CONFIG_ARMSSE) += armsse.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 9a8c49547d..129e7ea7fe 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -110,15 +110,16 @@ static bool irq_is_common[32] = {
     /* 30, 31: reserved */
 };
 
-/* Create an alias region of @size bytes starting at @base
+/*
+ * Create an alias region in @container of @size bytes starting at @base
  * which mirrors the memory starting at @orig.
  */
-static void make_alias(ARMSSE *s, MemoryRegion *mr, const char *name,
-                       hwaddr base, hwaddr size, hwaddr orig)
+static void make_alias(ARMSSE *s, MemoryRegion *mr, MemoryRegion *container,
+                       const char *name, hwaddr base, hwaddr size, hwaddr orig)
 {
-    memory_region_init_alias(mr, NULL, name, &s->container, orig, size);
+    memory_region_init_alias(mr, NULL, name, container, orig, size);
     /* The alias is even lower priority than unimplemented_device regions */
-    memory_region_add_subregion_overlap(&s->container, base, mr, -1500);
+    memory_region_add_subregion_overlap(container, base, mr, -1500);
 }
 
 static void irq_status_forwarder(void *opaque, int n, int level)
@@ -505,11 +506,10 @@ static void armsse_realize(DeviceState *dev, Error **errp)
          * the INITSVTOR* registers before powering up the CPUs in any case,
          * so the hardware's default value doesn't matter. QEMU doesn't emulate
          * the control processor, so instead we behave in the way that the
-         * firmware does. All boards currently known about have firmware that
-         * sets the INITSVTOR0 and INITSVTOR1 registers to 0x10000000, like the
-         * IoTKit default. We can make this more configurable if necessary.
+         * firmware does. The initial value is configurable by the board code
+         * to match whatever its firmware does.
          */
-        qdev_prop_set_uint32(cpudev, "init-svtor", 0x10000000);
+        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
@@ -608,16 +608,21 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     }
 
     /* Set up the big aliases first */
-    make_alias(s, &s->alias1, "alias 1", 0x10000000, 0x10000000, 0x00000000);
-    make_alias(s, &s->alias2, "alias 2", 0x30000000, 0x10000000, 0x20000000);
+    make_alias(s, &s->alias1, &s->container, "alias 1",
+               0x10000000, 0x10000000, 0x00000000);
+    make_alias(s, &s->alias2, &s->container,
+               "alias 2", 0x30000000, 0x10000000, 0x20000000);
     /* The 0x50000000..0x5fffffff region is not a pure alias: it has
      * a few extra devices that only appear there (generally the
      * control interfaces for the protection controllers).
      * We implement this by mapping those devices over the top of this
-     * alias MR at a higher priority.
+     * alias MR at a higher priority. Some of the devices in this range
+     * are per-CPU, so we must put this alias in the per-cpu containers.
      */
-    make_alias(s, &s->alias3, "alias 3", 0x50000000, 0x10000000, 0x40000000);
-
+    for (i = 0; i < info->num_cpus; i++) {
+        make_alias(s, &s->alias3[i], &s->cpu_container[i],
+                   "alias 3", 0x50000000, 0x10000000, 0x40000000);
+    }
 
     /* Security controller */
     object_property_set_bool(OBJECT(&s->secctl), true, "realized", &err);
@@ -762,26 +767,28 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 
     if (info->has_mhus) {
         for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {
-            char *name = g_strdup_printf("MHU%d", i);
-            char *port = g_strdup_printf("port[%d]", i + 3);
+            char *name;
+            char *port;
 
+            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);
             object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
                                      port, &err);
+            g_free(port);
             if (err) {
                 error_propagate(errp, err);
                 return;
             }
-            g_free(name);
-            g_free(port);
         }
     }
 
@@ -1185,6 +1192,7 @@ static Property armsse_properties[] = {
     DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
     DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
     DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
+    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/hw/arm/musca.c b/hw/arm/musca.c
new file mode 100644
index 0000000000..23aff43f4b
--- /dev/null
+++ b/hw/arm/musca.c
@@ -0,0 +1,669 @@
+/*
+ * Arm Musca-B1 test chip board emulation
+ *
+ * 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.
+ */
+
+/*
+ * The Musca boards are a reference implementation of a system using
+ * the SSE-200 subsystem for embedded:
+ * https://developer.arm.com/products/system-design/development-boards/iot-test-chips-and-boards/musca-a-test-chip-board
+ * https://developer.arm.com/products/system-design/development-boards/iot-test-chips-and-boards/musca-b-test-chip-board
+ * We model the A and B1 variants of this board, as described in the TRMs:
+ * http://infocenter.arm.com/help/topic/com.arm.doc.101107_0000_00_en/index.html
+ * http://infocenter.arm.com/help/topic/com.arm.doc.101312_0000_00_en/index.html
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+#include "sysemu/sysemu.h"
+#include "hw/arm/arm.h"
+#include "hw/arm/armsse.h"
+#include "hw/boards.h"
+#include "hw/char/pl011.h"
+#include "hw/core/split-irq.h"
+#include "hw/misc/tz-mpc.h"
+#include "hw/misc/tz-ppc.h"
+#include "hw/misc/unimp.h"
+#include "hw/timer/pl031.h"
+
+#define MUSCA_NUMIRQ_MAX 96
+#define MUSCA_PPC_MAX 3
+#define MUSCA_MPC_MAX 5
+
+typedef struct MPCInfo MPCInfo;
+
+typedef enum MuscaType {
+    MUSCA_A,
+    MUSCA_B1,
+} MuscaType;
+
+typedef struct {
+    MachineClass parent;
+    MuscaType type;
+    uint32_t init_svtor;
+    int sram_addr_width;
+    int num_irqs;
+    const MPCInfo *mpc_info;
+    int num_mpcs;
+} MuscaMachineClass;
+
+typedef struct {
+    MachineState parent;
+
+    ARMSSE sse;
+    /* RAM and flash */
+    MemoryRegion ram[MUSCA_MPC_MAX];
+    SplitIRQ cpu_irq_splitter[MUSCA_NUMIRQ_MAX];
+    SplitIRQ sec_resp_splitter;
+    TZPPC ppc[MUSCA_PPC_MAX];
+    MemoryRegion container;
+    UnimplementedDeviceState eflash[2];
+    UnimplementedDeviceState qspi;
+    TZMPC mpc[MUSCA_MPC_MAX];
+    UnimplementedDeviceState mhu[2];
+    UnimplementedDeviceState pwm[3];
+    UnimplementedDeviceState i2s;
+    PL011State uart[2];
+    UnimplementedDeviceState i2c[2];
+    UnimplementedDeviceState spi;
+    UnimplementedDeviceState scc;
+    UnimplementedDeviceState timer;
+    PL031State rtc;
+    UnimplementedDeviceState pvt;
+    UnimplementedDeviceState sdio;
+    UnimplementedDeviceState gpio;
+    UnimplementedDeviceState cryptoisland;
+} MuscaMachineState;
+
+#define TYPE_MUSCA_MACHINE "musca"
+#define TYPE_MUSCA_A_MACHINE MACHINE_TYPE_NAME("musca-a")
+#define TYPE_MUSCA_B1_MACHINE MACHINE_TYPE_NAME("musca-b1")
+
+#define MUSCA_MACHINE(obj) \
+    OBJECT_CHECK(MuscaMachineState, obj, TYPE_MUSCA_MACHINE)
+#define MUSCA_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(MuscaMachineClass, obj, TYPE_MUSCA_MACHINE)
+#define MUSCA_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(MuscaMachineClass, klass, TYPE_MUSCA_MACHINE)
+
+/*
+ * Main SYSCLK frequency in Hz
+ * TODO this should really be different for the two cores, but we
+ * don't model that in our SSE-200 model yet.
+ */
+#define SYSCLK_FRQ 40000000
+
+static qemu_irq get_sse_irq_in(MuscaMachineState *mms, int irqno)
+{
+    /* Return a qemu_irq which will signal IRQ n to all CPUs in the SSE. */
+    assert(irqno < MUSCA_NUMIRQ_MAX);
+
+    return qdev_get_gpio_in(DEVICE(&mms->cpu_irq_splitter[irqno]), 0);
+}
+
+/*
+ * Most of the devices in the Musca board sit behind Peripheral Protection
+ * Controllers. These data structures define the layout of which devices
+ * sit behind which PPCs.
+ * The devfn for each port is a function which creates, configures
+ * and initializes the device, returning the MemoryRegion which
+ * needs to be plugged into the downstream end of the PPC port.
+ */
+typedef MemoryRegion *MakeDevFn(MuscaMachineState *mms, void *opaque,
+                                const char *name, hwaddr size);
+
+typedef struct PPCPortInfo {
+    const char *name;
+    MakeDevFn *devfn;
+    void *opaque;
+    hwaddr addr;
+    hwaddr size;
+} PPCPortInfo;
+
+typedef struct PPCInfo {
+    const char *name;
+    PPCPortInfo ports[TZ_NUM_PORTS];
+} PPCInfo;
+
+static MemoryRegion *make_unimp_dev(MuscaMachineState *mms,
+                                    void *opaque, const char *name, hwaddr size)
+{
+    /*
+     * Initialize, configure and realize a TYPE_UNIMPLEMENTED_DEVICE,
+     * and return a pointer to its MemoryRegion.
+     */
+    UnimplementedDeviceState *uds = opaque;
+
+    sysbus_init_child_obj(OBJECT(mms), name, uds,
+                          sizeof(UnimplementedDeviceState),
+                          TYPE_UNIMPLEMENTED_DEVICE);
+    qdev_prop_set_string(DEVICE(uds), "name", name);
+    qdev_prop_set_uint64(DEVICE(uds), "size", size);
+    object_property_set_bool(OBJECT(uds), true, "realized", &error_fatal);
+    return sysbus_mmio_get_region(SYS_BUS_DEVICE(uds), 0);
+}
+
+typedef enum MPCInfoType {
+    MPC_RAM,
+    MPC_ROM,
+    MPC_CRYPTOISLAND,
+} MPCInfoType;
+
+struct MPCInfo {
+    const char *name;
+    hwaddr addr;
+    hwaddr size;
+    MPCInfoType type;
+};
+
+/* Order of the MPCs here must match the order of the bits in SECMPCINTSTATUS */
+static const MPCInfo a_mpc_info[] = { {
+        .name = "qspi",
+        .type = MPC_ROM,
+        .addr = 0x00200000,
+        .size = 0x00800000,
+    }, {
+        .name = "sram",
+        .type = MPC_RAM,
+        .addr = 0x00000000,
+        .size = 0x00200000,
+    }
+};
+
+static const MPCInfo b1_mpc_info[] = { {
+        .name = "qspi",
+        .type = MPC_ROM,
+        .addr = 0x00000000,
+        .size = 0x02000000,
+    }, {
+        .name = "sram",
+        .type = MPC_RAM,
+        .addr = 0x0a400000,
+        .size = 0x00080000,
+    }, {
+        .name = "eflash0",
+        .type = MPC_ROM,
+        .addr = 0x0a000000,
+        .size = 0x00200000,
+    }, {
+        .name = "eflash1",
+        .type = MPC_ROM,
+        .addr = 0x0a200000,
+        .size = 0x00200000,
+    }, {
+        .name = "cryptoisland",
+        .type = MPC_CRYPTOISLAND,
+        .addr = 0x0a000000,
+        .size = 0x00200000,
+    }
+};
+
+static MemoryRegion *make_mpc(MuscaMachineState *mms, void *opaque,
+                              const char *name, hwaddr size)
+{
+    /*
+     * Create an MPC and the RAM or flash behind it.
+     * MPC 0: eFlash 0
+     * MPC 1: eFlash 1
+     * MPC 2: SRAM
+     * MPC 3: QSPI flash
+     * MPC 4: CryptoIsland
+     * For now we implement the flash regions as ROM (ie not programmable)
+     * (with their control interface memory regions being unimplemented
+     * stubs behind the PPCs).
+     * The whole CryptoIsland region behind its MPC is an unimplemented stub.
+     */
+    MuscaMachineClass *mmc = MUSCA_MACHINE_GET_CLASS(mms);
+    TZMPC *mpc = opaque;
+    int i = mpc - &mms->mpc[0];
+    MemoryRegion *downstream;
+    MemoryRegion *upstream;
+    UnimplementedDeviceState *uds;
+    char *mpcname;
+    const MPCInfo *mpcinfo = mmc->mpc_info;
+
+    mpcname = g_strdup_printf("%s-mpc", mpcinfo[i].name);
+
+    switch (mpcinfo[i].type) {
+    case MPC_ROM:
+        downstream = &mms->ram[i];
+        memory_region_init_rom(downstream, NULL, mpcinfo[i].name,
+                               mpcinfo[i].size, &error_fatal);
+        break;
+    case MPC_RAM:
+        downstream = &mms->ram[i];
+        memory_region_init_ram(downstream, NULL, mpcinfo[i].name,
+                               mpcinfo[i].size, &error_fatal);
+        break;
+    case MPC_CRYPTOISLAND:
+        /* We don't implement the CryptoIsland yet */
+        uds = &mms->cryptoisland;
+        sysbus_init_child_obj(OBJECT(mms), name, uds,
+                              sizeof(UnimplementedDeviceState),
+                              TYPE_UNIMPLEMENTED_DEVICE);
+        qdev_prop_set_string(DEVICE(uds), "name", mpcinfo[i].name);
+        qdev_prop_set_uint64(DEVICE(uds), "size", mpcinfo[i].size);
+        object_property_set_bool(OBJECT(uds), true, "realized", &error_fatal);
+        downstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(uds), 0);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    sysbus_init_child_obj(OBJECT(mms), mpcname, mpc, sizeof(mms->mpc[0]),
+                          TYPE_TZ_MPC);
+    object_property_set_link(OBJECT(mpc), OBJECT(downstream),
+                             "downstream", &error_fatal);
+    object_property_set_bool(OBJECT(mpc), true, "realized", &error_fatal);
+    /* Map the upstream end of the MPC into system memory */
+    upstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 1);
+    memory_region_add_subregion(get_system_memory(), mpcinfo[i].addr, upstream);
+    /* and connect its interrupt to the SSE-200 */
+    qdev_connect_gpio_out_named(DEVICE(mpc), "irq", 0,
+                                qdev_get_gpio_in_named(DEVICE(&mms->sse),
+                                                       "mpcexp_status", i));
+
+    g_free(mpcname);
+    /* Return the register interface MR for our caller to map behind the PPC */
+    return sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 0);
+}
+
+static MemoryRegion *make_rtc(MuscaMachineState *mms, void *opaque,
+                              const char *name, hwaddr size)
+{
+    PL031State *rtc = opaque;
+
+    sysbus_init_child_obj(OBJECT(mms), name, rtc, sizeof(mms->rtc), TYPE_PL031);
+    object_property_set_bool(OBJECT(rtc), true, "realized", &error_fatal);
+    sysbus_connect_irq(SYS_BUS_DEVICE(rtc), 0, get_sse_irq_in(mms, 39));
+    return sysbus_mmio_get_region(SYS_BUS_DEVICE(rtc), 0);
+}
+
+static MemoryRegion *make_uart(MuscaMachineState *mms, void *opaque,
+                               const char *name, hwaddr size)
+{
+    PL011State *uart = opaque;
+    int i = uart - &mms->uart[0];
+    int irqbase = 7 + i * 6;
+    SysBusDevice *s;
+
+    sysbus_init_child_obj(OBJECT(mms), name, uart, sizeof(mms->uart[0]),
+                          TYPE_PL011);
+    qdev_prop_set_chr(DEVICE(uart), "chardev", serial_hd(i));
+    object_property_set_bool(OBJECT(uart), true, "realized", &error_fatal);
+    s = SYS_BUS_DEVICE(uart);
+    sysbus_connect_irq(s, 0, get_sse_irq_in(mms, irqbase + 5)); /* combined */
+    sysbus_connect_irq(s, 1, get_sse_irq_in(mms, irqbase + 0)); /* RX */
+    sysbus_connect_irq(s, 2, get_sse_irq_in(mms, irqbase + 1)); /* TX */
+    sysbus_connect_irq(s, 3, get_sse_irq_in(mms, irqbase + 2)); /* RT */
+    sysbus_connect_irq(s, 4, get_sse_irq_in(mms, irqbase + 3)); /* MS */
+    sysbus_connect_irq(s, 5, get_sse_irq_in(mms, irqbase + 4)); /* E */
+    return sysbus_mmio_get_region(SYS_BUS_DEVICE(uart), 0);
+}
+
+static MemoryRegion *make_musca_a_devs(MuscaMachineState *mms, void *opaque,
+                                       const char *name, hwaddr size)
+{
+    /*
+     * Create the container MemoryRegion for all the devices that live
+     * behind the Musca-A PPC's single port. These devices don't have a PPC
+     * port each, but we use the PPCPortInfo struct as a convenient way
+     * to describe them. Note that addresses here are relative to the base
+     * address of the PPC port region: 0x40100000, and devices appear both
+     * at the 0x4... NS region and the 0x5... S region.
+     */
+    int i;
+    MemoryRegion *container = &mms->container;
+
+    const PPCPortInfo devices[] = {
+        { "uart0", make_uart, &mms->uart[0], 0x1000, 0x1000 },
+        { "uart1", make_uart, &mms->uart[1], 0x2000, 0x1000 },
+        { "spi", make_unimp_dev, &mms->spi, 0x3000, 0x1000 },
+        { "i2c0", make_unimp_dev, &mms->i2c[0], 0x4000, 0x1000 },
+        { "i2c1", make_unimp_dev, &mms->i2c[1], 0x5000, 0x1000 },
+        { "i2s", make_unimp_dev, &mms->i2s, 0x6000, 0x1000 },
+        { "pwm0", make_unimp_dev, &mms->pwm[0], 0x7000, 0x1000 },
+        { "rtc", make_rtc, &mms->rtc, 0x8000, 0x1000 },
+        { "qspi", make_unimp_dev, &mms->qspi, 0xa000, 0x1000 },
+        { "timer", make_unimp_dev, &mms->timer, 0xb000, 0x1000 },
+        { "scc", make_unimp_dev, &mms->scc, 0xc000, 0x1000 },
+        { "pwm1", make_unimp_dev, &mms->pwm[1], 0xe000, 0x1000 },
+        { "pwm2", make_unimp_dev, &mms->pwm[2], 0xf000, 0x1000 },
+        { "gpio", make_unimp_dev, &mms->gpio, 0x10000, 0x1000 },
+        { "mpc0", make_mpc, &mms->mpc[0], 0x12000, 0x1000 },
+        { "mpc1", make_mpc, &mms->mpc[1], 0x13000, 0x1000 },
+    };
+
+    memory_region_init(container, OBJECT(mms), "musca-device-container", size);
+
+    for (i = 0; i < ARRAY_SIZE(devices); i++) {
+        const PPCPortInfo *pinfo = &devices[i];
+        MemoryRegion *mr;
+
+        mr = pinfo->devfn(mms, pinfo->opaque, pinfo->name, pinfo->size);
+        memory_region_add_subregion(container, pinfo->addr, mr);
+    }
+
+    return &mms->container;
+}
+
+static void musca_init(MachineState *machine)
+{
+    MuscaMachineState *mms = MUSCA_MACHINE(machine);
+    MuscaMachineClass *mmc = MUSCA_MACHINE_GET_CLASS(mms);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    MemoryRegion *system_memory = get_system_memory();
+    DeviceState *ssedev;
+    DeviceState *dev_splitter;
+    const PPCInfo *ppcs;
+    int num_ppcs;
+    int i;
+
+    assert(mmc->num_irqs <= MUSCA_NUMIRQ_MAX);
+    assert(mmc->num_mpcs <= MUSCA_MPC_MAX);
+
+    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
+        error_report("This board can only be used with CPU %s",
+                     mc->default_cpu_type);
+        exit(1);
+    }
+
+    sysbus_init_child_obj(OBJECT(machine), "sse-200", &mms->sse,
+                          sizeof(mms->sse), TYPE_SSE200);
+    ssedev = DEVICE(&mms->sse);
+    object_property_set_link(OBJECT(&mms->sse), OBJECT(system_memory),
+                             "memory", &error_fatal);
+    qdev_prop_set_uint32(ssedev, "EXP_NUMIRQ", mmc->num_irqs);
+    qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor);
+    qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width);
+    qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ);
+    object_property_set_bool(OBJECT(&mms->sse), true, "realized",
+                             &error_fatal);
+
+    /*
+     * We need to create splitters to feed the IRQ inputs
+     * for each CPU in the SSE-200 from each device in the board.
+     */
+    for (i = 0; i < mmc->num_irqs; i++) {
+        char *name = g_strdup_printf("musca-irq-splitter%d", i);
+        SplitIRQ *splitter = &mms->cpu_irq_splitter[i];
+
+        object_initialize_child(OBJECT(machine), name,
+                                splitter, sizeof(*splitter),
+                                TYPE_SPLIT_IRQ, &error_fatal, NULL);
+        g_free(name);
+
+        object_property_set_int(OBJECT(splitter), 2, "num-lines",
+                                &error_fatal);
+        object_property_set_bool(OBJECT(splitter), true, "realized",
+                                 &error_fatal);
+        qdev_connect_gpio_out(DEVICE(splitter), 0,
+                              qdev_get_gpio_in_named(ssedev, "EXP_IRQ", i));
+        qdev_connect_gpio_out(DEVICE(splitter), 1,
+                              qdev_get_gpio_in_named(ssedev,
+                                                     "EXP_CPU1_IRQ", i));
+    }
+
+    /*
+     * The sec_resp_cfg output from the SSE-200 must be split into multiple
+     * lines, one for each of the PPCs we create here.
+     */
+    object_initialize(&mms->sec_resp_splitter, sizeof(mms->sec_resp_splitter),
+                      TYPE_SPLIT_IRQ);
+    object_property_add_child(OBJECT(machine), "sec-resp-splitter",
+                              OBJECT(&mms->sec_resp_splitter), &error_fatal);
+    object_property_set_int(OBJECT(&mms->sec_resp_splitter),
+                            ARRAY_SIZE(mms->ppc), "num-lines", &error_fatal);
+    object_property_set_bool(OBJECT(&mms->sec_resp_splitter), true,
+                             "realized", &error_fatal);
+    dev_splitter = DEVICE(&mms->sec_resp_splitter);
+    qdev_connect_gpio_out_named(ssedev, "sec_resp_cfg", 0,
+                                qdev_get_gpio_in(dev_splitter, 0));
+
+    /*
+     * Most of the devices in the board are behind Peripheral Protection
+     * Controllers. The required order for initializing things is:
+     *  + initialize the PPC
+     *  + initialize, configure and realize downstream devices
+     *  + connect downstream device MemoryRegions to the PPC
+     *  + realize the PPC
+     *  + map the PPC's MemoryRegions to the places in the address map
+     *    where the downstream devices should appear
+     *  + wire up the PPC's control lines to the SSE object
+     *
+     * The PPC mapping differs for the -A and -B1 variants; the -A version
+     * is much simpler, using only a single port of a single PPC and putting
+     * all the devices behind that.
+     */
+    const PPCInfo a_ppcs[] = { {
+            .name = "ahb_ppcexp0",
+            .ports = {
+                { "musca-devices", make_musca_a_devs, 0, 0x40100000, 0x100000 },
+            },
+        },
+    };
+
+    /*
+     * Devices listed with an 0x4.. address appear in both the NS 0x4.. region
+     * and the 0x5.. S region. Devices listed with an 0x5.. address appear
+     * only in the S region.
+     */
+    const PPCInfo b1_ppcs[] = { {
+            .name = "apb_ppcexp0",
+            .ports = {
+                { "eflash0", make_unimp_dev, &mms->eflash[0],
+                  0x52400000, 0x1000 },
+                { "eflash1", make_unimp_dev, &mms->eflash[1],
+                  0x52500000, 0x1000 },
+                { "qspi", make_unimp_dev, &mms->qspi, 0x42800000, 0x100000 },
+                { "mpc0", make_mpc, &mms->mpc[0], 0x52000000, 0x1000 },
+                { "mpc1", make_mpc, &mms->mpc[1], 0x52100000, 0x1000 },
+                { "mpc2", make_mpc, &mms->mpc[2], 0x52200000, 0x1000 },
+                { "mpc3", make_mpc, &mms->mpc[3], 0x52300000, 0x1000 },
+                { "mhu0", make_unimp_dev, &mms->mhu[0], 0x42600000, 0x100000 },
+                { "mhu1", make_unimp_dev, &mms->mhu[1], 0x42700000, 0x100000 },
+                { }, /* port 9: unused */
+                { }, /* port 10: unused */
+                { }, /* port 11: unused */
+                { }, /* port 12: unused */
+                { }, /* port 13: unused */
+                { "mpc4", make_mpc, &mms->mpc[4], 0x52e00000, 0x1000 },
+            },
+        }, {
+            .name = "apb_ppcexp1",
+            .ports = {
+                { "pwm0", make_unimp_dev, &mms->pwm[0], 0x40101000, 0x1000 },
+                { "pwm1", make_unimp_dev, &mms->pwm[1], 0x40102000, 0x1000 },
+                { "pwm2", make_unimp_dev, &mms->pwm[2], 0x40103000, 0x1000 },
+                { "i2s", make_unimp_dev, &mms->i2s, 0x40104000, 0x1000 },
+                { "uart0", make_uart, &mms->uart[0], 0x40105000, 0x1000 },
+                { "uart1", make_uart, &mms->uart[1], 0x40106000, 0x1000 },
+                { "i2c0", make_unimp_dev, &mms->i2c[0], 0x40108000, 0x1000 },
+                { "i2c1", make_unimp_dev, &mms->i2c[1], 0x40109000, 0x1000 },
+                { "spi", make_unimp_dev, &mms->spi, 0x4010a000, 0x1000 },
+                { "scc", make_unimp_dev, &mms->scc, 0x5010b000, 0x1000 },
+                { "timer", make_unimp_dev, &mms->timer, 0x4010c000, 0x1000 },
+                { "rtc", make_rtc, &mms->rtc, 0x4010d000, 0x1000 },
+                { "pvt", make_unimp_dev, &mms->pvt, 0x4010e000, 0x1000 },
+                { "sdio", make_unimp_dev, &mms->sdio, 0x4010f000, 0x1000 },
+            },
+        }, {
+            .name = "ahb_ppcexp0",
+            .ports = {
+                { }, /* port 0: unused */
+                { "gpio", make_unimp_dev, &mms->gpio, 0x41000000, 0x1000 },
+            },
+        },
+    };
+
+    switch (mmc->type) {
+    case MUSCA_A:
+        ppcs = a_ppcs;
+        num_ppcs = ARRAY_SIZE(a_ppcs);
+        break;
+    case MUSCA_B1:
+        ppcs = b1_ppcs;
+        num_ppcs = ARRAY_SIZE(b1_ppcs);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    assert(num_ppcs <= MUSCA_PPC_MAX);
+
+    for (i = 0; i < num_ppcs; i++) {
+        const PPCInfo *ppcinfo = &ppcs[i];
+        TZPPC *ppc = &mms->ppc[i];
+        DeviceState *ppcdev;
+        int port;
+        char *gpioname;
+
+        sysbus_init_child_obj(OBJECT(machine), ppcinfo->name, ppc,
+                              sizeof(TZPPC), TYPE_TZ_PPC);
+        ppcdev = DEVICE(ppc);
+
+        for (port = 0; port < TZ_NUM_PORTS; port++) {
+            const PPCPortInfo *pinfo = &ppcinfo->ports[port];
+            MemoryRegion *mr;
+            char *portname;
+
+            if (!pinfo->devfn) {
+                continue;
+            }
+
+            mr = pinfo->devfn(mms, pinfo->opaque, pinfo->name, pinfo->size);
+            portname = g_strdup_printf("port[%d]", port);
+            object_property_set_link(OBJECT(ppc), OBJECT(mr),
+                                     portname, &error_fatal);
+            g_free(portname);
+        }
+
+        object_property_set_bool(OBJECT(ppc), true, "realized", &error_fatal);
+
+        for (port = 0; port < TZ_NUM_PORTS; port++) {
+            const PPCPortInfo *pinfo = &ppcinfo->ports[port];
+
+            if (!pinfo->devfn) {
+                continue;
+            }
+            sysbus_mmio_map(SYS_BUS_DEVICE(ppc), port, pinfo->addr);
+
+            gpioname = g_strdup_printf("%s_nonsec", ppcinfo->name);
+            qdev_connect_gpio_out_named(ssedev, gpioname, port,
+                                        qdev_get_gpio_in_named(ppcdev,
+                                                               "cfg_nonsec",
+                                                               port));
+            g_free(gpioname);
+            gpioname = g_strdup_printf("%s_ap", ppcinfo->name);
+            qdev_connect_gpio_out_named(ssedev, gpioname, port,
+                                        qdev_get_gpio_in_named(ppcdev,
+                                                               "cfg_ap", port));
+            g_free(gpioname);
+        }
+
+        gpioname = g_strdup_printf("%s_irq_enable", ppcinfo->name);
+        qdev_connect_gpio_out_named(ssedev, gpioname, 0,
+                                    qdev_get_gpio_in_named(ppcdev,
+                                                           "irq_enable", 0));
+        g_free(gpioname);
+        gpioname = g_strdup_printf("%s_irq_clear", ppcinfo->name);
+        qdev_connect_gpio_out_named(ssedev, gpioname, 0,
+                                    qdev_get_gpio_in_named(ppcdev,
+                                                           "irq_clear", 0));
+        g_free(gpioname);
+        gpioname = g_strdup_printf("%s_irq_status", ppcinfo->name);
+        qdev_connect_gpio_out_named(ppcdev, "irq", 0,
+                                    qdev_get_gpio_in_named(ssedev,
+                                                           gpioname, 0));
+        g_free(gpioname);
+
+        qdev_connect_gpio_out(dev_splitter, i,
+                              qdev_get_gpio_in_named(ppcdev,
+                                                     "cfg_sec_resp", 0));
+    }
+
+    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0x2000000);
+}
+
+static void musca_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->default_cpus = 2;
+    mc->min_cpus = mc->default_cpus;
+    mc->max_cpus = mc->default_cpus;
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+    mc->init = musca_init;
+}
+
+static void musca_a_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    MuscaMachineClass *mmc = MUSCA_MACHINE_CLASS(oc);
+
+    mc->desc = "ARM Musca-A board (dual Cortex-M33)";
+    mmc->type = MUSCA_A;
+    mmc->init_svtor = 0x10200000;
+    mmc->sram_addr_width = 15;
+    mmc->num_irqs = 64;
+    mmc->mpc_info = a_mpc_info;
+    mmc->num_mpcs = ARRAY_SIZE(a_mpc_info);
+}
+
+static void musca_b1_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    MuscaMachineClass *mmc = MUSCA_MACHINE_CLASS(oc);
+
+    mc->desc = "ARM Musca-B1 board (dual Cortex-M33)";
+    mmc->type = MUSCA_B1;
+    /*
+     * This matches the DAPlink firmware which boots from QSPI. There
+     * is also a firmware blob which boots from the eFlash, which
+     * uses init_svtor = 0x1A000000. QEMU doesn't currently support that,
+     * though we could in theory expose a machine property on the command
+     * line to allow the user to request eFlash boot.
+     */
+    mmc->init_svtor = 0x10000000;
+    mmc->sram_addr_width = 17;
+    mmc->num_irqs = 96;
+    mmc->mpc_info = b1_mpc_info;
+    mmc->num_mpcs = ARRAY_SIZE(b1_mpc_info);
+}
+
+static const TypeInfo musca_info = {
+    .name = TYPE_MUSCA_MACHINE,
+    .parent = TYPE_MACHINE,
+    .abstract = true,
+    .instance_size = sizeof(MuscaMachineState),
+    .class_size = sizeof(MuscaMachineClass),
+    .class_init = musca_class_init,
+};
+
+static const TypeInfo musca_a_info = {
+    .name = TYPE_MUSCA_A_MACHINE,
+    .parent = TYPE_MUSCA_MACHINE,
+    .class_init = musca_a_class_init,
+};
+
+static const TypeInfo musca_b1_info = {
+    .name = TYPE_MUSCA_B1_MACHINE,
+    .parent = TYPE_MUSCA_MACHINE,
+    .class_init = musca_b1_class_init,
+};
+
+static void musca_machine_init(void)
+{
+    type_register_static(&musca_info);
+    type_register_static(&musca_a_info);
+    type_register_static(&musca_b1_info);
+}
+
+type_init(musca_machine_init);
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 2aa277fc4f..e5dd448f85 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -7,40 +7,24 @@
  * This code is licensed under the GPL.
  */
 
+/*
+ * QEMU interface:
+ *  + sysbus MMIO region 0: device registers
+ *  + sysbus IRQ 0: UARTINTR (combined interrupt line)
+ *  + sysbus IRQ 1: UARTRXINTR (receive FIFO interrupt line)
+ *  + sysbus IRQ 2: UARTTXINTR (transmit FIFO interrupt line)
+ *  + sysbus IRQ 3: UARTRTINTR (receive timeout interrupt line)
+ *  + sysbus IRQ 4: UARTMSINTR (momem status interrupt line)
+ *  + sysbus IRQ 5: UARTEINTR (error interrupt line)
+ */
+
 #include "qemu/osdep.h"
+#include "hw/char/pl011.h"
 #include "hw/sysbus.h"
 #include "chardev/char-fe.h"
 #include "qemu/log.h"
 #include "trace.h"
 
-#define TYPE_PL011 "pl011"
-#define PL011(obj) OBJECT_CHECK(PL011State, (obj), TYPE_PL011)
-
-typedef struct PL011State {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    uint32_t readbuff;
-    uint32_t flags;
-    uint32_t lcr;
-    uint32_t rsr;
-    uint32_t cr;
-    uint32_t dmacr;
-    uint32_t int_enabled;
-    uint32_t int_level;
-    uint32_t read_fifo[16];
-    uint32_t ilpr;
-    uint32_t ibrd;
-    uint32_t fbrd;
-    uint32_t ifl;
-    int read_pos;
-    int read_count;
-    int read_trigger;
-    CharBackend chr;
-    qemu_irq irq;
-    const unsigned char *id;
-} PL011State;
-
 #define PL011_INT_TX 0x20
 #define PL011_INT_RX 0x10
 
@@ -49,18 +33,46 @@ typedef struct PL011State {
 #define PL011_FLAG_TXFF 0x20
 #define PL011_FLAG_RXFE 0x10
 
+/* Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC */
+#define INT_OE (1 << 10)
+#define INT_BE (1 << 9)
+#define INT_PE (1 << 8)
+#define INT_FE (1 << 7)
+#define INT_RT (1 << 6)
+#define INT_TX (1 << 5)
+#define INT_RX (1 << 4)
+#define INT_DSR (1 << 3)
+#define INT_DCD (1 << 2)
+#define INT_CTS (1 << 1)
+#define INT_RI (1 << 0)
+#define INT_E (INT_OE | INT_BE | INT_PE | INT_FE)
+#define INT_MS (INT_RI | INT_DSR | INT_DCD | INT_CTS)
+
 static const unsigned char pl011_id_arm[8] =
   { 0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1 };
 static const unsigned char pl011_id_luminary[8] =
   { 0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1 };
 
+/* Which bits in the interrupt status matter for each outbound IRQ line ? */
+static const uint32_t irqmask[] = {
+    INT_E | INT_MS | INT_RT | INT_TX | INT_RX, /* combined IRQ */
+    INT_RX,
+    INT_TX,
+    INT_RT,
+    INT_MS,
+    INT_E,
+};
+
 static void pl011_update(PL011State *s)
 {
     uint32_t flags;
+    int i;
 
     flags = s->int_level & s->int_enabled;
     trace_pl011_irq_state(flags != 0);
-    qemu_set_irq(s->irq, flags != 0);
+    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+        qemu_set_irq(s->irq[i], (flags & irqmask[i]) != 0);
+    }
 }
 
 static uint64_t pl011_read(void *opaque, hwaddr offset,
@@ -131,7 +143,7 @@ static uint64_t pl011_read(void *opaque, hwaddr offset,
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
-                      "pl011_read: Bad offset %x\n", (int)offset);
+                      "pl011_read: Bad offset 0x%x\n", (int)offset);
         r = 0;
         break;
     }
@@ -220,7 +232,7 @@ static void pl011_write(void *opaque, hwaddr offset,
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
-                      "pl011_write: Bad offset %x\n", (int)offset);
+                      "pl011_write: Bad offset 0x%x\n", (int)offset);
     }
 }
 
@@ -311,10 +323,13 @@ static void pl011_init(Object *obj)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     PL011State *s = PL011(obj);
+    int i;
 
     memory_region_init_io(&s->iomem, OBJECT(s), &pl011_ops, s, "pl011", 0x1000);
     sysbus_init_mmio(sbd, &s->iomem);
-    sysbus_init_irq(sbd, &s->irq);
+    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+        sysbus_init_irq(sbd, &s->irq[i]);
+    }
 
     s->read_trigger = 1;
     s->ifl = 0x12;
@@ -357,7 +372,7 @@ static void pl011_luminary_init(Object *obj)
 }
 
 static const TypeInfo pl011_luminary_info = {
-    .name          = "pl011_luminary",
+    .name          = TYPE_PL011_LUMINARY,
     .parent        = TYPE_PL011,
     .instance_init = pl011_luminary_init,
 };
diff --git a/hw/misc/tz-ppc.c b/hw/misc/tz-ppc.c
index 3dd045c15f..2e04837bea 100644
--- a/hw/misc/tz-ppc.c
+++ b/hw/misc/tz-ppc.c
@@ -181,6 +181,21 @@ static const MemoryRegionOps tz_ppc_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static bool tz_ppc_dummy_accepts(void *opaque, hwaddr addr,
+                                 unsigned size, bool is_write,
+                                 MemTxAttrs attrs)
+{
+    /*
+     * Board code should never map the upstream end of an unused port,
+     * so we should never try to make a memory access to it.
+     */
+    g_assert_not_reached();
+}
+
+static const MemoryRegionOps tz_ppc_dummy_ops = {
+    .valid.accepts = tz_ppc_dummy_accepts,
+};
+
 static void tz_ppc_reset(DeviceState *dev)
 {
     TZPPC *s = TZ_PPC(dev);
@@ -210,16 +225,33 @@ static void tz_ppc_realize(DeviceState *dev, Error **errp)
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     TZPPC *s = TZ_PPC(dev);
     int i;
+    int max_port = 0;
 
     /* We can't create the upstream end of the port until realize,
      * as we don't know the size of the MR used as the downstream until then.
      */
     for (i = 0; i < TZ_NUM_PORTS; i++) {
+        if (s->port[i].downstream) {
+            max_port = i;
+        }
+    }
+
+    for (i = 0; i <= max_port; i++) {
         TZPPCPort *port = &s->port[i];
         char *name;
         uint64_t size;
 
         if (!port->downstream) {
+            /*
+             * Create dummy sysbus MMIO region so the sysbus region
+             * numbering doesn't get out of sync with the port numbers.
+             * The size is entirely arbitrary.
+             */
+            name = g_strdup_printf("tz-ppc-dummy-port[%d]", i);
+            memory_region_init_io(&port->upstream, obj, &tz_ppc_dummy_ops,
+                                  port, name, 0x10000);
+            sysbus_init_mmio(sbd, &port->upstream);
+            g_free(name);
             continue;
         }
 
diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index d3aacce80d..274ad47a33 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -12,20 +12,13 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/timer/pl031.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "qemu/log.h"
-
-//#define DEBUG_PL031
-
-#ifdef DEBUG_PL031
-#define DPRINTF(fmt, ...) \
-do { printf("pl031: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
-#endif
+#include "trace.h"
 
 #define RTC_DR      0x00    /* Data read register */
 #define RTC_MR      0x04    /* Match register */
@@ -36,30 +29,6 @@ do { printf("pl031: " fmt , ## __VA_ARGS__); } while (0)
 #define RTC_MIS     0x18    /* Masked interrupt status register */
 #define RTC_ICR     0x1c    /* Interrupt clear register */
 
-#define TYPE_PL031 "pl031"
-#define PL031(obj) OBJECT_CHECK(PL031State, (obj), TYPE_PL031)
-
-typedef struct PL031State {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    QEMUTimer *timer;
-    qemu_irq irq;
-
-    /* Needed to preserve the tick_count across migration, even if the
-     * absolute value of the rtc_clock is different on the source and
-     * destination.
-     */
-    uint32_t tick_offset_vmstate;
-    uint32_t tick_offset;
-
-    uint32_t mr;
-    uint32_t lr;
-    uint32_t cr;
-    uint32_t im;
-    uint32_t is;
-} PL031State;
-
 static const unsigned char pl031_id[] = {
     0x31, 0x10, 0x14, 0x00,         /* Device ID        */
     0x0d, 0xf0, 0x05, 0xb1          /* Cell ID      */
@@ -67,7 +36,10 @@ static const unsigned char pl031_id[] = {
 
 static void pl031_update(PL031State *s)
 {
-    qemu_set_irq(s->irq, s->is & s->im);
+    uint32_t flags = s->is & s->im;
+
+    trace_pl031_irq_state(flags);
+    qemu_set_irq(s->irq, flags);
 }
 
 static void pl031_interrupt(void * opaque)
@@ -75,7 +47,7 @@ static void pl031_interrupt(void * opaque)
     PL031State *s = (PL031State *)opaque;
 
     s->is = 1;
-    DPRINTF("Alarm raised\n");
+    trace_pl031_alarm_raised();
     pl031_update(s);
 }
 
@@ -92,7 +64,7 @@ static void pl031_set_alarm(PL031State *s)
     /* The timer wraps around.  This subtraction also wraps in the same way,
        and gives correct results when alarm < now_ticks.  */
     ticks = s->mr - pl031_get_count(s);
-    DPRINTF("Alarm set in %ud ticks\n", ticks);
+    trace_pl031_set_alarm(ticks);
     if (ticks == 0) {
         timer_del(s->timer);
         pl031_interrupt(s);
@@ -106,38 +78,49 @@ static uint64_t pl031_read(void *opaque, hwaddr offset,
                            unsigned size)
 {
     PL031State *s = (PL031State *)opaque;
-
-    if (offset >= 0xfe0  &&  offset < 0x1000)
-        return pl031_id[(offset - 0xfe0) >> 2];
+    uint64_t r;
 
     switch (offset) {
     case RTC_DR:
-        return pl031_get_count(s);
+        r = pl031_get_count(s);
+        break;
     case RTC_MR:
-        return s->mr;
+        r = s->mr;
+        break;
     case RTC_IMSC:
-        return s->im;
+        r = s->im;
+        break;
     case RTC_RIS:
-        return s->is;
+        r = s->is;
+        break;
     case RTC_LR:
-        return s->lr;
+        r = s->lr;
+        break;
     case RTC_CR:
         /* RTC is permanently enabled.  */
-        return 1;
+        r = 1;
+        break;
     case RTC_MIS:
-        return s->is & s->im;
+        r = s->is & s->im;
+        break;
+    case 0xfe0 ... 0xfff:
+        r = pl031_id[(offset - 0xfe0) >> 2];
+        break;
     case RTC_ICR:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "pl031: read of write-only register at offset 0x%x\n",
                       (int)offset);
+        r = 0;
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "pl031_read: Bad offset 0x%x\n", (int)offset);
+        r = 0;
         break;
     }
 
-    return 0;
+    trace_pl031_read(offset, r);
+    return r;
 }
 
 static void pl031_write(void * opaque, hwaddr offset,
@@ -145,6 +128,7 @@ static void pl031_write(void * opaque, hwaddr offset,
 {
     PL031State *s = (PL031State *)opaque;
 
+    trace_pl031_write(offset, value);
 
     switch (offset) {
     case RTC_LR:
@@ -157,7 +141,6 @@ static void pl031_write(void * opaque, hwaddr offset,
         break;
     case RTC_IMSC:
         s->im = value & 1;
-        DPRINTF("Interrupt mask %d\n", s->im);
         pl031_update(s);
         break;
     case RTC_ICR:
@@ -165,7 +148,6 @@ static void pl031_write(void * opaque, hwaddr offset,
            cleared when bit 0 of the written value is set.  However the
            arm926e documentation (DDI0287B) states that the interrupt is
            cleared when any value is written.  */
-        DPRINTF("Interrupt cleared");
         s->is = 0;
         pl031_update(s);
         break;
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index 0144a68951..12eb505fee 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -77,3 +77,9 @@ xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec
 nrf51_timer_read(uint64_t addr, uint32_t value, unsigned size) "read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
 nrf51_timer_write(uint64_t addr, uint32_t value, unsigned size) "write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
 
+# hw/timer/pl031.c
+pl031_irq_state(int level) "irq state %d"
+pl031_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
+pl031_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
+pl031_alarm_raised(void) "alarm raised"
+pl031_set_alarm(uint32_t ticks) "alarm set for %u ticks"