[Qemu-devel] [PATCH 00/14] Add model of the Arm Musca devboards

[Qemu-devel] [PATCH 00/14] Add model of the Arm Musca devboards

[Peter Maydell]

This patchset adds a model of the Arm Musca devboards
('musca-a' and 'musca-b1').
These boards are described here:
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

They're based on the SSE-200 subsystem for embedded which
we already have a model of. The two boards are somewhat
similar but have a different layout of devices (Musca-B1
puts them behind a more sophisticated PPC setup).

I've tested Musca-A with the Zephyr RTOS's "hello world"
program, and Musca-B1 with Arm Trusted Firmware M.
(Arm-TFM does in theory support also Musca-A, but in practice
it requires proper QSPI flash device support there; Zephyr
currently supports only Musca-A.)

Zephyr getting started guide:
https://docs.zephyrproject.org/latest/getting_started/index.html
Arm-TFM build instructions:
https://git.trustedfirmware.org/trusted-firmware-m.git/about/docs/user_guides/tfm_build_instruction.md

The first half of the patchset is some fairly straightforward
bugfixes and enhancements to various devices; the second half
does the actual implementation.

thanks
-- PMM

Peter Maydell (14):
  hw/arm/armsse: Fix miswiring of expansion IRQs
  hw/misc/tz-ppc: Support having unused ports in the middle of the range
  hw/timer/pl031: Allow use as an embedded-struct device
  hw/timer/pl031: Convert to using trace events
  hw/char/pl011: Allow use as an embedded-struct device
  hw/char/pl011: Support all interrupt lines
  hw/char/pl011: Use '0x' prefix when logging hex numbers
  hw/arm/armsse: Document SRAM_ADDR_WIDTH property in header comment
  hw/arm/armsse: Allow boards to specify init-svtor
  hw/arm/musca.c: Implement models of the Musca-A and -B1 boards
  hw/arm/musca: Add PPCs
  hw/arm/musca: Add MPCs
  hw/arm/musca: Wire up PL031 RTC
  hw/arm/musca: Wire up PL011 UARTs

 hw/arm/Makefile.objs            |   1 +
 include/hw/arm/armsse.h         |   5 +
 include/hw/char/pl011.h         |  34 ++
 include/hw/misc/tz-ppc.h        |   8 +-
 include/hw/timer/pl031.h        |  44 +++
 hw/arm/armsse.c                 |  10 +-
 hw/arm/musca.c                  | 669 ++++++++++++++++++++++++++++++++
 hw/char/pl011.c                 |  81 ++--
 hw/misc/tz-ppc.c                |  32 ++
 hw/timer/pl031.c                |  80 ++--
 MAINTAINERS                     |   7 +
 default-configs/arm-softmmu.mak |   1 +
 hw/timer/trace-events           |   6 +
 13 files changed, 890 insertions(+), 88 deletions(-)
 create mode 100644 include/hw/timer/pl031.h
 create mode 100644 hw/arm/musca.c

-- 
2.20.1

[Qemu-devel] [PATCH 01/14] hw/arm/armsse: Fix miswiring of expansion IRQs

[Peter Maydell]

In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
the ARMSSE, we set up the wiring of the expansion IRQs via nested
loops: the outer loop on 'i' loops for each CPU, and the inner loop
on 'j' loops for each interrupt. Fix a typo which meant we were
wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
to external IRQ 1 on CPU 1.

Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/armsse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 5d53071a5a0..9a8c49547db 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -565,7 +565,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
         /* Connect EXP_IRQ/EXP_CPUn_IRQ GPIOs to the NVIC's lines 32 and up */
         s->exp_irqs[i] = g_new(qemu_irq, s->exp_numirq);
         for (j = 0; j < s->exp_numirq; j++) {
-            s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, i + 32);
+            s->exp_irqs[i][j] = qdev_get_gpio_in(cpudev, j + 32);
         }
         if (i == 0) {
             gpioname = g_strdup("EXP_IRQ");
-- 
2.20.1

Re: [Qemu-devel] [PATCH 01/14] hw/arm/armsse: Fix miswiring of expansion IRQs

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
> the ARMSSE, we set up the wiring of the expansion IRQs via nested
> loops: the outer loop on 'i' loops for each CPU, and the inner loop
> on 'j' loops for each interrupt. Fix a typo which meant we were
> wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
> to external IRQ 1 on CPU 1.
> 
> Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/armsse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

[Qemu-devel] [PATCH 02/14] hw/misc/tz-ppc: Support having unused ports in the middle of the range

[Peter Maydell]

The Peripheral Protection Controller's handling of unused ports
is that if there is nothing connected to the port's downstream
then it does not create the sysbus MMIO region for the upstream
end of the port. This results in odd behaviour when there is
an unused port in the middle of the range: since sysbus MMIO
regions are implicitly consecutively allocated, any used ports
above the unused ones end up with sysbus MMIO region numbers
that don't match the port number.

Avoid this numbering mismatch by creating dummy MMIO regions
for the unused ports. This doesn't change anything for our
existing boards, which don't have any gaps in the middle of
the port ranges they use; but it will be needed for the Musca
board.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/tz-ppc.h |  8 +++++++-
 hw/misc/tz-ppc.c         | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/tz-ppc.h b/include/hw/misc/tz-ppc.h
index fc8b806e4df..080d6e2ec17 100644
--- a/include/hw/misc/tz-ppc.h
+++ b/include/hw/misc/tz-ppc.h
@@ -38,7 +38,13 @@
  *
  * QEMU interface:
  * + sysbus MMIO regions 0..15: MemoryRegions defining the upstream end
- *   of each of the 16 ports of the PPC
+ *   of each of the 16 ports of the PPC. When a port is unused (i.e. no
+ *   downstream MemoryRegion is connected to it) at the end of the 0..15
+ *   range then no sysbus MMIO region is created for its upstream. When an
+ *   unused port lies in the middle of the range with other used ports at
+ *   higher port numbers, a dummy MMIO region is created to ensure that
+ *   port N's upstream is always sysbus MMIO region N. Dummy regions should
+ *   not be mapped, and will assert if any access is made to them.
  * + Property "port[0..15]": MemoryRegion defining the downstream device(s)
  *   for each of the 16 ports of the PPC
  * + Named GPIO inputs "cfg_nonsec[0..15]": set to 1 if the port should be
diff --git a/hw/misc/tz-ppc.c b/hw/misc/tz-ppc.c
index 3dd045c15f5..2e04837bea9 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;
         }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 02/14] hw/misc/tz-ppc: Support having unused ports in the middle of the range

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> The Peripheral Protection Controller's handling of unused ports
> is that if there is nothing connected to the port's downstream
> then it does not create the sysbus MMIO region for the upstream
> end of the port. This results in odd behaviour when there is
> an unused port in the middle of the range: since sysbus MMIO
> regions are implicitly consecutively allocated, any used ports
> above the unused ones end up with sysbus MMIO region numbers
> that don't match the port number.
> 
> Avoid this numbering mismatch by creating dummy MMIO regions
> for the unused ports. This doesn't change anything for our
> existing boards, which don't have any gaps in the middle of
> the port ranges they use; but it will be needed for the Musca
> board.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/misc/tz-ppc.h |  8 +++++++-
>  hw/misc/tz-ppc.c         | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 1 deletion(-)

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

r~

[Qemu-devel] [PATCH 03/14] hw/timer/pl031: Allow use as an embedded-struct device

[Peter Maydell]

Create a new include file for the pl031's device struct,
type macros, etc, so that it can be instantiated using
the "embedded struct" coding style.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/timer/pl031.h | 44 ++++++++++++++++++++++++++++++++++++++++
 hw/timer/pl031.c         | 25 +----------------------
 MAINTAINERS              |  1 +
 3 files changed, 46 insertions(+), 24 deletions(-)
 create mode 100644 include/hw/timer/pl031.h

diff --git a/include/hw/timer/pl031.h b/include/hw/timer/pl031.h
new file mode 100644
index 00000000000..99416d8ba52
--- /dev/null
+++ b/include/hw/timer/pl031.h
@@ -0,0 +1,44 @@
+/*
+ * ARM AMBA PrimeCell PL031 RTC
+ *
+ * Copyright (c) 2007 CodeSourcery
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef HW_TIMER_PL031
+#define HW_TIMER_PL031
+
+#include "hw/sysbus.h"
+
+#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;
+
+#endif
diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index d3aacce80da..f774dcd5223 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/timer/pl031.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
@@ -36,30 +37,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      */
diff --git a/MAINTAINERS b/MAINTAINERS
index 9a76845581b..85d4b4c9f7c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -493,6 +493,7 @@ F: hw/sd/pl181.c
 F: hw/ssi/pl022.c
 F: include/hw/ssi/pl022.h
 F: hw/timer/pl031.c
+F: include/hw/timer/pl031.h
 F: include/hw/arm/primecell.h
 F: hw/timer/cmsdk-apb-timer.c
 F: include/hw/timer/cmsdk-apb-timer.h
-- 
2.20.1

Re: [Qemu-devel] [PATCH 03/14] hw/timer/pl031: Allow use as an embedded-struct device

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> Create a new include file for the pl031's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/timer/pl031.h | 44 ++++++++++++++++++++++++++++++++++++++++
>  hw/timer/pl031.c         | 25 +----------------------
>  MAINTAINERS              |  1 +
>  3 files changed, 46 insertions(+), 24 deletions(-)
>  create mode 100644 include/hw/timer/pl031.h

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

r~

Re: [Qemu-devel] [PATCH 03/14] hw/timer/pl031: Allow use as an embedded-struct device

[Philippe Mathieu-Daudé]

On 2/14/19 1:50 PM, Peter Maydell wrote:
> Create a new include file for the pl031's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  include/hw/timer/pl031.h | 44 ++++++++++++++++++++++++++++++++++++++++
>  hw/timer/pl031.c         | 25 +----------------------
>  MAINTAINERS              |  1 +
>  3 files changed, 46 insertions(+), 24 deletions(-)
>  create mode 100644 include/hw/timer/pl031.h
> 
> diff --git a/include/hw/timer/pl031.h b/include/hw/timer/pl031.h
> new file mode 100644
> index 00000000000..99416d8ba52
> --- /dev/null
> +++ b/include/hw/timer/pl031.h
> @@ -0,0 +1,44 @@
> +/*
> + * ARM AMBA PrimeCell PL031 RTC
> + *
> + * Copyright (c) 2007 CodeSourcery
> + *
> + * This file is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#ifndef HW_TIMER_PL031
> +#define HW_TIMER_PL031
> +
> +#include "hw/sysbus.h"
> +
> +#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;
> +
> +#endif
> diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
> index d3aacce80da..f774dcd5223 100644
> --- a/hw/timer/pl031.c
> +++ b/hw/timer/pl031.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "hw/timer/pl031.h"
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
> @@ -36,30 +37,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      */
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9a76845581b..85d4b4c9f7c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -493,6 +493,7 @@ F: hw/sd/pl181.c
>  F: hw/ssi/pl022.c
>  F: include/hw/ssi/pl022.h
>  F: hw/timer/pl031.c
> +F: include/hw/timer/pl031.h
>  F: include/hw/arm/primecell.h
>  F: hw/timer/cmsdk-apb-timer.c
>  F: include/hw/timer/cmsdk-apb-timer.h
> 

[Qemu-devel] [PATCH 04/14] hw/timer/pl031: Convert to using trace events

[Peter Maydell]

Convert the debug printing in the PL031 device to use trace events,
and augment it to cover the interesting parts of device operation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/timer/pl031.c      | 55 +++++++++++++++++++++++--------------------
 hw/timer/trace-events |  6 +++++
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index f774dcd5223..274ad47a33a 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -18,15 +18,7 @@
 #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 */
@@ -44,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)
@@ -52,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);
 }
 
@@ -69,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);
@@ -83,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,
@@ -122,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:
@@ -134,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:
@@ -142,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 0144a68951c..12eb505fee7 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"
-- 
2.20.1

Re: [Qemu-devel] [PATCH 04/14] hw/timer/pl031: Convert to using trace events

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> Convert the debug printing in the PL031 device to use trace events,
> and augment it to cover the interesting parts of device operation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/timer/pl031.c      | 55 +++++++++++++++++++++++--------------------
>  hw/timer/trace-events |  6 +++++
>  2 files changed, 36 insertions(+), 25 deletions(-)

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

r~

Re: [Qemu-devel] [PATCH 04/14] hw/timer/pl031: Convert to using trace events

[Philippe Mathieu-Daudé]

On 2/14/19 1:50 PM, Peter Maydell wrote:
> Convert the debug printing in the PL031 device to use trace events,
> and augment it to cover the interesting parts of device operation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/timer/pl031.c      | 55 +++++++++++++++++++++++--------------------
>  hw/timer/trace-events |  6 +++++
>  2 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
> index f774dcd5223..274ad47a33a 100644
> --- a/hw/timer/pl031.c
> +++ b/hw/timer/pl031.c
> @@ -18,15 +18,7 @@
>  #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 */
> @@ -44,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)
> @@ -52,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);
>  }
>  
> @@ -69,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);
> @@ -83,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];

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +        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,
> @@ -122,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:
> @@ -134,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:
> @@ -142,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 0144a68951c..12eb505fee7 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"
> 

[Qemu-devel] [PATCH 05/14] hw/char/pl011: Allow use as an embedded-struct device

[Peter Maydell]

Create a new include file for the pl011's device struct,
type macros, etc, so that it can be instantiated using
the "embedded struct" coding style.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/char/pl011.h | 34 ++++++++++++++++++++++++++++++++++
 hw/char/pl011.c         | 31 ++-----------------------------
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index 83649324b6a..1b52bfd5c90 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -15,6 +15,40 @@
 #ifndef HW_PL011_H
 #define HW_PL011_H
 
+#include "hw/sysbus.h"
+#include "chardev/char-fe.h"
+
+#define TYPE_PL011 "pl011"
+#define PL011(obj) OBJECT_CHECK(PL011State, (obj), TYPE_PL011)
+
+/* This shares the same struct (and cast macro) as the base pl011 device */
+#define TYPE_PL011_LUMINARY "pl011_luminary"
+
+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;
+
 static inline DeviceState *pl011_create(hwaddr addr,
                                         qemu_irq irq,
                                         Chardev *chr)
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 2aa277fc4f2..0c4711e4027 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -8,39 +8,12 @@
  */
 
 #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
 
@@ -357,7 +330,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,
 };
-- 
2.20.1

Re: [Qemu-devel] [PATCH 05/14] hw/char/pl011: Allow use as an embedded-struct device

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> Create a new include file for the pl011's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/char/pl011.h | 34 ++++++++++++++++++++++++++++++++++
>  hw/char/pl011.c         | 31 ++-----------------------------
>  2 files changed, 36 insertions(+), 29 deletions(-)

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

r~

Re: [Qemu-devel] [PATCH 05/14] hw/char/pl011: Allow use as an embedded-struct device

[Philippe Mathieu-Daudé]

On 2/14/19 1:50 PM, Peter Maydell wrote:
> Create a new include file for the pl011's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  include/hw/char/pl011.h | 34 ++++++++++++++++++++++++++++++++++
>  hw/char/pl011.c         | 31 ++-----------------------------
>  2 files changed, 36 insertions(+), 29 deletions(-)
> 
> diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
> index 83649324b6a..1b52bfd5c90 100644
> --- a/include/hw/char/pl011.h
> +++ b/include/hw/char/pl011.h
> @@ -15,6 +15,40 @@
>  #ifndef HW_PL011_H
>  #define HW_PL011_H
>  
> +#include "hw/sysbus.h"
> +#include "chardev/char-fe.h"
> +
> +#define TYPE_PL011 "pl011"
> +#define PL011(obj) OBJECT_CHECK(PL011State, (obj), TYPE_PL011)
> +
> +/* This shares the same struct (and cast macro) as the base pl011 device */
> +#define TYPE_PL011_LUMINARY "pl011_luminary"
> +
> +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;
> +
>  static inline DeviceState *pl011_create(hwaddr addr,
>                                          qemu_irq irq,
>                                          Chardev *chr)
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index 2aa277fc4f2..0c4711e4027 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -8,39 +8,12 @@
>   */
>  
>  #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
>  
> @@ -357,7 +330,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,
>  };
> 

[Qemu-devel] [PATCH 06/14] hw/char/pl011: Support all interrupt lines

[Peter Maydell]

The PL011 UART has six interrupt lines:
 * RX (receive data)
 * TX (transmit data)
 * RT (receive timeout)
 * MS (modem status)
 * E (errors)
 * combined (logical OR of all the above)

So far we have only emulated the combined interrupt line;
add support for the others, so that boards that wire them
up to different interrupt controller inputs can do so.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/char/pl011.h |  2 +-
 hw/char/pl011.c         | 46 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index 1b52bfd5c90..dad3cf29121 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -45,7 +45,7 @@ typedef struct PL011State {
     int read_count;
     int read_trigger;
     CharBackend chr;
-    qemu_irq irq;
+    qemu_irq irq[6];
     const unsigned char *id;
 } PL011State;
 
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 0c4711e4027..29f4e5eb224 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -7,6 +7,17 @@
  * 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"
@@ -22,18 +33,46 @@
 #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,
@@ -284,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;
-- 
2.20.1

Re: [Qemu-devel] [PATCH 06/14] hw/char/pl011: Support all interrupt lines

[Richard Henderson]

On 2/14/19 4:50 AM, Peter Maydell wrote:
> The PL011 UART has six interrupt lines:
>  * RX (receive data)
>  * TX (transmit data)
>  * RT (receive timeout)
>  * MS (modem status)
>  * E (errors)
>  * combined (logical OR of all the above)
> 
> So far we have only emulated the combined interrupt line;
> add support for the others, so that boards that wire them
> up to different interrupt controller inputs can do so.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/char/pl011.h |  2 +-
>  hw/char/pl011.c         | 46 +++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 45 insertions(+), 3 deletions(-)

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

r~

[Qemu-devel] [PATCH 07/14] hw/char/pl011: Use '0x' prefix when logging hex numbers

[Peter Maydell]

The pl011 logs when the guest makes a bad access. It prints
the address offset in hex but confusingly omits the '0x'
prefix; add it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/char/pl011.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 29f4e5eb224..e5dd448f854 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -143,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;
     }
@@ -232,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);
     }
 }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 07/14] hw/char/pl011: Use '0x' prefix when logging hex numbers

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> The pl011 logs when the guest makes a bad access. It prints
> the address offset in hex but confusingly omits the '0x'
> prefix; add it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/char/pl011.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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

r~

Re: [Qemu-devel] [PATCH 07/14] hw/char/pl011: Use '0x' prefix when logging hex numbers

[Philippe Mathieu-Daudé]

On 2/14/19 1:51 PM, Peter Maydell wrote:
> The pl011 logs when the guest makes a bad access. It prints
> the address offset in hex but confusingly omits the '0x'
> prefix; add it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/char/pl011.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index 29f4e5eb224..e5dd448f854 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -143,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;
>      }
> @@ -232,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);
>      }
>  }
>  
> 

[Qemu-devel] [PATCH 08/14] hw/arm/armsse: Document SRAM_ADDR_WIDTH property in header comment

[Peter Maydell]

In commit 4b635cf7a95e501211 we added a QOM property to the ARMSSE
object, but forgot to add it to the documentation comment in the
header. Correct the omission.

Fixes: 4b635cf7a95e501211 ("hw/arm/armsse: Make SRAM bank size configurable")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armsse.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
index f800bafb14a..444605b44dc 100644
--- a/include/hw/arm/armsse.h
+++ b/include/hw/arm/armsse.h
@@ -46,6 +46,8 @@
  *    being the same for both, to avoid having to have separate Property
  *    lists for different variants. This restriction can be relaxed later
  *    if necessary.)
+ *  + QOM property "SRAM_ADDR_WIDTH" sets the number of bits used for the
+ *    address of each SRAM bank (and thus the total amount of internal SRAM)
  *  + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0,
  *    which are wired to its NVIC lines 32 .. n+32
  *  + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for
-- 
2.20.1

Re: [Qemu-devel] [PATCH 08/14] hw/arm/armsse: Document SRAM_ADDR_WIDTH property in header comment

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> In commit 4b635cf7a95e501211 we added a QOM property to the ARMSSE
> object, but forgot to add it to the documentation comment in the
> header. Correct the omission.
> 
> Fixes: 4b635cf7a95e501211 ("hw/arm/armsse: Make SRAM bank size configurable")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armsse.h | 2 ++
>  1 file changed, 2 insertions(+)

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

r~

[Qemu-devel] [PATCH 09/14] hw/arm/armsse: Allow boards to specify init-svtor

[Peter Maydell]

The Musca boards have DAPLink firmware that sets the initial
secure VTOR value (the location of the vector table) differently
depending on the boot mode (from flash, from RAM, etc). Export
the init-svtor as a QOM property of the ARMSSE object so that
the board can change it.

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

diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
index 444605b44dc..84879f40dd8 100644
--- a/include/hw/arm/armsse.h
+++ b/include/hw/arm/armsse.h
@@ -48,6 +48,8 @@
  *    if necessary.)
  *  + QOM property "SRAM_ADDR_WIDTH" sets the number of bits used for the
  *    address of each SRAM bank (and thus the total amount of internal SRAM)
+ *  + QOM property "init-svtor" sets the initial value of the CPU SVTOR register
+ *    (where it expects to load the PC and SP from the vector table on reset)
  *  + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0,
  *    which are wired to its NVIC lines 32 .. n+32
  *  + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for
@@ -204,6 +206,7 @@ typedef struct ARMSSE {
     uint32_t exp_numirq;
     uint32_t mainclk_frq;
     uint32_t sram_addr_width;
+    uint32_t init_svtor;
 } ARMSSE;
 
 typedef struct ARMSSEInfo ARMSSEInfo;
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 9a8c49547db..3040ea9324e 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -505,11 +505,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
@@ -1185,6 +1184,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()
 };
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 09/14] hw/arm/armsse: Allow boards to specify init-svtor

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> The Musca boards have DAPLink firmware that sets the initial
> secure VTOR value (the location of the vector table) differently
> depending on the boot mode (from flash, from RAM, etc). Export
> the init-svtor as a QOM property of the ARMSSE object so that
> the board can change it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armsse.h | 3 +++
>  hw/arm/armsse.c         | 8 ++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)

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

r~

Re: [Qemu-devel] [PATCH 09/14] hw/arm/armsse: Allow boards to specify init-svtor

[Philippe Mathieu-Daudé]

On 2/14/19 1:51 PM, Peter Maydell wrote:
> The Musca boards have DAPLink firmware that sets the initial
> secure VTOR value (the location of the vector table) differently
> depending on the boot mode (from flash, from RAM, etc). Export
> the init-svtor as a QOM property of the ARMSSE object so that
> the board can change it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  include/hw/arm/armsse.h | 3 +++
>  hw/arm/armsse.c         | 8 ++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
> index 444605b44dc..84879f40dd8 100644
> --- a/include/hw/arm/armsse.h
> +++ b/include/hw/arm/armsse.h
> @@ -48,6 +48,8 @@
>   *    if necessary.)
>   *  + QOM property "SRAM_ADDR_WIDTH" sets the number of bits used for the
>   *    address of each SRAM bank (and thus the total amount of internal SRAM)
> + *  + QOM property "init-svtor" sets the initial value of the CPU SVTOR register
> + *    (where it expects to load the PC and SP from the vector table on reset)
>   *  + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0,
>   *    which are wired to its NVIC lines 32 .. n+32
>   *  + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for
> @@ -204,6 +206,7 @@ typedef struct ARMSSE {
>      uint32_t exp_numirq;
>      uint32_t mainclk_frq;
>      uint32_t sram_addr_width;
> +    uint32_t init_svtor;
>  } ARMSSE;
>  
>  typedef struct ARMSSEInfo ARMSSEInfo;
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 9a8c49547db..3040ea9324e 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -505,11 +505,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
> @@ -1185,6 +1184,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()
>  };
>  
> 

[Qemu-devel] [PATCH 10/14] hw/arm/musca.c: Implement models of the Musca-A and -B1 boards

[Peter Maydell]

The Musca-A and Musca-B1 development boards are based on the
SSE-200 subsystem for embedded. Implement an initial skeleton
model of these boards, which are similar but not identical.

This commit creates the board model with the SSE and the IRQ
splitters to wire IRQs up to its two CPUs. As yet there
are no devices and no memory: these will be added later.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/Makefile.objs            |   1 +
 hw/arm/musca.c                  | 197 ++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   6 +
 default-configs/arm-softmmu.mak |   1 +
 4 files changed, 205 insertions(+)
 create mode 100644 hw/arm/musca.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fa40e8d6412..fa57c7c7704 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/musca.c b/hw/arm/musca.c
new file mode 100644
index 00000000000..cc624c7d160
--- /dev/null
+++ b/hw/arm/musca.c
@@ -0,0 +1,197 @@
+/*
+ * 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 "hw/arm/arm.h"
+#include "hw/arm/armsse.h"
+#include "hw/boards.h"
+#include "hw/core/split-irq.h"
+
+#define MUSCA_NUMIRQ_MAX 96
+
+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;
+} MuscaMachineClass;
+
+typedef struct {
+    MachineState parent;
+
+    ARMSSE sse;
+    SplitIRQ cpu_irq_splitter[MUSCA_NUMIRQ_MAX];
+} 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 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;
+    int i;
+
+    assert(mmc->num_irqs <= MUSCA_NUMIRQ_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));
+    }
+
+    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;
+}
+
+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;
+}
+
+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/MAINTAINERS b/MAINTAINERS
index 85d4b4c9f7c..9b5042b883a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -636,6 +636,12 @@ F: include/hw/misc/iotkit-sysinfo.h
 F: hw/misc/armsse-cpuid.c
 F: include/hw/misc/armsse-cpuid.h
 
+Musca
+M: Peter Maydell <peter.maydell@linaro.org>
+L: qemu-arm@nongnu.org
+S: Maintained
+F: hw/arm/musca.c
+
 Musicpal
 M: Jan Kiszka <jan.kiszka@web.de>
 M: Peter Maydell <peter.maydell@linaro.org>
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 734ca721e9e..87ad2674946 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -89,6 +89,7 @@ CONFIG_TUSB6010=y
 CONFIG_IMX=y
 CONFIG_MAINSTONE=y
 CONFIG_MPS2=y
+CONFIG_MUSCA=y
 CONFIG_NSERIES=y
 CONFIG_RASPI=y
 CONFIG_REALVIEW=y
-- 
2.20.1

Re: [Qemu-devel] [PATCH 10/14] hw/arm/musca.c: Implement models of the Musca-A and -B1 boards

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> The Musca-A and Musca-B1 development boards are based on the
> SSE-200 subsystem for embedded. Implement an initial skeleton
> model of these boards, which are similar but not identical.
> 
> This commit creates the board model with the SSE and the IRQ
> splitters to wire IRQs up to its two CPUs. As yet there
> are no devices and no memory: these will be added later.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/Makefile.objs            |   1 +
>  hw/arm/musca.c                  | 197 ++++++++++++++++++++++++++++++++
>  MAINTAINERS                     |   6 +
>  default-configs/arm-softmmu.mak |   1 +
>  4 files changed, 205 insertions(+)
>  create mode 100644 hw/arm/musca.c

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

r~

[Qemu-devel] [PATCH 11/14] hw/arm/musca: Add PPCs

[Peter Maydell]

Many of the devices on the Musca board live behind TrustZone
Peripheral Protection Controllers (PPCs); add models of the
PPCs, using a similar scheme to the MPS2 board models.
This commit wires up the PPCs with "unimplemented device"
stubs behind them in the correct places in the address map.

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

diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index cc624c7d160..8774e0b87b7 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -27,8 +27,11 @@
 #include "hw/arm/armsse.h"
 #include "hw/boards.h"
 #include "hw/core/split-irq.h"
+#include "hw/misc/tz-ppc.h"
+#include "hw/misc/unimp.h"
 
 #define MUSCA_NUMIRQ_MAX 96
+#define MUSCA_PPC_MAX 3
 
 typedef enum MuscaType {
     MUSCA_A,
@@ -48,6 +51,24 @@ typedef struct {
 
     ARMSSE sse;
     SplitIRQ cpu_irq_splitter[MUSCA_NUMIRQ_MAX];
+    SplitIRQ sec_resp_splitter;
+    TZPPC ppc[MUSCA_PPC_MAX];
+    MemoryRegion container;
+    UnimplementedDeviceState eflash[2];
+    UnimplementedDeviceState qspi;
+    UnimplementedDeviceState mpc[5];
+    UnimplementedDeviceState mhu[2];
+    UnimplementedDeviceState pwm[3];
+    UnimplementedDeviceState i2s;
+    UnimplementedDeviceState uart[2];
+    UnimplementedDeviceState i2c[2];
+    UnimplementedDeviceState spi;
+    UnimplementedDeviceState scc;
+    UnimplementedDeviceState timer;
+    UnimplementedDeviceState rtc;
+    UnimplementedDeviceState pvt;
+    UnimplementedDeviceState sdio;
+    UnimplementedDeviceState gpio;
 } MuscaMachineState;
 
 #define TYPE_MUSCA_MACHINE "musca"
@@ -68,6 +89,94 @@ typedef struct {
  */
 #define SYSCLK_FRQ 40000000
 
+/*
+ * 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);
+}
+
+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_unimp_dev, &mms->uart[0], 0x1000, 0x1000 },
+        { "uart1", make_unimp_dev, &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_unimp_dev, &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_unimp_dev, &mms->mpc[0], 0x12000, 0x1000 },
+        { "mpc1", make_unimp_dev, &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);
@@ -75,6 +184,9 @@ static void musca_init(MachineState *machine)
     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);
@@ -121,6 +233,183 @@ static void musca_init(MachineState *machine)
                                                      "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_unimp_dev, &mms->mpc[0], 0x52000000, 0x1000 },
+                { "mpc1", make_unimp_dev, &mms->mpc[1], 0x52100000, 0x1000 },
+                { "mpc2", make_unimp_dev, &mms->mpc[2], 0x52200000, 0x1000 },
+                { "mpc3", make_unimp_dev, &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_unimp_dev, &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_unimp_dev, &mms->uart[0], 0x40105000, 0x1000 },
+                { "uart1", make_unimp_dev, &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_unimp_dev, &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);
 }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 11/14] hw/arm/musca: Add PPCs

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> +    const PPCPortInfo devices[] = {
> +    const PPCInfo a_ppcs[] = { {
> +    const PPCInfo b1_ppcs[] = { {

static const for all of these.  Otherwise,

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

r~

Re: [Qemu-devel] [PATCH 11/14] hw/arm/musca: Add PPCs

[Richard Henderson]

On 2/17/19 10:14 AM, Richard Henderson wrote:
> On 2/14/19 4:51 AM, Peter Maydell wrote:
>> +    const PPCPortInfo devices[] = {
>> +    const PPCInfo a_ppcs[] = { {
>> +    const PPCInfo b1_ppcs[] = { {
> 
> static const for all of these.  Otherwise,

... but of course in subsequent patches you put non-const things in there.
Nevermind.


r~

[Qemu-devel] [PATCH 12/14] hw/arm/musca: Add MPCs

[Peter Maydell]

The Musca board puts its SRAM and flash behind TrustZone
Memory Protection Controllers (MPCs). Each MPC sits between
the CPU and the RAM/flash, and also has a set of memory mapped
control registers. Wire up the MPCs, and the memory behind them.
For the moment we implement the flash as simple ROM, which
cannot be reprogrammed by the guest.

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

diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index 8774e0b87b7..5fadac8c09b 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -27,11 +27,15 @@
 #include "hw/arm/armsse.h"
 #include "hw/boards.h"
 #include "hw/core/split-irq.h"
+#include "hw/misc/tz-mpc.h"
 #include "hw/misc/tz-ppc.h"
 #include "hw/misc/unimp.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,
@@ -44,19 +48,23 @@ typedef struct {
     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;
-    UnimplementedDeviceState mpc[5];
+    TZMPC mpc[MUSCA_MPC_MAX];
     UnimplementedDeviceState mhu[2];
     UnimplementedDeviceState pwm[3];
     UnimplementedDeviceState i2s;
@@ -69,6 +77,7 @@ typedef struct {
     UnimplementedDeviceState pvt;
     UnimplementedDeviceState sdio;
     UnimplementedDeviceState gpio;
+    UnimplementedDeviceState cryptoisland;
 } MuscaMachineState;
 
 #define TYPE_MUSCA_MACHINE "musca"
@@ -131,6 +140,131 @@ static MemoryRegion *make_unimp_dev(MuscaMachineState *mms,
     return sysbus_mmio_get_region(SYS_BUS_DEVICE(uds), 0);
 }
 
+typedef enum MPCInfoType {
+    MPC_RAM,
+    MPC_ROM,
+    MPC_CRYPTOISLAND,
+} MPCInfoType;
+
+typedef struct MPCInfo {
+    const char *name;
+    hwaddr addr;
+    hwaddr size;
+    MPCInfoType type;
+} MPCInfo;
+
+/* 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_musca_a_devs(MuscaMachineState *mms, void *opaque,
                                        const char *name, hwaddr size)
 {
@@ -160,8 +294,8 @@ static MemoryRegion *make_musca_a_devs(MuscaMachineState *mms, void *opaque,
         { "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_unimp_dev, &mms->mpc[0], 0x12000, 0x1000 },
-        { "mpc1", make_unimp_dev, &mms->mpc[1], 0x13000, 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);
@@ -190,6 +324,7 @@ static void musca_init(MachineState *machine)
     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",
@@ -285,10 +420,10 @@ static void musca_init(MachineState *machine)
                 { "eflash1", make_unimp_dev, &mms->eflash[1],
                   0x52500000, 0x1000 },
                 { "qspi", make_unimp_dev, &mms->qspi, 0x42800000, 0x100000 },
-                { "mpc0", make_unimp_dev, &mms->mpc[0], 0x52000000, 0x1000 },
-                { "mpc1", make_unimp_dev, &mms->mpc[1], 0x52100000, 0x1000 },
-                { "mpc2", make_unimp_dev, &mms->mpc[2], 0x52200000, 0x1000 },
-                { "mpc3", make_unimp_dev, &mms->mpc[3], 0x52300000, 0x1000 },
+                { "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 */
@@ -296,7 +431,7 @@ static void musca_init(MachineState *machine)
                 { }, /* port 11: unused */
                 { }, /* port 12: unused */
                 { }, /* port 13: unused */
-                { "mpc4", make_unimp_dev, &mms->mpc[4], 0x52e00000, 0x1000 },
+                { "mpc4", make_mpc, &mms->mpc[4], 0x52e00000, 0x1000 },
             },
         }, {
             .name = "apb_ppcexp1",
@@ -434,6 +569,8 @@ static void musca_a_class_init(ObjectClass *oc, void *data)
     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)
@@ -453,6 +590,8 @@ static void musca_b1_class_init(ObjectClass *oc, void *data)
     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 = {
-- 
2.20.1

Re: [Qemu-devel] [PATCH 12/14] hw/arm/musca: Add MPCs

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> The Musca board puts its SRAM and flash behind TrustZone
> Memory Protection Controllers (MPCs). Each MPC sits between
> the CPU and the RAM/flash, and also has a set of memory mapped
> control registers. Wire up the MPCs, and the memory behind them.
> For the moment we implement the flash as simple ROM, which
> cannot be reprogrammed by the guest.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/musca.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 147 insertions(+), 8 deletions(-)

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

r~

Re: [Qemu-devel] [PATCH 12/14] hw/arm/musca: Add MPCs

[Peter Maydell]

On Thu, 14 Feb 2019 at 12:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The Musca board puts its SRAM and flash behind TrustZone
> Memory Protection Controllers (MPCs). Each MPC sits between
> the CPU and the RAM/flash, and also has a set of memory mapped
> control registers. Wire up the MPCs, and the memory behind them.
> For the moment we implement the flash as simple ROM, which
> cannot be reprogrammed by the guest.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/musca.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 147 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/musca.c b/hw/arm/musca.c
> index 8774e0b87b7..5fadac8c09b 100644
> --- a/hw/arm/musca.c
> +++ b/hw/arm/musca.c
> @@ -27,11 +27,15 @@
>  #include "hw/arm/armsse.h"
>  #include "hw/boards.h"
>  #include "hw/core/split-irq.h"
> +#include "hw/misc/tz-mpc.h"
>  #include "hw/misc/tz-ppc.h"
>  #include "hw/misc/unimp.h"
>
>  #define MUSCA_NUMIRQ_MAX 96
>  #define MUSCA_PPC_MAX 3
> +#define MUSCA_MPC_MAX 5
> +
> +typedef struct MPCInfo MPCInfo;

[...]

> +typedef struct MPCInfo {
> +    const char *name;
> +    hwaddr addr;
> +    hwaddr size;
> +    MPCInfoType type;
> +} MPCInfo;

This should just be "struct MPCInfo { ... };" to avoid clang
complaining:

hw/arm/musca.c:165:3: error: redefinition of typedef 'MPCInfo' is a C11
      feature [-Werror,-Wtypedef-redefinition]

Since it's a minor thing I'll just squash it in when I
apply this series, assuming I don't need to respin for
anything else.

thanks
-- PMM

[Qemu-devel] [PATCH 13/14] hw/arm/musca: Wire up PL031 RTC

[Peter Maydell]

Wire up the PL031 RTC for the Musca board.

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

diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index 5fadac8c09b..ec8dfee1964 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -30,6 +30,7 @@
 #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
@@ -73,7 +74,7 @@ typedef struct {
     UnimplementedDeviceState spi;
     UnimplementedDeviceState scc;
     UnimplementedDeviceState timer;
-    UnimplementedDeviceState rtc;
+    PL031State rtc;
     UnimplementedDeviceState pvt;
     UnimplementedDeviceState sdio;
     UnimplementedDeviceState gpio;
@@ -98,6 +99,14 @@ typedef struct {
  */
 #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
@@ -265,6 +274,17 @@ static MemoryRegion *make_mpc(MuscaMachineState *mms, void *opaque,
     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_musca_a_devs(MuscaMachineState *mms, void *opaque,
                                        const char *name, hwaddr size)
 {
@@ -287,7 +307,7 @@ static MemoryRegion *make_musca_a_devs(MuscaMachineState *mms, void *opaque,
         { "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_unimp_dev, &mms->rtc, 0x8000, 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 },
@@ -447,7 +467,7 @@ static void musca_init(MachineState *machine)
                 { "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_unimp_dev, &mms->rtc, 0x4010d000, 0x1000 },
+                { "rtc", make_rtc, &mms->rtc, 0x4010d000, 0x1000 },
                 { "pvt", make_unimp_dev, &mms->pvt, 0x4010e000, 0x1000 },
                 { "sdio", make_unimp_dev, &mms->sdio, 0x4010f000, 0x1000 },
             },
-- 
2.20.1

Re: [Qemu-devel] [PATCH 13/14] hw/arm/musca: Wire up PL031 RTC

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> Wire up the PL031 RTC for the Musca board.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/musca.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)

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

r~

[Qemu-devel] [PATCH 14/14] hw/arm/musca: Wire up PL011 UARTs

[Peter Maydell]

Wire up the two PL011 UARTs in the Musca board.

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

diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index ec8dfee1964..e9701533d20 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -23,9 +23,11 @@
 #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"
@@ -69,7 +71,7 @@ typedef struct {
     UnimplementedDeviceState mhu[2];
     UnimplementedDeviceState pwm[3];
     UnimplementedDeviceState i2s;
-    UnimplementedDeviceState uart[2];
+    PL011State uart[2];
     UnimplementedDeviceState i2c[2];
     UnimplementedDeviceState spi;
     UnimplementedDeviceState scc;
@@ -285,6 +287,28 @@ static MemoryRegion *make_rtc(MuscaMachineState *mms, void *opaque,
     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)
 {
@@ -300,8 +324,8 @@ static MemoryRegion *make_musca_a_devs(MuscaMachineState *mms, void *opaque,
     MemoryRegion *container = &mms->container;
 
     const PPCPortInfo devices[] = {
-        { "uart0", make_unimp_dev, &mms->uart[0], 0x1000, 0x1000 },
-        { "uart1", make_unimp_dev, &mms->uart[1], 0x2000, 0x1000 },
+        { "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 },
@@ -460,8 +484,8 @@ static void musca_init(MachineState *machine)
                 { "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_unimp_dev, &mms->uart[0], 0x40105000, 0x1000 },
-                { "uart1", make_unimp_dev, &mms->uart[1], 0x40106000, 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 },
-- 
2.20.1

Re: [Qemu-devel] [PATCH 14/14] hw/arm/musca: Wire up PL011 UARTs

[Richard Henderson]

On 2/14/19 4:51 AM, Peter Maydell wrote:
> Wire up the two PL011 UARTs in the Musca board.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/musca.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)

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

r~