[Qemu-devel] [PATCH v3 0/8] arm: implement TZ MPC

[Qemu-devel] [PATCH v3 0/8] arm: implement TZ MPC

[Peter Maydell]

Hi; this is v3 of my iommu patchset. All the IOMMU stuff is now
in master, so the remaining part is just implementing and using
the Trustzone Memory Protection Controller in the mps2-an505.

Changes from v2 to v3 (all fairly minor):
 * add new variable to clarify sense of LUT bits
 * only autoinc the IDX register if CTRL.AUTOINC is set
 * NS accesses should see IDregs only
   (The datasheet is unclear on the exact behaviour on an
   NS access to a non-ID register, so I've made a best guess
   and had them RAZ/WI. This behaviour is not reachable for
   the mps2-an505 anyway, so it doesn't really matter.)

Patches still needing review: 2, 4, 5

thanks
-- PMM

Peter Maydell (8):
  hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection
    Controller
  hw/misc/tz-mpc.c: Implement registers
  hw/misc/tz-mpc.c: Implement correct blocked-access behaviour
  hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate
  hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS
  hw/arm/iotkit: Instantiate MPC
  hw/arm/iotkit: Wire up MPC interrupt lines
  hw/arm/mps2-tz.c: Instantiate MPCs

 hw/misc/Makefile.objs           |   1 +
 include/hw/arm/iotkit.h         |   8 +
 include/hw/misc/iotkit-secctl.h |   8 +
 include/hw/misc/tz-mpc.h        |  80 ++++
 hw/arm/iotkit.c                 | 112 +++++-
 hw/arm/mps2-tz.c                |  71 ++--
 hw/misc/iotkit-secctl.c         |  38 +-
 hw/misc/tz-mpc.c                | 628 ++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   2 +
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/trace-events            |   8 +
 11 files changed, 917 insertions(+), 40 deletions(-)
 create mode 100644 include/hw/misc/tz-mpc.h
 create mode 100644 hw/misc/tz-mpc.c

-- 
2.17.1

[Qemu-devel] [PATCH v3 1/8] hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller

[Peter Maydell]

Implement the Arm TrustZone Memory Protection Controller, which sits
in front of RAM and allows secure software to configure it to either
pass through or reject transactions.

We implement the MPC as a QEMU IOMMU, which will direct transactions
either through to the devices and memory behind it or to a special
"never works" AddressSpace if they are blocked.

This initial commit implements the skeleton of the device:
 * it always permits accesses
 * it doesn't implement most of the registers
 * it doesn't implement the interrupt or other behaviour
   for blocked transactions

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/misc/Makefile.objs           |   1 +
 include/hw/misc/tz-mpc.h        |  70 ++++++
 hw/misc/tz-mpc.c                | 399 ++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   2 +
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/trace-events            |   7 +
 6 files changed, 480 insertions(+)
 create mode 100644 include/hw/misc/tz-mpc.h
 create mode 100644 hw/misc/tz-mpc.c

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index ecd8d610984..93509008451 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -62,6 +62,7 @@ obj-$(CONFIG_MIPS_ITU) += mips_itu.o
 obj-$(CONFIG_MPS2_FPGAIO) += mps2-fpgaio.o
 obj-$(CONFIG_MPS2_SCC) += mps2-scc.o
 
+obj-$(CONFIG_TZ_MPC) += tz-mpc.o
 obj-$(CONFIG_TZ_PPC) += tz-ppc.o
 obj-$(CONFIG_IOTKIT_SECCTL) += iotkit-secctl.o
 
diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
new file mode 100644
index 00000000000..d1a65fd9a38
--- /dev/null
+++ b/include/hw/misc/tz-mpc.h
@@ -0,0 +1,70 @@
+/*
+ * ARM AHB5 TrustZone Memory Protection Controller emulation
+ *
+ * Copyright (c) 2018 Linaro Limited
+ * Written by Peter Maydell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+/* This is a model of the TrustZone memory protection controller (MPC).
+ * It is documented in the ARM CoreLink SIE-200 System IP for Embedded TRM
+ * (DDI 0571G):
+ * https://developer.arm.com/products/architecture/m-profile/docs/ddi0571/g
+ *
+ * The MPC sits in front of memory and allows secure software to
+ * configure it to either pass through or reject transactions.
+ * Rejected transactions may be configured to either be aborted, or to
+ * behave as RAZ/WI. An interrupt can be signalled for a rejected transaction.
+ *
+ * The MPC has a register interface which the guest uses to configure it.
+ *
+ * QEMU interface:
+ * + sysbus MMIO region 0: MemoryRegion for the MPC's config registers
+ * + sysbus MMIO region 1: MemoryRegion for the upstream end of the MPC
+ * + Property "downstream": MemoryRegion defining the downstream memory
+ * + Named GPIO output "irq": set for a transaction-failed interrupt
+ */
+
+#ifndef TZ_MPC_H
+#define TZ_MPC_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_TZ_MPC "tz-mpc"
+#define TZ_MPC(obj) OBJECT_CHECK(TZMPC, (obj), TYPE_TZ_MPC)
+
+#define TZ_NUM_PORTS 16
+
+#define TYPE_TZ_MPC_IOMMU_MEMORY_REGION "tz-mpc-iommu-memory-region"
+
+typedef struct TZMPC TZMPC;
+
+struct TZMPC {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+
+    qemu_irq irq;
+
+    /* Properties */
+    MemoryRegion *downstream;
+
+    hwaddr blocksize;
+    uint32_t blk_max;
+
+    /* MemoryRegions exposed to user */
+    MemoryRegion regmr;
+    IOMMUMemoryRegion upstream;
+
+    /* MemoryRegion used internally */
+    MemoryRegion blocked_io;
+
+    AddressSpace downstream_as;
+    AddressSpace blocked_io_as;
+};
+
+#endif
diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
new file mode 100644
index 00000000000..569bccfda7a
--- /dev/null
+++ b/hw/misc/tz-mpc.c
@@ -0,0 +1,399 @@
+/*
+ * ARM AHB5 TrustZone Memory Protection Controller emulation
+ *
+ * Copyright (c) 2018 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.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "trace.h"
+#include "hw/sysbus.h"
+#include "hw/registerfields.h"
+#include "hw/misc/tz-mpc.h"
+
+/* Our IOMMU has two IOMMU indexes, one for secure transactions and one for
+ * non-secure transactions.
+ */
+enum {
+    IOMMU_IDX_S,
+    IOMMU_IDX_NS,
+    IOMMU_NUM_INDEXES,
+};
+
+/* Config registers */
+REG32(CTRL, 0x00)
+REG32(BLK_MAX, 0x10)
+REG32(BLK_CFG, 0x14)
+REG32(BLK_IDX, 0x18)
+REG32(BLK_LUT, 0x1c)
+REG32(INT_STAT, 0x20)
+REG32(INT_CLEAR, 0x24)
+REG32(INT_EN, 0x28)
+REG32(INT_INFO1, 0x2c)
+REG32(INT_INFO2, 0x30)
+REG32(INT_SET, 0x34)
+REG32(PIDR4, 0xfd0)
+REG32(PIDR5, 0xfd4)
+REG32(PIDR6, 0xfd8)
+REG32(PIDR7, 0xfdc)
+REG32(PIDR0, 0xfe0)
+REG32(PIDR1, 0xfe4)
+REG32(PIDR2, 0xfe8)
+REG32(PIDR3, 0xfec)
+REG32(CIDR0, 0xff0)
+REG32(CIDR1, 0xff4)
+REG32(CIDR2, 0xff8)
+REG32(CIDR3, 0xffc)
+
+static const uint8_t tz_mpc_idregs[] = {
+    0x04, 0x00, 0x00, 0x00,
+    0x60, 0xb8, 0x1b, 0x00,
+    0x0d, 0xf0, 0x05, 0xb1,
+};
+
+static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
+                                   uint64_t *pdata,
+                                   unsigned size, MemTxAttrs attrs)
+{
+    uint64_t r;
+    uint32_t offset = addr & ~0x3;
+
+    if (!attrs.secure && offset < A_PIDR4) {
+        /* NS accesses can only see the ID registers */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register read: NS access to offset 0x%x\n",
+                      offset);
+        r = 0;
+        goto read_out;
+    }
+
+    switch (offset) {
+    case A_PIDR4:
+    case A_PIDR5:
+    case A_PIDR6:
+    case A_PIDR7:
+    case A_PIDR0:
+    case A_PIDR1:
+    case A_PIDR2:
+    case A_PIDR3:
+    case A_CIDR0:
+    case A_CIDR1:
+    case A_CIDR2:
+    case A_CIDR3:
+        r = tz_mpc_idregs[(offset - A_PIDR4) / 4];
+        break;
+    case A_INT_CLEAR:
+    case A_INT_SET:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register read: write-only offset 0x%x\n",
+                      offset);
+        r = 0;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register read: bad offset 0x%x\n", offset);
+        r = 0;
+        break;
+    }
+
+    if (size != 4) {
+        /* None of our registers are read-sensitive (except BLK_LUT,
+         * which can special case the "size not 4" case), so just
+         * pull the right bytes out of the word read result.
+         */
+        r = extract32(r, (addr & 3) * 8, size * 8);
+    }
+
+read_out:
+    trace_tz_mpc_reg_read(addr, r, size);
+    *pdata = r;
+    return MEMTX_OK;
+}
+
+static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
+                                    uint64_t value,
+                                    unsigned size, MemTxAttrs attrs)
+{
+    uint32_t offset = addr & ~0x3;
+
+    trace_tz_mpc_reg_write(addr, value, size);
+
+    if (!attrs.secure && offset < A_PIDR4) {
+        /* NS accesses can only see the ID registers */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register read: NS access to offset 0x%x\n",
+                      offset);
+        return MEMTX_OK;
+    }
+
+    if (size != 4) {
+        /* Expand the byte or halfword write to a full word size.
+         * In most cases we can do this with zeroes; the exceptions
+         * are CTRL, BLK_IDX and BLK_LUT.
+         */
+        uint32_t oldval;
+
+        switch (offset) {
+            /* As we add support for registers which need expansions
+             * other than zeroes we'll fill in cases here.
+             */
+        default:
+            oldval = 0;
+            break;
+        }
+        value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
+    }
+
+    switch (offset) {
+    case A_PIDR4:
+    case A_PIDR5:
+    case A_PIDR6:
+    case A_PIDR7:
+    case A_PIDR0:
+    case A_PIDR1:
+    case A_PIDR2:
+    case A_PIDR3:
+    case A_CIDR0:
+    case A_CIDR1:
+    case A_CIDR2:
+    case A_CIDR3:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register write: read-only offset 0x%x\n", offset);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "TZ MPC register write: bad offset 0x%x\n", offset);
+        break;
+    }
+
+    return MEMTX_OK;
+}
+
+static const MemoryRegionOps tz_mpc_reg_ops = {
+    .read_with_attrs = tz_mpc_reg_read,
+    .write_with_attrs = tz_mpc_reg_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 4,
+};
+
+/* Accesses only reach these read and write functions if the MPC is
+ * blocking them; non-blocked accesses go directly to the downstream
+ * memory region without passing through this code.
+ */
+static MemTxResult tz_mpc_mem_blocked_read(void *opaque, hwaddr addr,
+                                           uint64_t *pdata,
+                                           unsigned size, MemTxAttrs attrs)
+{
+    trace_tz_mpc_mem_blocked_read(addr, size, attrs.secure);
+
+    *pdata = 0;
+    return MEMTX_OK;
+}
+
+static MemTxResult tz_mpc_mem_blocked_write(void *opaque, hwaddr addr,
+                                            uint64_t value,
+                                            unsigned size, MemTxAttrs attrs)
+{
+    trace_tz_mpc_mem_blocked_write(addr, value, size, attrs.secure);
+
+    return MEMTX_OK;
+}
+
+static const MemoryRegionOps tz_mpc_mem_blocked_ops = {
+    .read_with_attrs = tz_mpc_mem_blocked_read,
+    .write_with_attrs = tz_mpc_mem_blocked_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 8,
+};
+
+static IOMMUTLBEntry tz_mpc_translate(IOMMUMemoryRegion *iommu,
+                                      hwaddr addr, IOMMUAccessFlags flags,
+                                      int iommu_idx)
+{
+    TZMPC *s = TZ_MPC(container_of(iommu, TZMPC, upstream));
+    bool ok;
+
+    IOMMUTLBEntry ret = {
+        .iova = addr & ~(s->blocksize - 1),
+        .translated_addr = addr & ~(s->blocksize - 1),
+        .addr_mask = s->blocksize - 1,
+        .perm = IOMMU_RW,
+    };
+
+    /* Look at the per-block configuration for this address, and
+     * return a TLB entry directing the transaction at either
+     * downstream_as or blocked_io_as, as appropriate.
+     * For the moment, always permit accesses.
+     */
+    ok = true;
+
+    trace_tz_mpc_translate(addr, flags,
+                           iommu_idx == IOMMU_IDX_S ? "S" : "NS",
+                           ok ? "pass" : "block");
+
+    ret.target_as = ok ? &s->downstream_as : &s->blocked_io_as;
+    return ret;
+}
+
+static int tz_mpc_attrs_to_index(IOMMUMemoryRegion *iommu, MemTxAttrs attrs)
+{
+    /* We treat unspecified attributes like secure. Transactions with
+     * unspecified attributes come from places like
+     * cpu_physical_memory_write_rom() for initial image load, and we want
+     * those to pass through the from-reset "everything is secure" config.
+     * All the real during-emulation transactions from the CPU will
+     * specify attributes.
+     */
+    return (attrs.unspecified || attrs.secure) ? IOMMU_IDX_S : IOMMU_IDX_NS;
+}
+
+static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
+{
+    return IOMMU_NUM_INDEXES;
+}
+
+static void tz_mpc_reset(DeviceState *dev)
+{
+}
+
+static void tz_mpc_init(Object *obj)
+{
+    DeviceState *dev = DEVICE(obj);
+    TZMPC *s = TZ_MPC(obj);
+
+    qdev_init_gpio_out_named(dev, &s->irq, "irq", 1);
+}
+
+static void tz_mpc_realize(DeviceState *dev, Error **errp)
+{
+    Object *obj = OBJECT(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    TZMPC *s = TZ_MPC(dev);
+    uint64_t size;
+
+    /* 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.
+     * We insist on having a downstream, to avoid complicating the code
+     * with handling the "don't know how big this is" case. It's easy
+     * enough for the user to create an unimplemented_device as downstream
+     * if they have nothing else to plug into this.
+     */
+    if (!s->downstream) {
+        error_setg(errp, "MPC 'downstream' link not set");
+        return;
+    }
+
+    size = memory_region_size(s->downstream);
+
+    memory_region_init_iommu(&s->upstream, sizeof(s->upstream),
+                             TYPE_TZ_MPC_IOMMU_MEMORY_REGION,
+                             obj, "tz-mpc-upstream", size);
+
+    /* In real hardware the block size is configurable. In QEMU we could
+     * make it configurable but will need it to be at least as big as the
+     * target page size so we can execute out of the resulting MRs. Guest
+     * software is supposed to check the block size using the BLK_CFG
+     * register, so make it fixed at the page size.
+     */
+    s->blocksize = memory_region_iommu_get_min_page_size(&s->upstream);
+    if (size % s->blocksize != 0) {
+        error_setg(errp,
+                   "MPC 'downstream' size %" PRId64
+                   " is not a multiple of %" HWADDR_PRIx " bytes",
+                   size, s->blocksize);
+        object_unref(OBJECT(&s->upstream));
+        return;
+    }
+
+    /* BLK_MAX is the max value of BLK_IDX, which indexes an array of 32-bit
+     * words, each bit of which indicates one block.
+     */
+    s->blk_max = DIV_ROUND_UP(size / s->blocksize, 32);
+
+    memory_region_init_io(&s->regmr, obj, &tz_mpc_reg_ops,
+                          s, "tz-mpc-regs", 0x1000);
+    sysbus_init_mmio(sbd, &s->regmr);
+
+    sysbus_init_mmio(sbd, MEMORY_REGION(&s->upstream));
+
+    /* This memory region is not exposed to users of this device as a
+     * sysbus MMIO region, but is instead used internally as something
+     * that our IOMMU translate function might direct accesses to.
+     */
+    memory_region_init_io(&s->blocked_io, obj, &tz_mpc_mem_blocked_ops,
+                          s, "tz-mpc-blocked-io", size);
+
+    address_space_init(&s->downstream_as, s->downstream,
+                       "tz-mpc-downstream");
+    address_space_init(&s->blocked_io_as, &s->blocked_io,
+                       "tz-mpc-blocked-io");
+}
+
+static const VMStateDescription tz_mpc_vmstate = {
+    .name = "tz-mpc",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property tz_mpc_properties[] = {
+    DEFINE_PROP_LINK("downstream", TZMPC, downstream,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void tz_mpc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = tz_mpc_realize;
+    dc->vmsd = &tz_mpc_vmstate;
+    dc->reset = tz_mpc_reset;
+    dc->props = tz_mpc_properties;
+}
+
+static const TypeInfo tz_mpc_info = {
+    .name = TYPE_TZ_MPC,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(TZMPC),
+    .instance_init = tz_mpc_init,
+    .class_init = tz_mpc_class_init,
+};
+
+static void tz_mpc_iommu_memory_region_class_init(ObjectClass *klass,
+                                                  void *data)
+{
+    IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
+
+    imrc->translate = tz_mpc_translate;
+    imrc->attrs_to_index = tz_mpc_attrs_to_index;
+    imrc->num_indexes = tz_mpc_num_indexes;
+}
+
+static const TypeInfo tz_mpc_iommu_memory_region_info = {
+    .name = TYPE_TZ_MPC_IOMMU_MEMORY_REGION,
+    .parent = TYPE_IOMMU_MEMORY_REGION,
+    .class_init = tz_mpc_iommu_memory_region_class_init,
+};
+
+static void tz_mpc_register_types(void)
+{
+    type_register_static(&tz_mpc_info);
+    type_register_static(&tz_mpc_iommu_memory_region_info);
+}
+
+type_init(tz_mpc_register_types);
diff --git a/MAINTAINERS b/MAINTAINERS
index da91501c7a6..babbaaf5355 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -449,6 +449,8 @@ F: hw/char/cmsdk-apb-uart.c
 F: include/hw/char/cmsdk-apb-uart.h
 F: hw/misc/tz-ppc.c
 F: include/hw/misc/tz-ppc.h
+F: hw/misc/tz-mpc.c
+F: include/hw/misc/tz-mpc.h
 
 ARM cores
 M: Peter Maydell <peter.maydell@linaro.org>
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 7cf73d2f273..834d45cfaf9 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -108,6 +108,7 @@ CONFIG_CMSDK_APB_UART=y
 CONFIG_MPS2_FPGAIO=y
 CONFIG_MPS2_SCC=y
 
+CONFIG_TZ_MPC=y
 CONFIG_TZ_PPC=y
 CONFIG_IOTKIT=y
 CONFIG_IOTKIT_SECCTL=y
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index ec5a9f0da13..72bf9d57000 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -84,6 +84,13 @@ mos6522_set_sr_int(void) "set sr_int"
 mos6522_write(uint64_t addr, uint64_t val) "reg=0x%"PRIx64 " val=0x%"PRIx64
 mos6522_read(uint64_t addr, unsigned val) "reg=0x%"PRIx64 " val=0x%x"
 
+# hw/misc/tz-mpc.c
+tz_mpc_reg_read(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs read: offset 0x%x data 0x%" PRIx64 " size %u"
+tz_mpc_reg_write(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs write: offset 0x%x data 0x%" PRIx64 " size %u"
+tz_mpc_mem_blocked_read(uint64_t addr, unsigned size, bool secure) "TZ MPC blocked read: offset 0x%" PRIx64 " size %u secure %d"
+tz_mpc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size, bool secure) "TZ MPC blocked write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u secure %d"
+tz_mpc_translate(uint64_t addr, int flags, const char *idx, const char *res) "TZ MPC translate: addr 0x%" PRIx64 " flags 0x%x iommu_idx %s: %s"
+
 # hw/misc/tz-ppc.c
 tz_ppc_reset(void) "TZ PPC: reset"
 tz_ppc_cfg_nonsec(int n, int level) "TZ PPC: cfg_nonsec[%d] = %d"
-- 
2.17.1

[Qemu-devel] [PATCH v3 2/8] hw/misc/tz-mpc.c: Implement registers

[Peter Maydell]

Implement the missing registers for the TZ MPC.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/tz-mpc.h |  10 +++
 hw/misc/tz-mpc.c         | 142 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
index d1a65fd9a38..6f15945410d 100644
--- a/include/hw/misc/tz-mpc.h
+++ b/include/hw/misc/tz-mpc.h
@@ -48,6 +48,16 @@ struct TZMPC {
 
     /*< public >*/
 
+    /* State */
+    uint32_t ctrl;
+    uint32_t blk_idx;
+    uint32_t int_stat;
+    uint32_t int_en;
+    uint32_t int_info1;
+    uint32_t int_info2;
+
+    uint32_t *blk_lut;
+
     qemu_irq irq;
 
     /* Properties */
diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
index 569bccfda7a..e5b91bf81a3 100644
--- a/hw/misc/tz-mpc.c
+++ b/hw/misc/tz-mpc.c
@@ -28,16 +28,23 @@ enum {
 
 /* Config registers */
 REG32(CTRL, 0x00)
+    FIELD(CTRL, SEC_RESP, 4, 1)
+    FIELD(CTRL, AUTOINC, 8, 1)
+    FIELD(CTRL, LOCKDOWN, 31, 1)
 REG32(BLK_MAX, 0x10)
 REG32(BLK_CFG, 0x14)
 REG32(BLK_IDX, 0x18)
 REG32(BLK_LUT, 0x1c)
 REG32(INT_STAT, 0x20)
+    FIELD(INT_STAT, IRQ, 0, 1)
 REG32(INT_CLEAR, 0x24)
+    FIELD(INT_CLEAR, IRQ, 0, 1)
 REG32(INT_EN, 0x28)
+    FIELD(INT_EN, IRQ, 0, 1)
 REG32(INT_INFO1, 0x2c)
 REG32(INT_INFO2, 0x30)
 REG32(INT_SET, 0x34)
+    FIELD(INT_SET, IRQ, 0, 1)
 REG32(PIDR4, 0xfd0)
 REG32(PIDR5, 0xfd4)
 REG32(PIDR6, 0xfd8)
@@ -57,10 +64,25 @@ static const uint8_t tz_mpc_idregs[] = {
     0x0d, 0xf0, 0x05, 0xb1,
 };
 
+static void tz_mpc_irq_update(TZMPC *s)
+{
+    qemu_set_irq(s->irq, s->int_stat && s->int_en);
+}
+
+static void tz_mpc_autoinc_idx(TZMPC *s, unsigned access_size)
+{
+    /* Auto-increment BLK_IDX if necessary */
+    if (access_size == 4 && (s->ctrl & R_CTRL_AUTOINC_MASK)) {
+        s->blk_idx++;
+        s->blk_idx %= s->blk_max;
+    }
+}
+
 static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
                                    uint64_t *pdata,
                                    unsigned size, MemTxAttrs attrs)
 {
+    TZMPC *s = TZ_MPC(opaque);
     uint64_t r;
     uint32_t offset = addr & ~0x3;
 
@@ -74,6 +96,38 @@ static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
     }
 
     switch (offset) {
+    case A_CTRL:
+        r = s->ctrl;
+        break;
+    case A_BLK_MAX:
+        r = s->blk_max;
+        break;
+    case A_BLK_CFG:
+        /* We are never in "init in progress state", so this just indicates
+         * the block size. s->blocksize == (1 << BLK_CFG + 5), so
+         * BLK_CFG == ctz32(s->blocksize) - 5
+         */
+        r = ctz32(s->blocksize) - 5;
+        break;
+    case A_BLK_IDX:
+        r = s->blk_idx;
+        break;
+    case A_BLK_LUT:
+        r = s->blk_lut[s->blk_idx];
+        tz_mpc_autoinc_idx(s, size);
+        break;
+    case A_INT_STAT:
+        r = s->int_stat;
+        break;
+    case A_INT_EN:
+        r = s->int_en;
+        break;
+    case A_INT_INFO1:
+        r = s->int_info1;
+        break;
+    case A_INT_INFO2:
+        r = s->int_info2;
+        break;
     case A_PIDR4:
     case A_PIDR5:
     case A_PIDR6:
@@ -120,6 +174,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
                                     uint64_t value,
                                     unsigned size, MemTxAttrs attrs)
 {
+    TZMPC *s = TZ_MPC(opaque);
     uint32_t offset = addr & ~0x3;
 
     trace_tz_mpc_reg_write(addr, value, size);
@@ -127,7 +182,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
     if (!attrs.secure && offset < A_PIDR4) {
         /* NS accesses can only see the ID registers */
         qemu_log_mask(LOG_GUEST_ERROR,
-                      "TZ MPC register read: NS access to offset 0x%x\n",
+                      "TZ MPC register write: NS access to offset 0x%x\n",
                       offset);
         return MEMTX_OK;
     }
@@ -140,9 +195,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
         uint32_t oldval;
 
         switch (offset) {
-            /* As we add support for registers which need expansions
-             * other than zeroes we'll fill in cases here.
-             */
+        case A_CTRL:
+            oldval = s->ctrl;
+            break;
+        case A_BLK_IDX:
+            oldval = s->blk_idx;
+            break;
+        case A_BLK_LUT:
+            oldval = s->blk_lut[s->blk_idx];
+            break;
         default:
             oldval = 0;
             break;
@@ -150,7 +211,48 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
         value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
     }
 
+    if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
+        (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
+        /* Lockdown mode makes these three registers read-only, and
+         * the only way out of it is to reset the device.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
+                      "while MPC is in lockdown mode\n", offset);
+        return MEMTX_OK;
+    }
+
     switch (offset) {
+    case A_CTRL:
+        /* We don't implement the 'data gating' feature so all other bits
+         * are reserved and we make them RAZ/WI.
+         */
+        s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
+                           R_CTRL_AUTOINC_MASK |
+                           R_CTRL_LOCKDOWN_MASK);
+        break;
+    case A_BLK_IDX:
+        s->blk_idx = value % s->blk_max;
+        break;
+    case A_BLK_LUT:
+        s->blk_lut[s->blk_idx] = value;
+        tz_mpc_autoinc_idx(s, size);
+        break;
+    case A_INT_CLEAR:
+        if (value & R_INT_CLEAR_IRQ_MASK) {
+            s->int_stat = 0;
+            tz_mpc_irq_update(s);
+        }
+        break;
+    case A_INT_EN:
+        s->int_en = value & R_INT_EN_IRQ_MASK;
+        tz_mpc_irq_update(s);
+        break;
+    case A_INT_SET:
+        if (value & R_INT_SET_IRQ_MASK) {
+            s->int_stat = R_INT_STAT_IRQ_MASK;
+            tz_mpc_irq_update(s);
+        }
+        break;
     case A_PIDR4:
     case A_PIDR5:
     case A_PIDR6:
@@ -266,6 +368,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
 
 static void tz_mpc_reset(DeviceState *dev)
 {
+    TZMPC *s = TZ_MPC(dev);
+
+    s->ctrl = 0x00000100;
+    s->blk_idx = 0;
+    s->int_stat = 0;
+    s->int_en = 1;
+    s->int_info1 = 0;
+    s->int_info2 = 0;
+
+    memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
 }
 
 static void tz_mpc_init(Object *obj)
@@ -339,13 +451,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
                        "tz-mpc-downstream");
     address_space_init(&s->blocked_io_as, &s->blocked_io,
                        "tz-mpc-blocked-io");
+
+    s->blk_lut = g_new(uint32_t, s->blk_max);
+}
+
+static int tz_mpc_post_load(void *opaque, int version_id)
+{
+    TZMPC *s = TZ_MPC(opaque);
+
+    /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
+    if (s->blk_idx >= s->blk_max) {
+        return -1;
+    }
+    return 0;
 }
 
 static const VMStateDescription tz_mpc_vmstate = {
     .name = "tz-mpc",
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = tz_mpc_post_load,
     .fields = (VMStateField[]) {
+        VMSTATE_UINT32(ctrl, TZMPC),
+        VMSTATE_UINT32(blk_idx, TZMPC),
+        VMSTATE_UINT32(int_stat, TZMPC),
+        VMSTATE_UINT32(int_en, TZMPC),
+        VMSTATE_UINT32(int_info1, TZMPC),
+        VMSTATE_UINT32(int_info2, TZMPC),
+        VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
+                              0, vmstate_info_uint32, uint32_t),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.17.1

[Qemu-devel] [PATCH v3 3/8] hw/misc/tz-mpc.c: Implement correct blocked-access behaviour

[Peter Maydell]

The MPC is guest-configurable for whether blocked accesses:
 * should be RAZ/WI or cause a bus error
 * should generate an interrupt or not

Implement this behaviour in the blocked-access handlers.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/misc/tz-mpc.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
index e5b91bf81a3..fded5922a21 100644
--- a/hw/misc/tz-mpc.c
+++ b/hw/misc/tz-mpc.c
@@ -43,6 +43,9 @@ REG32(INT_EN, 0x28)
     FIELD(INT_EN, IRQ, 0, 1)
 REG32(INT_INFO1, 0x2c)
 REG32(INT_INFO2, 0x30)
+    FIELD(INT_INFO2, HMASTER, 0, 16)
+    FIELD(INT_INFO2, HNONSEC, 16, 1)
+    FIELD(INT_INFO2, CFG_NS, 17, 1)
 REG32(INT_SET, 0x34)
     FIELD(INT_SET, IRQ, 0, 1)
 REG32(PIDR4, 0xfd0)
@@ -287,6 +290,45 @@ static const MemoryRegionOps tz_mpc_reg_ops = {
     .impl.max_access_size = 4,
 };
 
+static inline bool tz_mpc_cfg_ns(TZMPC *s, hwaddr addr)
+{
+    /* Return the cfg_ns bit from the LUT for the specified address */
+    hwaddr blknum = addr / s->blocksize;
+    hwaddr blkword = blknum / 32;
+    uint32_t blkbit = 1U << (blknum % 32);
+
+    /* This would imply the address was larger than the size we
+     * defined this memory region to be, so it can't happen.
+     */
+    assert(blkword < s->blk_max);
+    return s->blk_lut[blkword] & blkbit;
+}
+
+static MemTxResult tz_mpc_handle_block(TZMPC *s, hwaddr addr, MemTxAttrs attrs)
+{
+    /* Handle a blocked transaction: raise IRQ, capture info, etc */
+    if (!s->int_stat) {
+        /* First blocked transfer: capture information into INT_INFO1 and
+         * INT_INFO2. Subsequent transfers are still blocked but don't
+         * capture information until the guest clears the interrupt.
+         */
+
+        s->int_info1 = addr;
+        s->int_info2 = 0;
+        s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, HMASTER,
+                                  attrs.requester_id & 0xffff);
+        s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, HNONSEC,
+                                  ~attrs.secure);
+        s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, CFG_NS,
+                                  tz_mpc_cfg_ns(s, addr));
+        s->int_stat |= R_INT_STAT_IRQ_MASK;
+        tz_mpc_irq_update(s);
+    }
+
+    /* Generate bus error if desired; otherwise RAZ/WI */
+    return (s->ctrl & R_CTRL_SEC_RESP_MASK) ? MEMTX_ERROR : MEMTX_OK;
+}
+
 /* Accesses only reach these read and write functions if the MPC is
  * blocking them; non-blocked accesses go directly to the downstream
  * memory region without passing through this code.
@@ -295,19 +337,23 @@ static MemTxResult tz_mpc_mem_blocked_read(void *opaque, hwaddr addr,
                                            uint64_t *pdata,
                                            unsigned size, MemTxAttrs attrs)
 {
+    TZMPC *s = TZ_MPC(opaque);
+
     trace_tz_mpc_mem_blocked_read(addr, size, attrs.secure);
 
     *pdata = 0;
-    return MEMTX_OK;
+    return tz_mpc_handle_block(s, addr, attrs);
 }
 
 static MemTxResult tz_mpc_mem_blocked_write(void *opaque, hwaddr addr,
                                             uint64_t value,
                                             unsigned size, MemTxAttrs attrs)
 {
+    TZMPC *s = TZ_MPC(opaque);
+
     trace_tz_mpc_mem_blocked_write(addr, value, size, attrs.secure);
 
-    return MEMTX_OK;
+    return tz_mpc_handle_block(s, addr, attrs);
 }
 
 static const MemoryRegionOps tz_mpc_mem_blocked_ops = {
-- 
2.17.1

[Qemu-devel] [PATCH v3 4/8] hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate

[Peter Maydell]

The final part of the Memory Protection Controller we need to
implement is actually using the BLK_LUT data programmed by the
guest to determine whether to block the transaction or not.

Since this means we now change transaction mappings when
the guest writes to BLK_LUT, we must also call the IOMMU
notifiers at that point.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/tz-mpc.c     | 53 ++++++++++++++++++++++++++++++++++++++++++--
 hw/misc/trace-events |  1 +
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
index fded5922a21..8316079b4bf 100644
--- a/hw/misc/tz-mpc.c
+++ b/hw/misc/tz-mpc.c
@@ -72,6 +72,53 @@ static void tz_mpc_irq_update(TZMPC *s)
     qemu_set_irq(s->irq, s->int_stat && s->int_en);
 }
 
+static void tz_mpc_iommu_notify(TZMPC *s, uint32_t lutidx,
+                                uint32_t oldlut, uint32_t newlut)
+{
+    /* Called when the LUT word at lutidx has changed from oldlut to newlut;
+     * must call the IOMMU notifiers for the changed blocks.
+     */
+    IOMMUTLBEntry entry = {
+        .addr_mask = s->blocksize - 1,
+    };
+    hwaddr addr = lutidx * s->blocksize * 32;
+    int i;
+
+    for (i = 0; i < 32; i++, addr += s->blocksize) {
+        bool block_is_ns;
+
+        if (!((oldlut ^ newlut) & (1 << i))) {
+            continue;
+        }
+        /* This changes the mappings for both the S and the NS space,
+         * so we need to do four notifies: an UNMAP then a MAP for each.
+         */
+        block_is_ns = newlut & (1 << i);
+
+        trace_tz_mpc_iommu_notify(addr);
+        entry.iova = addr;
+        entry.translated_addr = addr;
+
+        entry.perm = IOMMU_NONE;
+        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry);
+        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry);
+
+        entry.perm = IOMMU_RW;
+        if (block_is_ns) {
+            entry.target_as = &s->blocked_io_as;
+        } else {
+            entry.target_as = &s->downstream_as;
+        }
+        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry);
+        if (block_is_ns) {
+            entry.target_as = &s->downstream_as;
+        } else {
+            entry.target_as = &s->blocked_io_as;
+        }
+        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry);
+    }
+}
+
 static void tz_mpc_autoinc_idx(TZMPC *s, unsigned access_size)
 {
     /* Auto-increment BLK_IDX if necessary */
@@ -237,6 +284,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
         s->blk_idx = value % s->blk_max;
         break;
     case A_BLK_LUT:
+        tz_mpc_iommu_notify(s, s->blk_idx, s->blk_lut[s->blk_idx], value);
         s->blk_lut[s->blk_idx] = value;
         tz_mpc_autoinc_idx(s, size);
         break;
@@ -383,9 +431,10 @@ static IOMMUTLBEntry tz_mpc_translate(IOMMUMemoryRegion *iommu,
     /* Look at the per-block configuration for this address, and
      * return a TLB entry directing the transaction at either
      * downstream_as or blocked_io_as, as appropriate.
-     * For the moment, always permit accesses.
+     * If the LUT cfg_ns bit is 1, only non-secure transactions
+     * may pass. If the bit is 0, only secure transactions may pass.
      */
-    ok = true;
+    ok = tz_mpc_cfg_ns(s, addr) == (iommu_idx == IOMMU_IDX_NS);
 
     trace_tz_mpc_translate(addr, flags,
                            iommu_idx == IOMMU_IDX_S ? "S" : "NS",
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 72bf9d57000..c956e1419b7 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -90,6 +90,7 @@ tz_mpc_reg_write(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs wri
 tz_mpc_mem_blocked_read(uint64_t addr, unsigned size, bool secure) "TZ MPC blocked read: offset 0x%" PRIx64 " size %u secure %d"
 tz_mpc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size, bool secure) "TZ MPC blocked write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u secure %d"
 tz_mpc_translate(uint64_t addr, int flags, const char *idx, const char *res) "TZ MPC translate: addr 0x%" PRIx64 " flags 0x%x iommu_idx %s: %s"
+tz_mpc_iommu_notify(uint64_t addr) "TZ MPC iommu: notifying UNMAP/MAP for 0x%" PRIx64
 
 # hw/misc/tz-ppc.c
 tz_ppc_reset(void) "TZ PPC: reset"
-- 
2.17.1

[Qemu-devel] [PATCH v3 5/8] hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS

[Peter Maydell]

Implement the SECMPCINTSTATUS register. This is the only register
in the security controller that deals with Memory Protection
Controllers, and it simply provides a read-only view of the
interrupt lines from the various MPCs in the system.

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

diff --git a/include/hw/misc/iotkit-secctl.h b/include/hw/misc/iotkit-secctl.h
index faad0c91901..082c14c925e 100644
--- a/include/hw/misc/iotkit-secctl.h
+++ b/include/hw/misc/iotkit-secctl.h
@@ -39,6 +39,11 @@
  *  + named GPIO outputs ahb_ppcexp{0,1,2,3}_irq_enable
  *  + named GPIO outputs ahb_ppcexp{0,1,2,3}_irq_clear
  *  + named GPIO inputs ahb_ppcexp{0,1,2,3}_irq_status
+ * Controlling the MPC in the IoTKit:
+ *  + named GPIO input mpc_status
+ * Controlling each of the 16 expansion MPCs which a system using the IoTKit
+ * might provide:
+ *  + named GPIO inputs mpcexp_status[0..15]
  */
 
 #ifndef IOTKIT_SECCTL_H
@@ -55,6 +60,8 @@
 #define IOTS_NUM_APB_PPC 2
 #define IOTS_NUM_APB_EXP_PPC 4
 #define IOTS_NUM_AHB_EXP_PPC 4
+#define IOTS_NUM_EXP_MPC 16
+#define IOTS_NUM_MPC 1
 
 typedef struct IoTKitSecCtl IoTKitSecCtl;
 
@@ -94,6 +101,7 @@ struct IoTKitSecCtl {
     uint32_t secrespcfg;
     uint32_t nsccfg;
     uint32_t brginten;
+    uint32_t mpcintstatus;
 
     IoTKitSecCtlPPC apb[IOTS_NUM_APB_PPC];
     IoTKitSecCtlPPC apbexp[IOTS_NUM_APB_EXP_PPC];
diff --git a/hw/misc/iotkit-secctl.c b/hw/misc/iotkit-secctl.c
index ddd1584d341..de4fd8e36d2 100644
--- a/hw/misc/iotkit-secctl.c
+++ b/hw/misc/iotkit-secctl.c
@@ -139,6 +139,9 @@ static MemTxResult iotkit_secctl_s_read(void *opaque, hwaddr addr,
     case A_NSCCFG:
         r = s->nsccfg;
         break;
+    case A_SECMPCINTSTATUS:
+        r = s->mpcintstatus;
+        break;
     case A_SECPPCINTSTAT:
         r = s->secppcintstat;
         break;
@@ -186,7 +189,6 @@ static MemTxResult iotkit_secctl_s_read(void *opaque, hwaddr addr,
     case A_APBSPPPCEXP3:
         r = s->apbexp[offset_to_ppc_idx(offset)].sp;
         break;
-    case A_SECMPCINTSTATUS:
     case A_SECMSCINTSTAT:
     case A_SECMSCINTEN:
     case A_NSMSCEXP:
@@ -572,6 +574,20 @@ static void iotkit_secctl_reset(DeviceState *dev)
     foreach_ppc(s, iotkit_secctl_reset_ppc);
 }
 
+static void iotkit_secctl_mpc_status(void *opaque, int n, int level)
+{
+    IoTKitSecCtl *s = IOTKIT_SECCTL(opaque);
+
+    s->mpcintstatus = deposit32(s->mpcintstatus, 0, 1, !!level);
+}
+
+static void iotkit_secctl_mpcexp_status(void *opaque, int n, int level)
+{
+    IoTKitSecCtl *s = IOTKIT_SECCTL(opaque);
+
+    s->mpcintstatus = deposit32(s->mpcintstatus, n + 16, 1, !!level);
+}
+
 static void iotkit_secctl_ppc_irqstatus(void *opaque, int n, int level)
 {
     IoTKitSecCtlPPC *ppc = opaque;
@@ -640,6 +656,10 @@ static void iotkit_secctl_init(Object *obj)
     qdev_init_gpio_out_named(dev, &s->sec_resp_cfg, "sec_resp_cfg", 1);
     qdev_init_gpio_out_named(dev, &s->nsc_cfg_irq, "nsc_cfg", 1);
 
+    qdev_init_gpio_in_named(dev, iotkit_secctl_mpc_status, "mpc_status", 1);
+    qdev_init_gpio_in_named(dev, iotkit_secctl_mpcexp_status,
+                            "mpcexp_status", IOTS_NUM_EXP_MPC);
+
     memory_region_init_io(&s->s_regs, obj, &iotkit_secctl_s_ops,
                           s, "iotkit-secctl-s-regs", 0x1000);
     memory_region_init_io(&s->ns_regs, obj, &iotkit_secctl_ns_ops,
@@ -660,6 +680,16 @@ static const VMStateDescription iotkit_secctl_ppc_vmstate = {
     }
 };
 
+static const VMStateDescription iotkit_secctl_mpcintstatus_vmstate = {
+    .name = "iotkit-secctl-mpcintstatus",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(mpcintstatus, IoTKitSecCtl),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription iotkit_secctl_vmstate = {
     .name = "iotkit-secctl",
     .version_id = 1,
@@ -677,7 +707,11 @@ static const VMStateDescription iotkit_secctl_vmstate = {
         VMSTATE_STRUCT_ARRAY(ahbexp, IoTKitSecCtl, IOTS_NUM_AHB_EXP_PPC, 1,
                              iotkit_secctl_ppc_vmstate, IoTKitSecCtlPPC),
         VMSTATE_END_OF_LIST()
-    }
+    },
+    .subsections = (const VMStateDescription*[]) {
+        &iotkit_secctl_mpcintstatus_vmstate,
+        NULL
+    },
 };
 
 static void iotkit_secctl_class_init(ObjectClass *klass, void *data)
-- 
2.17.1

[Qemu-devel] [PATCH v3 6/8] hw/arm/iotkit: Instantiate MPC

[Peter Maydell]

Wire up the one MPC that is part of the IoTKit itself. For the
moment we don't wire up its interrupt line.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/hw/arm/iotkit.h |  2 ++
 hw/arm/iotkit.c         | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/hw/arm/iotkit.h b/include/hw/arm/iotkit.h
index c6129d926b6..b21cf1ab9d1 100644
--- a/include/hw/arm/iotkit.h
+++ b/include/hw/arm/iotkit.h
@@ -51,6 +51,7 @@
 #include "hw/arm/armv7m.h"
 #include "hw/misc/iotkit-secctl.h"
 #include "hw/misc/tz-ppc.h"
+#include "hw/misc/tz-mpc.h"
 #include "hw/timer/cmsdk-apb-timer.h"
 #include "hw/misc/unimp.h"
 #include "hw/or-irq.h"
@@ -74,6 +75,7 @@ typedef struct IoTKit {
     IoTKitSecCtl secctl;
     TZPPC apb_ppc0;
     TZPPC apb_ppc1;
+    TZMPC mpc;
     CMSDKAPBTIMER timer0;
     CMSDKAPBTIMER timer1;
     qemu_or_irq ppc_irq_orgate;
diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
index 234185e8f78..160e40c7449 100644
--- a/hw/arm/iotkit.c
+++ b/hw/arm/iotkit.c
@@ -130,6 +130,7 @@ static void iotkit_init(Object *obj)
                       TYPE_TZ_PPC);
     init_sysbus_child(obj, "apb-ppc1", &s->apb_ppc1, sizeof(s->apb_ppc1),
                       TYPE_TZ_PPC);
+    init_sysbus_child(obj, "mpc", &s->mpc, sizeof(s->mpc), TYPE_TZ_MPC);
     init_sysbus_child(obj, "timer0", &s->timer0, sizeof(s->timer0),
                       TYPE_CMSDK_APB_TIMER);
     init_sysbus_child(obj, "timer1", &s->timer1, sizeof(s->timer1),
@@ -266,15 +267,6 @@ static void iotkit_realize(DeviceState *dev, Error **errp)
      */
     make_alias(s, &s->alias3, "alias 3", 0x50000000, 0x10000000, 0x40000000);
 
-    /* This RAM should be behind a Memory Protection Controller, but we
-     * don't implement that yet.
-     */
-    memory_region_init_ram(&s->sram0, NULL, "iotkit.sram0", 0x00008000, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-    memory_region_add_subregion(&s->container, 0x20000000, &s->sram0);
 
     /* Security controller */
     object_property_set_bool(OBJECT(&s->secctl), true, "realized", &err);
@@ -310,6 +302,32 @@ static void iotkit_realize(DeviceState *dev, Error **errp)
     qdev_connect_gpio_out_named(dev_secctl, "sec_resp_cfg", 0,
                                 qdev_get_gpio_in(dev_splitter, 0));
 
+    /* This RAM lives behind the Memory Protection Controller */
+    memory_region_init_ram(&s->sram0, NULL, "iotkit.sram0", 0x00008000, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    object_property_set_link(OBJECT(&s->mpc), OBJECT(&s->sram0),
+                             "downstream", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    object_property_set_bool(OBJECT(&s->mpc), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    /* Map the upstream end of the MPC into the right place... */
+    memory_region_add_subregion(&s->container, 0x20000000,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mpc),
+                                                       1));
+    /* ...and its register interface */
+    memory_region_add_subregion(&s->container, 0x50083000,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mpc),
+                                                       0));
+
     /* Devices behind APB PPC0:
      *   0x40000000: timer0
      *   0x40001000: timer1
@@ -473,8 +491,6 @@ static void iotkit_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("NS watchdog", 0x40081000, 0x1000);
     create_unimplemented_device("S watchdog", 0x50081000, 0x1000);
 
-    create_unimplemented_device("SRAM0 MPC", 0x50083000, 0x1000);
-
     for (i = 0; i < ARRAY_SIZE(s->ppc_irq_splitter); i++) {
         Object *splitter = OBJECT(&s->ppc_irq_splitter[i]);
 
-- 
2.17.1

[Qemu-devel] [PATCH v3 7/8] hw/arm/iotkit: Wire up MPC interrupt lines

[Peter Maydell]

The interrupt outputs from the MPC in the IoTKit and the expansion
MPCs in the board must be wired up to the security controller, and
also all ORed together to produce a single line to the NVIC.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/hw/arm/iotkit.h |  6 ++++
 hw/arm/iotkit.c         | 74 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/include/hw/arm/iotkit.h b/include/hw/arm/iotkit.h
index b21cf1ab9d1..2cddde55dd1 100644
--- a/include/hw/arm/iotkit.h
+++ b/include/hw/arm/iotkit.h
@@ -42,6 +42,9 @@
  *  + named GPIO outputs ahb_ppcexp{0,1,2,3}_irq_enable
  *  + named GPIO outputs ahb_ppcexp{0,1,2,3}_irq_clear
  *  + named GPIO inputs ahb_ppcexp{0,1,2,3}_irq_status
+ * Controlling each of the 16 expansion MPCs which a system using the IoTKit
+ * might provide:
+ *  + named GPIO inputs mpcexp_status[0..15]
  */
 
 #ifndef IOTKIT_H
@@ -81,6 +84,8 @@ typedef struct IoTKit {
     qemu_or_irq ppc_irq_orgate;
     SplitIRQ sec_resp_splitter;
     SplitIRQ ppc_irq_splitter[NUM_PPCS];
+    SplitIRQ mpc_irq_splitter[IOTS_NUM_EXP_MPC + IOTS_NUM_MPC];
+    qemu_or_irq mpc_irq_orgate;
 
     UnimplementedDeviceState dualtimer;
     UnimplementedDeviceState s32ktimer;
@@ -99,6 +104,7 @@ typedef struct IoTKit {
     qemu_irq nsc_cfg_in;
 
     qemu_irq irq_status_in[NUM_EXTERNAL_PPCS];
+    qemu_irq mpcexp_status_in[IOTS_NUM_EXP_MPC];
 
     uint32_t nsccfg;
 
diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
index 160e40c7449..133d5bb34f4 100644
--- a/hw/arm/iotkit.c
+++ b/hw/arm/iotkit.c
@@ -131,6 +131,18 @@ static void iotkit_init(Object *obj)
     init_sysbus_child(obj, "apb-ppc1", &s->apb_ppc1, sizeof(s->apb_ppc1),
                       TYPE_TZ_PPC);
     init_sysbus_child(obj, "mpc", &s->mpc, sizeof(s->mpc), TYPE_TZ_MPC);
+    object_initialize(&s->mpc_irq_orgate, sizeof(s->mpc_irq_orgate),
+                      TYPE_OR_IRQ);
+    object_property_add_child(obj, "mpc-irq-orgate",
+                              OBJECT(&s->mpc_irq_orgate), &error_abort);
+    for (i = 0; i < ARRAY_SIZE(s->mpc_irq_splitter); i++) {
+        char *name = g_strdup_printf("mpc-irq-splitter-%d", i);
+        SplitIRQ *splitter = &s->mpc_irq_splitter[i];
+
+        object_initialize(splitter, sizeof(*splitter), TYPE_SPLIT_IRQ);
+        object_property_add_child(obj, name, OBJECT(splitter), &error_abort);
+        g_free(name);
+    }
     init_sysbus_child(obj, "timer0", &s->timer0, sizeof(s->timer0),
                       TYPE_CMSDK_APB_TIMER);
     init_sysbus_child(obj, "timer1", &s->timer1, sizeof(s->timer1),
@@ -163,6 +175,12 @@ static void iotkit_exp_irq(void *opaque, int n, int level)
     qemu_set_irq(s->exp_irqs[n], level);
 }
 
+static void iotkit_mpcexp_status(void *opaque, int n, int level)
+{
+    IoTKit *s = IOTKIT(opaque);
+    qemu_set_irq(s->mpcexp_status_in[n], level);
+}
+
 static void iotkit_realize(DeviceState *dev, Error **errp)
 {
     IoTKit *s = IOTKIT(dev);
@@ -328,6 +346,22 @@ static void iotkit_realize(DeviceState *dev, Error **errp)
                                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mpc),
                                                        0));
 
+    /* We must OR together lines from the MPC splitters to go to the NVIC */
+    object_property_set_int(OBJECT(&s->mpc_irq_orgate),
+                            IOTS_NUM_EXP_MPC + IOTS_NUM_MPC, "num-lines", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    object_property_set_bool(OBJECT(&s->mpc_irq_orgate), true,
+                             "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    qdev_connect_gpio_out(DEVICE(&s->mpc_irq_orgate), 0,
+                          qdev_get_gpio_in(DEVICE(&s->armv7m), 9));
+
     /* Devices behind APB PPC0:
      *   0x40000000: timer0
      *   0x40001000: timer1
@@ -536,6 +570,46 @@ static void iotkit_realize(DeviceState *dev, Error **errp)
         g_free(gpioname);
     }
 
+    /* Wire up the splitters for the MPC IRQs */
+    for (i = 0; i < IOTS_NUM_EXP_MPC + IOTS_NUM_MPC; i++) {
+        SplitIRQ *splitter = &s->mpc_irq_splitter[i];
+        DeviceState *dev_splitter = DEVICE(splitter);
+
+        object_property_set_int(OBJECT(splitter), 2, "num-lines", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        object_property_set_bool(OBJECT(splitter), true, "realized", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        if (i < IOTS_NUM_EXP_MPC) {
+            /* Splitter input is from GPIO input line */
+            s->mpcexp_status_in[i] = qdev_get_gpio_in(dev_splitter, 0);
+            qdev_connect_gpio_out(dev_splitter, 0,
+                                  qdev_get_gpio_in_named(dev_secctl,
+                                                         "mpcexp_status", i));
+        } else {
+            /* Splitter input is from our own MPC */
+            qdev_connect_gpio_out_named(DEVICE(&s->mpc), "irq", 0,
+                                        qdev_get_gpio_in(dev_splitter, 0));
+            qdev_connect_gpio_out(dev_splitter, 0,
+                                  qdev_get_gpio_in_named(dev_secctl,
+                                                         "mpc_status", 0));
+        }
+
+        qdev_connect_gpio_out(dev_splitter, 1,
+                              qdev_get_gpio_in(DEVICE(&s->mpc_irq_orgate), i));
+    }
+    /* Create GPIO inputs which will pass the line state for our
+     * mpcexp_irq inputs to the correct splitter devices.
+     */
+    qdev_init_gpio_in_named(dev, iotkit_mpcexp_status, "mpcexp_status",
+                            IOTS_NUM_EXP_MPC);
+
     iotkit_forward_sec_resp_cfg(s);
 
     system_clock_scale = NANOSECONDS_PER_SECOND / s->mainclk_frq;
-- 
2.17.1

[Qemu-devel] [PATCH v3 8/8] hw/arm/mps2-tz.c: Instantiate MPCs

[Peter Maydell]

Instantiate and wire up the Memory Protection Controllers
in the MPS2 board itself.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/arm/mps2-tz.c | 71 ++++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 27 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index c5ef95e4cc0..22180c56fb7 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -44,6 +44,7 @@
 #include "hw/timer/cmsdk-apb-timer.h"
 #include "hw/misc/mps2-scc.h"
 #include "hw/misc/mps2-fpgaio.h"
+#include "hw/misc/tz-mpc.h"
 #include "hw/arm/iotkit.h"
 #include "hw/devices.h"
 #include "net/net.h"
@@ -64,13 +65,12 @@ typedef struct {
 
     IoTKit iotkit;
     MemoryRegion psram;
-    MemoryRegion ssram1;
+    MemoryRegion ssram[3];
     MemoryRegion ssram1_m;
-    MemoryRegion ssram23;
     MPS2SCC scc;
     MPS2FPGAIO fpgaio;
     TZPPC ppc[5];
-    UnimplementedDeviceState ssram_mpc[3];
+    TZMPC ssram_mpc[3];
     UnimplementedDeviceState spi[5];
     UnimplementedDeviceState i2c[4];
     UnimplementedDeviceState i2s_audio;
@@ -96,16 +96,6 @@ typedef struct {
 /* Main SYSCLK frequency in Hz */
 #define SYSCLK_FRQ 20000000
 
-/* Initialize the auxiliary RAM region @mr and map it into
- * the memory map at @base.
- */
-static void make_ram(MemoryRegion *mr, const char *name,
-                     hwaddr base, hwaddr size)
-{
-    memory_region_init_ram(mr, NULL, name, size, &error_fatal);
-    memory_region_add_subregion(get_system_memory(), base, mr);
-}
-
 /* Create an alias of an entire original MemoryRegion @orig
  * located at @base in the memory map.
  */
@@ -245,6 +235,44 @@ static MemoryRegion *make_eth_dev(MPS2TZMachineState *mms, void *opaque,
     return sysbus_mmio_get_region(s, 0);
 }
 
+static MemoryRegion *make_mpc(MPS2TZMachineState *mms, void *opaque,
+                              const char *name, hwaddr size)
+{
+    TZMPC *mpc = opaque;
+    int i = mpc - &mms->ssram_mpc[0];
+    MemoryRegion *ssram = &mms->ssram[i];
+    MemoryRegion *upstream;
+    char *mpcname = g_strdup_printf("%s-mpc", name);
+    static uint32_t ramsize[] = { 0x00400000, 0x00200000, 0x00200000 };
+    static uint32_t rambase[] = { 0x00000000, 0x28000000, 0x28200000 };
+
+    memory_region_init_ram(ssram, NULL, name, ramsize[i], &error_fatal);
+
+    init_sysbus_child(OBJECT(mms), mpcname, mpc,
+                      sizeof(mms->ssram_mpc[0]), TYPE_TZ_MPC);
+    object_property_set_link(OBJECT(mpc), OBJECT(ssram),
+                             "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(), rambase[i], upstream);
+    /* and connect its interrupt to the IoTKit */
+    qdev_connect_gpio_out_named(DEVICE(mpc), "irq", 0,
+                                qdev_get_gpio_in_named(DEVICE(&mms->iotkit),
+                                                       "mpcexp_status", i));
+
+    /* The first SSRAM is a special case as it has an alias; accesses to
+     * the alias region at 0x00400000 must also go to the MPC upstream.
+     */
+    if (i == 0) {
+        make_ram_alias(&mms->ssram1_m, "mps.ssram1_m", upstream, 0x00400000);
+    }
+
+    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 void mps2tz_common_init(MachineState *machine)
 {
     MPS2TZMachineState *mms = MPS2TZ_MACHINE(machine);
@@ -306,14 +334,6 @@ static void mps2tz_common_init(MachineState *machine)
                                          NULL, "mps.ram", 0x01000000);
     memory_region_add_subregion(system_memory, 0x80000000, &mms->psram);
 
-    /* The SSRAM memories should all be behind Memory Protection Controllers,
-     * but we don't implement that yet.
-     */
-    make_ram(&mms->ssram1, "mps.ssram1", 0x00000000, 0x00400000);
-    make_ram_alias(&mms->ssram1_m, "mps.ssram1_m", &mms->ssram1, 0x00400000);
-
-    make_ram(&mms->ssram23, "mps.ssram23", 0x28000000, 0x00400000);
-
     /* The overflow IRQs for all UARTs are ORed together.
      * Tx, Rx and "combined" IRQs are sent to the NVIC separately.
      * Create the OR gate for this.
@@ -343,12 +363,9 @@ static void mps2tz_common_init(MachineState *machine)
     const PPCInfo ppcs[] = { {
             .name = "apb_ppcexp0",
             .ports = {
-                { "ssram-mpc0", make_unimp_dev, &mms->ssram_mpc[0],
-                  0x58007000, 0x1000 },
-                { "ssram-mpc1", make_unimp_dev, &mms->ssram_mpc[1],
-                  0x58008000, 0x1000 },
-                { "ssram-mpc2", make_unimp_dev, &mms->ssram_mpc[2],
-                  0x58009000, 0x1000 },
+                { "ssram-0", make_mpc, &mms->ssram_mpc[0], 0x58007000, 0x1000 },
+                { "ssram-1", make_mpc, &mms->ssram_mpc[1], 0x58008000, 0x1000 },
+                { "ssram-2", make_mpc, &mms->ssram_mpc[2], 0x58009000, 0x1000 },
             },
         }, {
             .name = "apb_ppcexp1",
-- 
2.17.1

Re: [Qemu-devel] [PATCH v3 2/8] hw/misc/tz-mpc.c: Implement registers

[Auger Eric]

Hi Peter,

On 06/20/2018 03:20 PM, Peter Maydell wrote:
> Implement the missing registers for the TZ MPC.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/misc/tz-mpc.h |  10 +++
>  hw/misc/tz-mpc.c         | 142 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 148 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
> index d1a65fd9a38..6f15945410d 100644
> --- a/include/hw/misc/tz-mpc.h
> +++ b/include/hw/misc/tz-mpc.h
> @@ -48,6 +48,16 @@ struct TZMPC {
>  
>      /*< public >*/
>  
> +    /* State */
> +    uint32_t ctrl;
> +    uint32_t blk_idx;
> +    uint32_t int_stat;
> +    uint32_t int_en;
> +    uint32_t int_info1;
> +    uint32_t int_info2;
> +
> +    uint32_t *blk_lut;
> +
>      qemu_irq irq;
>  
>      /* Properties */
> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
> index 569bccfda7a..e5b91bf81a3 100644
> --- a/hw/misc/tz-mpc.c
> +++ b/hw/misc/tz-mpc.c
> @@ -28,16 +28,23 @@ enum {
>  
>  /* Config registers */
>  REG32(CTRL, 0x00)
> +    FIELD(CTRL, SEC_RESP, 4, 1)
> +    FIELD(CTRL, AUTOINC, 8, 1)
> +    FIELD(CTRL, LOCKDOWN, 31, 1)
>  REG32(BLK_MAX, 0x10)
>  REG32(BLK_CFG, 0x14)
>  REG32(BLK_IDX, 0x18)
>  REG32(BLK_LUT, 0x1c)
>  REG32(INT_STAT, 0x20)
> +    FIELD(INT_STAT, IRQ, 0, 1)
>  REG32(INT_CLEAR, 0x24)
> +    FIELD(INT_CLEAR, IRQ, 0, 1)
>  REG32(INT_EN, 0x28)
> +    FIELD(INT_EN, IRQ, 0, 1)
>  REG32(INT_INFO1, 0x2c)
>  REG32(INT_INFO2, 0x30)
>  REG32(INT_SET, 0x34)
> +    FIELD(INT_SET, IRQ, 0, 1)
>  REG32(PIDR4, 0xfd0)
>  REG32(PIDR5, 0xfd4)
>  REG32(PIDR6, 0xfd8)
> @@ -57,10 +64,25 @@ static const uint8_t tz_mpc_idregs[] = {
>      0x0d, 0xf0, 0x05, 0xb1,
>  };
>  
> +static void tz_mpc_irq_update(TZMPC *s)
> +{
> +    qemu_set_irq(s->irq, s->int_stat && s->int_en);
> +}
> +
> +static void tz_mpc_autoinc_idx(TZMPC *s, unsigned access_size)
> +{
> +    /* Auto-increment BLK_IDX if necessary */
> +    if (access_size == 4 && (s->ctrl & R_CTRL_AUTOINC_MASK)) {
> +        s->blk_idx++;
> +        s->blk_idx %= s->blk_max;
> +    }
> +}
> +
>  static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
>                                     uint64_t *pdata,
>                                     unsigned size, MemTxAttrs attrs)
>  {
> +    TZMPC *s = TZ_MPC(opaque);
>      uint64_t r;
>      uint32_t offset = addr & ~0x3;
>  
> @@ -74,6 +96,38 @@ static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
>      }
>  
>      switch (offset) {
> +    case A_CTRL:
> +        r = s->ctrl;
> +        break;
> +    case A_BLK_MAX:
> +        r = s->blk_max;
> +        break;
> +    case A_BLK_CFG:
> +        /* We are never in "init in progress state", so this just indicates
> +         * the block size. s->blocksize == (1 << BLK_CFG + 5), so
> +         * BLK_CFG == ctz32(s->blocksize) - 5
> +         */
> +        r = ctz32(s->blocksize) - 5;
> +        break;
> +    case A_BLK_IDX:
> +        r = s->blk_idx;
> +        break;
> +    case A_BLK_LUT:
> +        r = s->blk_lut[s->blk_idx];
> +        tz_mpc_autoinc_idx(s, size);
> +        break;
> +    case A_INT_STAT:
> +        r = s->int_stat;
> +        break;
> +    case A_INT_EN:
> +        r = s->int_en;
> +        break;
> +    case A_INT_INFO1:
> +        r = s->int_info1;
> +        break;
> +    case A_INT_INFO2:
> +        r = s->int_info2;
> +        break;
>      case A_PIDR4:
>      case A_PIDR5:
>      case A_PIDR6:
> @@ -120,6 +174,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>                                      uint64_t value,
>                                      unsigned size, MemTxAttrs attrs)
>  {
> +    TZMPC *s = TZ_MPC(opaque);
>      uint32_t offset = addr & ~0x3;
>  
>      trace_tz_mpc_reg_write(addr, value, size);
> @@ -127,7 +182,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>      if (!attrs.secure && offset < A_PIDR4) {
>          /* NS accesses can only see the ID registers */
When reading
"Identification registers can be read by any type of access." I
understand all others can only be read by secure accesses. So I would
have expected the same check on read side.

Besides

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
>          qemu_log_mask(LOG_GUEST_ERROR,
> -                      "TZ MPC register read: NS access to offset 0x%x\n",
> +                      "TZ MPC register write: NS access to offset 0x%x\n",
>                        offset);
>          return MEMTX_OK;
>      }
> @@ -140,9 +195,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>          uint32_t oldval;
>  
>          switch (offset) {
> -            /* As we add support for registers which need expansions
> -             * other than zeroes we'll fill in cases here.
> -             */
> +        case A_CTRL:
> +            oldval = s->ctrl;
> +            break;
> +        case A_BLK_IDX:
> +            oldval = s->blk_idx;
> +            break;
> +        case A_BLK_LUT:
> +            oldval = s->blk_lut[s->blk_idx];
> +            break;
>          default:
>              oldval = 0;
>              break;
> @@ -150,7 +211,48 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>          value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
>      }
>  
> +    if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
> +        (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
> +        /* Lockdown mode makes these three registers read-only, and
> +         * the only way out of it is to reset the device.
> +         */
> +        qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
> +                      "while MPC is in lockdown mode\n", offset);
> +        return MEMTX_OK;
> +    }
> +
>      switch (offset) {
> +    case A_CTRL:
> +        /* We don't implement the 'data gating' feature so all other bits
> +         * are reserved and we make them RAZ/WI.
> +         */
> +        s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
> +                           R_CTRL_AUTOINC_MASK |
> +                           R_CTRL_LOCKDOWN_MASK);
> +        break;
> +    case A_BLK_IDX:
> +        s->blk_idx = value % s->blk_max;
> +        break;
> +    case A_BLK_LUT:
> +        s->blk_lut[s->blk_idx] = value;
> +        tz_mpc_autoinc_idx(s, size);
> +        break;
> +    case A_INT_CLEAR:
> +        if (value & R_INT_CLEAR_IRQ_MASK) {
> +            s->int_stat = 0;
> +            tz_mpc_irq_update(s);
> +        }
> +        break;
> +    case A_INT_EN:
> +        s->int_en = value & R_INT_EN_IRQ_MASK;
> +        tz_mpc_irq_update(s);
> +        break;
> +    case A_INT_SET:
> +        if (value & R_INT_SET_IRQ_MASK) {
> +            s->int_stat = R_INT_STAT_IRQ_MASK;
> +            tz_mpc_irq_update(s);
> +        }
> +        break;
>      case A_PIDR4:
>      case A_PIDR5:
>      case A_PIDR6:
> @@ -266,6 +368,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
>  
>  static void tz_mpc_reset(DeviceState *dev)
>  {
> +    TZMPC *s = TZ_MPC(dev);
> +
> +    s->ctrl = 0x00000100;
> +    s->blk_idx = 0;
> +    s->int_stat = 0;
> +    s->int_en = 1;
> +    s->int_info1 = 0;
> +    s->int_info2 = 0;
> +
> +    memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
>  }
>  
>  static void tz_mpc_init(Object *obj)
> @@ -339,13 +451,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
>                         "tz-mpc-downstream");
>      address_space_init(&s->blocked_io_as, &s->blocked_io,
>                         "tz-mpc-blocked-io");
> +
> +    s->blk_lut = g_new(uint32_t, s->blk_max);
> +}
> +
> +static int tz_mpc_post_load(void *opaque, int version_id)
> +{
> +    TZMPC *s = TZ_MPC(opaque);
> +
> +    /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
> +    if (s->blk_idx >= s->blk_max) {
> +        return -1;
> +    }
> +    return 0;
>  }
>  
>  static const VMStateDescription tz_mpc_vmstate = {
>      .name = "tz-mpc",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .post_load = tz_mpc_post_load,
>      .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(ctrl, TZMPC),
> +        VMSTATE_UINT32(blk_idx, TZMPC),
> +        VMSTATE_UINT32(int_stat, TZMPC),
> +        VMSTATE_UINT32(int_en, TZMPC),
> +        VMSTATE_UINT32(int_info1, TZMPC),
> +        VMSTATE_UINT32(int_info2, TZMPC),
> +        VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
> +                              0, vmstate_info_uint32, uint32_t),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 

Re: [Qemu-devel] [PATCH v3 4/8] hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate

[Auger Eric]

Hi Peter,

On 06/20/2018 03:20 PM, Peter Maydell wrote:
> The final part of the Memory Protection Controller we need to
> implement is actually using the BLK_LUT data programmed by the
> guest to determine whether to block the transaction or not.
> 
> Since this means we now change transaction mappings when
> the guest writes to BLK_LUT, we must also call the IOMMU
> notifiers at that point.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

> ---
>  hw/misc/tz-mpc.c     | 53 ++++++++++++++++++++++++++++++++++++++++++--
>  hw/misc/trace-events |  1 +
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
> index fded5922a21..8316079b4bf 100644
> --- a/hw/misc/tz-mpc.c
> +++ b/hw/misc/tz-mpc.c
> @@ -72,6 +72,53 @@ static void tz_mpc_irq_update(TZMPC *s)
>      qemu_set_irq(s->irq, s->int_stat && s->int_en);
>  }
>  
> +static void tz_mpc_iommu_notify(TZMPC *s, uint32_t lutidx,
> +                                uint32_t oldlut, uint32_t newlut)
> +{
> +    /* Called when the LUT word at lutidx has changed from oldlut to newlut;
> +     * must call the IOMMU notifiers for the changed blocks.
> +     */
> +    IOMMUTLBEntry entry = {
> +        .addr_mask = s->blocksize - 1,
> +    };
> +    hwaddr addr = lutidx * s->blocksize * 32;
> +    int i;
> +
> +    for (i = 0; i < 32; i++, addr += s->blocksize) {
> +        bool block_is_ns;
> +
> +        if (!((oldlut ^ newlut) & (1 << i))) {
> +            continue;
> +        }
> +        /* This changes the mappings for both the S and the NS space,
> +         * so we need to do four notifies: an UNMAP then a MAP for each.
> +         */
> +        block_is_ns = newlut & (1 << i);
> +
> +        trace_tz_mpc_iommu_notify(addr);
> +        entry.iova = addr;
> +        entry.translated_addr = addr;
> +
> +        entry.perm = IOMMU_NONE;
> +        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry);
> +        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry);
> +
> +        entry.perm = IOMMU_RW;
> +        if (block_is_ns) {
> +            entry.target_as = &s->blocked_io_as;
> +        } else {
> +            entry.target_as = &s->downstream_as;
> +        }
> +        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry);
> +        if (block_is_ns) {
> +            entry.target_as = &s->downstream_as;
> +        } else {
> +            entry.target_as = &s->blocked_io_as;
> +        }
> +        memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry);
> +    }
> +}
> +
>  static void tz_mpc_autoinc_idx(TZMPC *s, unsigned access_size)
>  {
>      /* Auto-increment BLK_IDX if necessary */
> @@ -237,6 +284,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>          s->blk_idx = value % s->blk_max;
>          break;
>      case A_BLK_LUT:
> +        tz_mpc_iommu_notify(s, s->blk_idx, s->blk_lut[s->blk_idx], value);
>          s->blk_lut[s->blk_idx] = value;
>          tz_mpc_autoinc_idx(s, size);
>          break;
> @@ -383,9 +431,10 @@ static IOMMUTLBEntry tz_mpc_translate(IOMMUMemoryRegion *iommu,
>      /* Look at the per-block configuration for this address, and
>       * return a TLB entry directing the transaction at either
>       * downstream_as or blocked_io_as, as appropriate.
> -     * For the moment, always permit accesses.
> +     * If the LUT cfg_ns bit is 1, only non-secure transactions
> +     * may pass. If the bit is 0, only secure transactions may pass.
>       */
> -    ok = true;
> +    ok = tz_mpc_cfg_ns(s, addr) == (iommu_idx == IOMMU_IDX_NS);
>  
>      trace_tz_mpc_translate(addr, flags,
>                             iommu_idx == IOMMU_IDX_S ? "S" : "NS",
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 72bf9d57000..c956e1419b7 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -90,6 +90,7 @@ tz_mpc_reg_write(uint32_t offset, uint64_t data, unsigned size) "TZ MPC regs wri
>  tz_mpc_mem_blocked_read(uint64_t addr, unsigned size, bool secure) "TZ MPC blocked read: offset 0x%" PRIx64 " size %u secure %d"
>  tz_mpc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size, bool secure) "TZ MPC blocked write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u secure %d"
>  tz_mpc_translate(uint64_t addr, int flags, const char *idx, const char *res) "TZ MPC translate: addr 0x%" PRIx64 " flags 0x%x iommu_idx %s: %s"
> +tz_mpc_iommu_notify(uint64_t addr) "TZ MPC iommu: notifying UNMAP/MAP for 0x%" PRIx64
>  
>  # hw/misc/tz-ppc.c
>  tz_ppc_reset(void) "TZ PPC: reset"
> 

Re: [Qemu-devel] [PATCH v3 0/8] arm: implement TZ MPC

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180620132032.28865-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v3 0/8] arm: implement TZ MPC

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ff1d3de2bd hw/arm/mps2-tz.c: Instantiate MPCs
5e8ef7e70e hw/arm/iotkit: Wire up MPC interrupt lines
e628d6a2fd hw/arm/iotkit: Instantiate MPC
97fd882413 hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS
b890499e67 hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate
4a900664be hw/misc/tz-mpc.c: Implement correct blocked-access behaviour
db71eee1b0 hw/misc/tz-mpc.c: Implement registers
f702ef6f8e hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller

=== OUTPUT BEGIN ===
Checking PATCH 1/8: hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#85: 
new file mode 100644

total: 0 errors, 1 warnings, 504 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/8: hw/misc/tz-mpc.c: Implement registers...
Checking PATCH 3/8: hw/misc/tz-mpc.c: Implement correct blocked-access behaviour...
Checking PATCH 4/8: hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate...
Checking PATCH 5/8: hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS...
ERROR: spaces required around that '*' (ctx:VxV)
#91: FILE: hw/misc/iotkit-secctl.c:711:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

total: 1 errors, 0 warnings, 100 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/8: hw/arm/iotkit: Instantiate MPC...
Checking PATCH 7/8: hw/arm/iotkit: Wire up MPC interrupt lines...
Checking PATCH 8/8: hw/arm/mps2-tz.c: Instantiate MPCs...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v3 2/8] hw/misc/tz-mpc.c: Implement registers

[Peter Maydell]

On 20 June 2018 at 16:26, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Peter,
>
> On 06/20/2018 03:20 PM, Peter Maydell wrote:
>> Implement the missing registers for the TZ MPC.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> @@ -127,7 +182,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
>>      if (!attrs.secure && offset < A_PIDR4) {
>>          /* NS accesses can only see the ID registers */
> When reading
> "Identification registers can be read by any type of access." I
> understand all others can only be read by secure accesses. So I would
> have expected the same check on read side.

The check is there for reads too, it's in patch 1.

> Eric
>>          qemu_log_mask(LOG_GUEST_ERROR,
>> -                      "TZ MPC register read: NS access to offset 0x%x\n",
>> +                      "TZ MPC register write: NS access to offset 0x%x\n",
>>                        offset);

...this change should have been squashed into patch 2 (I cut-n-pasted
the check from the read function to the write function, forgot
to update the string, noticed that later but put the string fix into
the wrong patch).

Sorry about the confusion.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 0/8] arm: implement TZ MPC

[Peter Maydell]

On 20 June 2018 at 14:20, Peter Maydell <peter.maydell@linaro.org> wrote:
> Hi; this is v3 of my iommu patchset. All the IOMMU stuff is now
> in master, so the remaining part is just implementing and using
> the Trustzone Memory Protection Controller in the mps2-an505.
>
> Changes from v2 to v3 (all fairly minor):
>  * add new variable to clarify sense of LUT bits
>  * only autoinc the IDX register if CTRL.AUTOINC is set
>  * NS accesses should see IDregs only
>    (The datasheet is unclear on the exact behaviour on an
>    NS access to a non-ID register, so I've made a best guess
>    and had them RAZ/WI. This behaviour is not reachable for
>    the mps2-an505 anyway, so it doesn't really matter.)



Applied to target-arm.next, thanks.

-- PMM