Hi Philippe, Nice improvement, I didnt know about this API. Makes sense to use it indeed. The patch works fine. I tested your patches by applying the previous two sets first, and then this one. It ran well with the avocado tests and also with the official image OrangePi_pc_debian_stretch_server_linux5.3.5_v1.0.img.
Tested-by: Niek Linnenbank <[email protected]> Reviewed-by: Niek Linnenbank <[email protected]> Regards, Niek On Fri, Aug 14, 2020 at 1:01 PM Philippe Mathieu-Daudé <[email protected]> wrote: > Allow the device to execute the DMA transfers in a different > AddressSpace. > > The A10 and H3 SoC keep using the system_memory address space, > but via the proper dma_memory_access() API. > > Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > --- > Based-on: <[email protected]> > "hw/sd: Use sdbus_read_data/sdbus_write_data for multiple bytes access" > > Tested with: > AVOCADO_ALLOW_LARGE_STORAGE=1 avocado run -t machine:orangepi-pc -t > machine:cubieboard tests/acceptance/ > --- > include/hw/sd/allwinner-sdhost.h | 6 ++++++ > hw/arm/allwinner-a10.c | 2 ++ > hw/arm/allwinner-h3.c | 2 ++ > hw/sd/allwinner-sdhost.c | 37 ++++++++++++++++++++++++++------ > 4 files changed, 41 insertions(+), 6 deletions(-) > > diff --git a/include/hw/sd/allwinner-sdhost.h > b/include/hw/sd/allwinner-sdhost.h > index d94606a853..839732ebf3 100644 > --- a/include/hw/sd/allwinner-sdhost.h > +++ b/include/hw/sd/allwinner-sdhost.h > @@ -71,6 +71,12 @@ typedef struct AwSdHostState { > /** Interrupt output signal to notify CPU */ > qemu_irq irq; > > + /** Memory region where DMA transfers are done */ > + MemoryRegion *dma_mr; > + > + /** Address space used internally for DMA transfers */ > + AddressSpace dma_as; > + > /** Number of bytes left in current DMA transfer */ > uint32_t transfer_cnt; > > diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c > index e258463747..d404f31e02 100644 > --- a/hw/arm/allwinner-a10.c > +++ b/hw/arm/allwinner-a10.c > @@ -155,6 +155,8 @@ static void aw_a10_realize(DeviceState *dev, Error > **errp) > } > > /* SD/MMC */ > + object_property_set_link(OBJECT(&s->mmc0), "dma-memory", > + OBJECT(get_system_memory()), &error_fatal); > sysbus_realize(SYS_BUS_DEVICE(&s->mmc0), &error_fatal); > sysbus_mmio_map(SYS_BUS_DEVICE(&s->mmc0), 0, AW_A10_MMC0_BASE); > sysbus_connect_irq(SYS_BUS_DEVICE(&s->mmc0), 0, qdev_get_gpio_in(dev, > 32)); > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > index ff92ded82c..43a8d3dc48 100644 > --- a/hw/arm/allwinner-h3.c > +++ b/hw/arm/allwinner-h3.c > @@ -349,6 +349,8 @@ static void allwinner_h3_realize(DeviceState *dev, > Error **errp) > sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0, s->memmap[AW_H3_SID]); > > /* SD/MMC */ > + object_property_set_link(OBJECT(&s->mmc0), "dma-memory", > + OBJECT(get_system_memory()), &error_fatal); > sysbus_realize(SYS_BUS_DEVICE(&s->mmc0), &error_fatal); > sysbus_mmio_map(SYS_BUS_DEVICE(&s->mmc0), 0, s->memmap[AW_H3_MMC0]); > sysbus_connect_irq(SYS_BUS_DEVICE(&s->mmc0), 0, > diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c > index f9eb92c09e..e82afb75eb 100644 > --- a/hw/sd/allwinner-sdhost.c > +++ b/hw/sd/allwinner-sdhost.c > @@ -21,7 +21,10 @@ > #include "qemu/log.h" > #include "qemu/module.h" > #include "qemu/units.h" > +#include "qapi/error.h" > #include "sysemu/blockdev.h" > +#include "sysemu/dma.h" > +#include "hw/qdev-properties.h" > #include "hw/irq.h" > #include "hw/sd/allwinner-sdhost.h" > #include "migration/vmstate.h" > @@ -306,7 +309,7 @@ static uint32_t > allwinner_sdhost_process_desc(AwSdHostState *s, > uint8_t buf[1024]; > > /* Read descriptor */ > - cpu_physical_memory_read(desc_addr, desc, sizeof(*desc)); > + dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc)); > if (desc->size == 0) { > desc->size = klass->max_desc_size; > } else if (desc->size > klass->max_desc_size) { > @@ -331,22 +334,24 @@ static uint32_t > allwinner_sdhost_process_desc(AwSdHostState *s, > > /* Write to SD bus */ > if (is_write) { > - cpu_physical_memory_read((desc->addr & DESC_SIZE_MASK) + > num_done, > - buf, buf_bytes); > + dma_memory_read(&s->dma_as, > + (desc->addr & DESC_SIZE_MASK) + num_done, > + buf, buf_bytes); > sdbus_write_data(&s->sdbus, buf, buf_bytes); > > /* Read from SD bus */ > } else { > sdbus_read_data(&s->sdbus, buf, buf_bytes); > - cpu_physical_memory_write((desc->addr & DESC_SIZE_MASK) + > num_done, > - buf, buf_bytes); > + dma_memory_write(&s->dma_as, > + (desc->addr & DESC_SIZE_MASK) + num_done, > + buf, buf_bytes); > } > num_done += buf_bytes; > } > > /* Clear hold flag and flush descriptor */ > desc->status &= ~DESC_STATUS_HOLD; > - cpu_physical_memory_write(desc_addr, desc, sizeof(*desc)); > + dma_memory_write(&s->dma_as, desc_addr, desc, sizeof(*desc)); > > return num_done; > } > @@ -721,6 +726,12 @@ static const VMStateDescription > vmstate_allwinner_sdhost = { > } > }; > > +static Property allwinner_sdhost_properties[] = { > + DEFINE_PROP_LINK("dma-memory", AwSdHostState, dma_mr, > + TYPE_MEMORY_REGION, MemoryRegion *), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void allwinner_sdhost_init(Object *obj) > { > AwSdHostState *s = AW_SDHOST(obj); > @@ -734,6 +745,18 @@ static void allwinner_sdhost_init(Object *obj) > sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq); > } > > +static void allwinner_sdhost_realize(DeviceState *dev, Error **errp) > +{ > + AwSdHostState *s = AW_SDHOST(dev); > + > + if (!s->dma_mr) { > + error_setg(errp, TYPE_AW_SDHOST " 'dma-memory' link not set"); > + return; > + } > + > + address_space_init(&s->dma_as, s->dma_mr, "sdhost-dma"); > +} > + > static void allwinner_sdhost_reset(DeviceState *dev) > { > AwSdHostState *s = AW_SDHOST(dev); > @@ -792,6 +815,8 @@ static void allwinner_sdhost_class_init(ObjectClass > *klass, void *data) > > dc->reset = allwinner_sdhost_reset; > dc->vmsd = &vmstate_allwinner_sdhost; > + dc->realize = allwinner_sdhost_realize; > + device_class_set_props(dc, allwinner_sdhost_properties); > } > > static void allwinner_sdhost_sun4i_class_init(ObjectClass *klass, void > *data) > -- > 2.21.3 > > -- Niek Linnenbank
