On 18 August 2018 at 02:09, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > 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 <peter.mayd...@linaro.org> >> --- >> 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 - &mms->dma[0]; > > This line is not trivial to read. I wondered "isn't this ptrdiff_t? why > not divide by sizeof(dma)...
It's following the same approach as the other existing make_*() functions in this file, though. >> + SysBusDevice *s; >> + char *mscname = g_strdup_printf("%s-msc", name); >> + TZMSC *msc = &mms->msc[i]; >> + DeviceState *iotkitdev = DEVICE(&mms->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? That would work and probably is better; again I was just following the same thing I'd done in other make_*() functions earlier. thanks -- PMM