On 08/09/2018 10:01 AM, Peter Maydell wrote:
> The AN505 FPGA image includes four PL081 DMA controllers, each
> of which is gated by a Master Security Controller that allows
> the guest to prevent a non-secure DMA controller from accessing
> memory that is used by secure guest code. Create and wire
> up these devices.
>
> Signed-off-by: Peter Maydell
> ---
> hw/arm/mps2-tz.c | 101 +++
> 1 file changed, 94 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 22180c56fb7..7d92bc5fe1c 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -45,7 +45,9 @@
> #include "hw/misc/mps2-scc.h"
> #include "hw/misc/mps2-fpgaio.h"
> #include "hw/misc/tz-mpc.h"
> +#include "hw/misc/tz-msc.h"
> #include "hw/arm/iotkit.h"
> +#include "hw/dma/pl080.h"
> #include "hw/devices.h"
> #include "net/net.h"
> #include "hw/core/split-irq.h"
> @@ -75,8 +77,9 @@ typedef struct {
> UnimplementedDeviceState i2c[4];
> UnimplementedDeviceState i2s_audio;
> UnimplementedDeviceState gpio[4];
> -UnimplementedDeviceState dma[4];
> UnimplementedDeviceState gfx;
> +PL080State dma[4];
> +TZMSC msc[4];
> CMSDKAPBUART uart[5];
> SplitIRQ sec_resp_splitter;
> qemu_or_irq uart_irq_orgate;
> @@ -273,6 +276,65 @@ static MemoryRegion *make_mpc(MPS2TZMachineState *mms,
> void *opaque,
> return sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 0);
> }
>
> +static MemoryRegion *make_dma(MPS2TZMachineState *mms, void *opaque,
> + const char *name, hwaddr size)
> +{
> +PL080State *dma = opaque;
> +int i = dma - >dma[0];
This line is not trivial to read. I wondered "isn't this ptrdiff_t? why
not divide by sizeof(dma)...
> +SysBusDevice *s;
> +char *mscname = g_strdup_printf("%s-msc", name);
> +TZMSC *msc = >msc[i];
> +DeviceState *iotkitdev = DEVICE(>iotkit);
> +MemoryRegion *msc_upstream;
> +MemoryRegion *msc_downstream;
> +
> +/*
> + * Each DMA device is a PL081 whose transaction master interface
> + * is guarded by a Master Security Controller. The downstream end of
> + * the MSC connects to the IoTKit AHB Slave Expansion port, so the
> + * DMA devices can see all devices and memory that the CPU does.
> + */
> +init_sysbus_child(OBJECT(mms), mscname, msc, sizeof(mms->msc[0]),
sizeof(*msc) easier to read?
> + TYPE_TZ_MSC);
> +msc_downstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(>iotkit), 0);
> +object_property_set_link(OBJECT(msc), OBJECT(msc_downstream),
> + "downstream", _fatal);
> +object_property_set_link(OBJECT(msc), OBJECT(mms),
> + "idau", _fatal);
> +object_property_set_bool(OBJECT(msc), true, "realized", _fatal);
> +
> +qdev_connect_gpio_out_named(DEVICE(msc), "irq", 0,
> +qdev_get_gpio_in_named(iotkitdev,
> + "mscexp_status", i));
> +qdev_connect_gpio_out_named(iotkitdev, "mscexp_clear", i,
> +qdev_get_gpio_in_named(DEVICE(msc),
> + "irq_clear", 0));
> +qdev_connect_gpio_out_named(iotkitdev, "mscexp_ns", i,
> +qdev_get_gpio_in_named(DEVICE(msc),
> + "cfg_nonsec", 0));
> +qdev_connect_gpio_out(DEVICE(>sec_resp_splitter),
> + ARRAY_SIZE(mms->ppc) + i,
> + qdev_get_gpio_in_named(DEVICE(msc),
> + "cfg_sec_resp", 0));
> +msc_upstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(msc), 0);
> +
> +init_sysbus_child(OBJECT(mms), name, dma, sizeof(mms->dma[0]),
> TYPE_PL081);
> +object_property_set_link(OBJECT(dma), OBJECT(msc_upstream),
> + "downstream", _fatal);
> +object_property_set_bool(OBJECT(dma), true, "realized", _fatal);
> +
> +s = SYS_BUS_DEVICE(dma);
> +/* Wire up DMACINTR, DMACINTERR, DMACINTTC */
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in_named(iotkitdev,
> +"EXP_IRQ", 58 + i * 3));
> +sysbus_connect_irq(s, 1, qdev_get_gpio_in_named(iotkitdev,
> +"EXP_IRQ", 56 + i * 3));
> +sysbus_connect_irq(s, 2, qdev_get_gpio_in_named(iotkitdev,
> +"EXP_IRQ", 57 + i * 3));
> +
> +return sysbus_mmio_get_region(s, 0);
> +}
> +
> static void mps2tz_common_init(MachineState *machine)
> {
> MPS2TZMachineState *mms = MPS2TZ_MACHINE(machine);
> @@ -299,13 +361,14 @@ static void mps2tz_common_init(MachineState *machine)
> _fatal);
>
> /* The sec_resp_cfg output from the IoTKit must be split into multiple
>
