On Mon, Aug 31, 2015 at 11:10:15AM +0200, Marc Marí wrote: > Based on the specifications on docs/specs/fw_cfg.txt > > This interface is an addon. The old interface can still be used as usual. > > Based on Gerd Hoffman's initial implementation. > > Signed-off-by: Marc Marí <mar...@redhat.com> > --- > hw/arm/virt.c | 2 +- > hw/nvram/fw_cfg.c | 261 > +++++++++++++++++++++++++++++++++++++++++++--- > include/hw/nvram/fw_cfg.h | 15 ++- > 3 files changed, 260 insertions(+), 18 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index d5a8417..b88c104 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -620,7 +620,7 @@ static void create_fw_cfg(const VirtBoardInfo *vbi) > hwaddr size = vbi->memmap[VIRT_FW_CFG].size; > char *nodename; > > - fw_cfg_init_mem_wide(base + 8, base, 8); > + fw_cfg_init_mem_wide(base + 8, base, 8, 0, NULL); > > nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base); > qemu_fdt_add_subnode(vbi->fdt, nodename); > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 88481b7..7e5ba96 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -23,6 +23,7 @@ > */ > #include "hw/hw.h" > #include "sysemu/sysemu.h" > +#include "sysemu/dma.h" > #include "hw/isa/isa.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/sysbus.h" > @@ -30,7 +31,8 @@ > #include "qemu/error-report.h" > #include "qemu/config-file.h" > > -#define FW_CFG_SIZE 2 > +#define FW_CFG_IO_SIZE 12 /* Address aligned to 4 bytes */ > +#define FW_CFG_CTL_SIZE 2 > #define FW_CFG_NAME "fw_cfg" > #define FW_CFG_PATH "/machine/" FW_CFG_NAME > > @@ -42,6 +44,15 @@ > #define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) > #define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) > > +/* FW_CFG_VERSION bits */ > +#define FW_CFG_VERSION 0x01 > +#define FW_CFG_VERSION_DMA 0x02 > + > +/* FW_CFG_DMA_CONTROL bits */ > +#define FW_CFG_DMA_CTL_ERROR 0x01 > +#define FW_CFG_DMA_CTL_READ 0x02 > +#define FW_CFG_DMA_CTL_SKIP 0x04 > + > typedef struct FWCfgEntry { > uint32_t len; > uint8_t *data; > @@ -59,6 +70,10 @@ struct FWCfgState { > uint16_t cur_entry; > uint32_t cur_offset; > Notifier machine_ready; > + > + bool dma_enabled; > + AddressSpace *dma_as; > + dma_addr_t dma_addr; > }; > > struct FWCfgIoState { > @@ -75,7 +90,7 @@ struct FWCfgMemState { > FWCfgState parent_obj; > /*< public >*/ > > - MemoryRegion ctl_iomem, data_iomem; > + MemoryRegion ctl_iomem, data_iomem, dma_iomem; > uint32_t data_width; > MemoryRegionOps wide_data_ops; > }; > @@ -294,6 +309,142 @@ static void fw_cfg_data_mem_write(void *opaque, hwaddr > addr, > } while (i); > } > > +static void fw_cfg_dma_transfer(FWCfgState *s) > +{ > + dma_addr_t len; > + uint8_t *ptr; > + void *addr; > + FWCfgDmaAccess dma; > + int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > + FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > + > + len = sizeof(dma); > + addr = dma_memory_map(s->dma_as, s->dma_addr, &len, > + DMA_DIRECTION_FROM_DEVICE); > + > + s->dma_addr = 0; > + > + if (!addr || !len) { > + return; > + } > + > + dma.address = be64_to_cpu(*(uint64_t *)(addr + > + offsetof(FWCfgDmaAccess, address))); > + dma.length = be32_to_cpu(*(uint32_t *)(addr + > + offsetof(FWCfgDmaAccess, length))); > + dma.control = be32_to_cpu(*(uint32_t *)(addr + > + offsetof(FWCfgDmaAccess, control)));
I am not that familiar with QEMU, but shouldn't that be DMA_DIRECTION_TO_DEVICE? It looks like other drivers use dma_memory_read() which would simplify this code: dma_memory_read(s->dma_as, s->dma_addr, &dma, sizeof(dma)); dma.address = be64_to_cpu(dma.address); dma.length = be32_to_cpu(dma.length); dma.control = be32_to_cpu(dma.control); > + if (dma.control & FW_CFG_DMA_CTL_ERROR) { > + goto out; > + } > + > + if (!(dma.control & (FW_CFG_DMA_CTL_READ | FW_CFG_DMA_CTL_SKIP))) { > + goto out; > + } > + > + while (dma.length > 0) { > + if (s->cur_entry == FW_CFG_INVALID || !e->data || > + s->cur_offset >= e->len) { > + len = dma.length; > + > + /* If the access is not a read access, it will be a skip access, > + * tested before. > + */ > + if (dma.control & FW_CFG_DMA_CTL_READ) { > + ptr = dma_memory_map(s->dma_as, dma.address, &len, > + DMA_DIRECTION_FROM_DEVICE); > + if (!ptr || !len) { > + dma.control |= FW_CFG_DMA_CTL_ERROR; > + goto out; > + } > + > + memset(ptr, 0, len); > + > + dma_memory_unmap(s->dma_as, ptr, len, > + DMA_DIRECTION_FROM_DEVICE, len); > + } > + > + } else { > + if (dma.length <= e->len) { > + len = dma.length; > + } else { > + len = e->len; > + } > + > + if (e->read_callback) { > + e->read_callback(e->callback_opaque, s->cur_offset); > + } > + > + /* If the access is not a read access, it will be a skip access, > + * tested before. > + */ > + if (dma.control & FW_CFG_DMA_CTL_READ) { > + ptr = dma_memory_map(s->dma_as, dma.address, &len, > + DMA_DIRECTION_FROM_DEVICE); > + if (!ptr || !len) { > + dma.control |= FW_CFG_DMA_CTL_ERROR; > + goto out; > + } > + > + memcpy(ptr, &e->data[s->cur_offset], len); > + > + dma_memory_unmap(s->dma_as, ptr, len, > + DMA_DIRECTION_FROM_DEVICE, len); > + } > + > + s->cur_offset += len; > + > + } > + > + dma.address += len; > + dma.length -= len; > + dma.control = 0; > + > + *(uint64_t *)(addr + offsetof(FWCfgDmaAccess, address)) = > + cpu_to_be64(dma.address); > + *(uint32_t *)(addr + offsetof(FWCfgDmaAccess, length)) = > + cpu_to_be32(dma.length); > + *(uint32_t *)(addr + offsetof(FWCfgDmaAccess, control)) = > + cpu_to_be32(dma.control); > + } I don't think it makes sense for this update to be performed within the loop. As I mentioned in another email, I think just updating control would be sufficient. Looks like include/sysemu/dma.h defines a stl_be_dma() macro for performing a single 32bit dma write. Thanks, -Kevin