[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface |
Date: |
Fri, 9 Oct 2015 01:11:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/08/15 17:02, 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í <address@hidden>
> ---
> hw/arm/virt.c | 2 +-
> hw/nvram/fw_cfg.c | 240
> +++++++++++++++++++++++++++++++++++++++++++---
> include/hw/nvram/fw_cfg.h | 16 +++-
> 3 files changed, 244 insertions(+), 14 deletions(-)
I diffed this against the corresponding patch in v4, and then compared
the result with my v4 comments (and those of Stefan).
Reviewed-by: Laszlo Ersek <address@hidden>
Thanks!
Laszlo
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d25d6cf..7ae984f 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -683,7 +683,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("/address@hidden" PRIx64, base);
> qemu_fdt_add_subnode(vbi->fdt, nodename);
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 658f8c4..d2d1bca 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,7 @@
> #include "qemu/error-report.h"
> #include "qemu/config-file.h"
>
> -#define FW_CFG_SIZE 2
> +#define FW_CFG_CTL_SIZE 2
> #define FW_CFG_NAME "fw_cfg"
> #define FW_CFG_PATH "/machine/" FW_CFG_NAME
>
> @@ -42,6 +43,16 @@
> #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
> +#define FW_CFG_DMA_CTL_SELECT 0x08
> +
> typedef struct FWCfgEntry {
> uint32_t len;
> uint8_t *data;
> @@ -59,6 +70,11 @@ struct FWCfgState {
> uint16_t cur_entry;
> uint32_t cur_offset;
> Notifier machine_ready;
> +
> + bool dma_enabled;
> + dma_addr_t dma_addr;
> + AddressSpace *dma_as;
> + MemoryRegion dma_iomem;
> };
>
> struct FWCfgIoState {
> @@ -67,7 +83,7 @@ struct FWCfgIoState {
> /*< public >*/
>
> MemoryRegion comb_iomem;
> - uint32_t iobase;
> + uint32_t iobase, dma_iobase;
> };
>
> struct FWCfgMemState {
> @@ -292,6 +308,122 @@ 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;
> + FWCfgDmaAccess dma;
> + int arch;
> + FWCfgEntry *e;
> + int read;
> + dma_addr_t dma_addr;
> +
> + /* Reset the address before the next access */
> + dma_addr = s->dma_addr;
> + s->dma_addr = 0;
> +
> + if (dma_memory_read(s->dma_as, dma_addr, &dma, sizeof(dma))) {
> + stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control),
> + FW_CFG_DMA_CTL_ERROR);
> + return;
> + }
> +
> + 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_SELECT) {
> + fw_cfg_select(s, dma.control >> 16);
> + }
> +
> + arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
> + e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
> +
> + if (dma.control & FW_CFG_DMA_CTL_READ) {
> + read = 1;
> + } else if (dma.control & FW_CFG_DMA_CTL_SKIP) {
> + read = 0;
> + } else {
> + dma.length = 0;
> + }
> +
> + dma.control = 0;
> +
> + while (dma.length > 0 && !(dma.control & FW_CFG_DMA_CTL_ERROR)) {
> + 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 (read) {
> + if (dma_memory_set(s->dma_as, dma.address, 0, len)) {
> + dma.control |= FW_CFG_DMA_CTL_ERROR;
> + }
> + }
> +
> + } else {
> + if (dma.length <= (e->len - s->cur_offset)) {
> + len = dma.length;
> + } else {
> + len = (e->len - s->cur_offset);
> + }
> +
> + 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 (read) {
> + if (dma_memory_write(s->dma_as, dma.address,
> + &e->data[s->cur_offset], len)) {
> + dma.control |= FW_CFG_DMA_CTL_ERROR;
> + }
> + }
> +
> + s->cur_offset += len;
> + }
> +
> + dma.address += len;
> + dma.length -= len;
> +
> + }
> +
> + stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control),
> + dma.control);
> +
> + trace_fw_cfg_read(s, 0);
> +}
> +
> +static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
> + uint64_t value, unsigned size)
> +{
> + FWCfgState *s = opaque;
> +
> + if (size == 4) {
> + if (addr == 0) {
> + /* FWCfgDmaAccess high address */
> + s->dma_addr = value << 32;
> + } else if (addr == 4) {
> + /* FWCfgDmaAccess low address */
> + s->dma_addr |= value;
> + fw_cfg_dma_transfer(s);
> + }
> + } else if (size == 8 && addr == 0) {
> + s->dma_addr = value;
> + fw_cfg_dma_transfer(s);
> + }
> +}
> +
> +static bool fw_cfg_dma_mem_valid(void *opaque, hwaddr addr,
> + unsigned size, bool is_write)
> +{
> + return is_write && ((size == 4 && (addr == 0 || addr == 4)) ||
> + (size == 8 && addr == 0));
> +}
> +
> static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
> unsigned size, bool is_write)
> {
> @@ -359,6 +491,14 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
> .valid.accepts = fw_cfg_comb_valid,
> };
>
> +static const MemoryRegionOps fw_cfg_dma_mem_ops = {
> + .write = fw_cfg_dma_mem_write,
> + .endianness = DEVICE_BIG_ENDIAN,
> + .valid.accepts = fw_cfg_dma_mem_valid,
> + .valid.max_access_size = 8,
> + .impl.max_access_size = 8,
> +};
> +
> static void fw_cfg_reset(DeviceState *d)
> {
> FWCfgState *s = FW_CFG(d);
> @@ -399,6 +539,22 @@ static bool is_version_1(void *opaque, int version_id)
> return version_id == 1;
> }
>
> +static bool fw_cfg_dma_enabled(void *opaque)
> +{
> + FWCfgState *s = opaque;
> +
> + return s->dma_enabled;
> +}
> +
> +static VMStateDescription vmstate_fw_cfg_dma = {
> + .name = "fw_cfg/dma",
> + .needed = fw_cfg_dma_enabled,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT64(dma_addr, FWCfgState),
> + VMSTATE_END_OF_LIST()
> + },
> +};
> +
> static const VMStateDescription vmstate_fw_cfg = {
> .name = "fw_cfg",
> .version_id = 2,
> @@ -408,6 +564,10 @@ static const VMStateDescription vmstate_fw_cfg = {
> VMSTATE_UINT16_HACK(cur_offset, FWCfgState, is_version_1),
> VMSTATE_UINT32_V(cur_offset, FWCfgState, 2),
> VMSTATE_END_OF_LIST()
> + },
> + .subsections = (const VMStateDescription*[]) {
> + &vmstate_fw_cfg_dma,
> + NULL,
> }
> };
>
> @@ -593,7 +753,6 @@ static void fw_cfg_init1(DeviceState *dev)
> qdev_init_nofail(dev);
>
> fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> - fw_cfg_add_i32(s, FW_CFG_ID, 1);
> fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type ==
> DT_NOGRAPHIC));
> fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> @@ -605,25 +764,53 @@ static void fw_cfg_init1(DeviceState *dev)
> qemu_add_machine_init_done_notifier(&s->machine_ready);
> }
>
> -FWCfgState *fw_cfg_init_io(uint32_t iobase)
> +FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
> + AddressSpace *dma_as)
> {
> DeviceState *dev;
> + FWCfgState *s;
> + uint32_t version = FW_CFG_VERSION;
> + bool dma_enabled = dma_iobase && dma_as;
>
> dev = qdev_create(NULL, TYPE_FW_CFG_IO);
> qdev_prop_set_uint32(dev, "iobase", iobase);
> + qdev_prop_set_uint32(dev, "dma_iobase", dma_iobase);
> + qdev_prop_set_bit(dev, "dma_enabled", dma_enabled);
> +
> fw_cfg_init1(dev);
> + s = FW_CFG(dev);
> +
> + if (dma_enabled) {
> + /* 64 bits for the address field */
> + s->dma_as = dma_as;
> + s->dma_addr = 0;
> +
> + version |= FW_CFG_VERSION_DMA;
> + }
> +
> + fw_cfg_add_i32(s, FW_CFG_ID, version);
>
> - return FW_CFG(dev);
> + return s;
> +}
> +
> +FWCfgState *fw_cfg_init_io(uint32_t iobase)
> +{
> + return fw_cfg_init_io_dma(iobase, 0, NULL);
> }
>
> -FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, hwaddr data_addr,
> - uint32_t data_width)
> +FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> + hwaddr data_addr, uint32_t data_width,
> + hwaddr dma_addr, AddressSpace *dma_as)
> {
> DeviceState *dev;
> SysBusDevice *sbd;
> + FWCfgState *s;
> + uint32_t version = FW_CFG_VERSION;
> + bool dma_enabled = dma_addr && dma_as;
>
> dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
> qdev_prop_set_uint32(dev, "data_width", data_width);
> + qdev_prop_set_bit(dev, "dma_enabled", dma_enabled);
>
> fw_cfg_init1(dev);
>
> @@ -631,13 +818,25 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> hwaddr data_addr,
> sysbus_mmio_map(sbd, 0, ctl_addr);
> sysbus_mmio_map(sbd, 1, data_addr);
>
> - return FW_CFG(dev);
> + s = FW_CFG(dev);
> +
> + if (dma_enabled) {
> + s->dma_as = dma_as;
> + s->dma_addr = 0;
> + sysbus_mmio_map(sbd, 2, dma_addr);
> + version |= FW_CFG_VERSION_DMA;
> + }
> +
> + fw_cfg_add_i32(s, FW_CFG_ID, version);
> +
> + return s;
> }
>
> FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr data_addr)
> {
> return fw_cfg_init_mem_wide(ctl_addr, data_addr,
> - fw_cfg_data_mem_ops.valid.max_access_size);
> + fw_cfg_data_mem_ops.valid.max_access_size,
> + 0, NULL);
> }
>
>
> @@ -664,6 +863,9 @@ static const TypeInfo fw_cfg_info = {
>
> static Property fw_cfg_io_properties[] = {
> DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1),
> + DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1),
> + DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
> + false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -673,8 +875,15 @@ static void fw_cfg_io_realize(DeviceState *dev, Error
> **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
> memory_region_init_io(&s->comb_iomem, OBJECT(s), &fw_cfg_comb_mem_ops,
> - FW_CFG(s), "fwcfg", FW_CFG_SIZE);
> + FW_CFG(s), "fwcfg", FW_CFG_CTL_SIZE);
> sysbus_add_io(sbd, s->iobase, &s->comb_iomem);
> +
> + if (FW_CFG(s)->dma_enabled) {
> + memory_region_init_io(&FW_CFG(s)->dma_iomem, OBJECT(s),
> + &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
> + sizeof(dma_addr_t));
> + sysbus_add_io(sbd, s->dma_iobase, &FW_CFG(s)->dma_iomem);
> + }
> }
>
> static void fw_cfg_io_class_init(ObjectClass *klass, void *data)
> @@ -695,6 +904,8 @@ static const TypeInfo fw_cfg_io_info = {
>
> static Property fw_cfg_mem_properties[] = {
> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
> + DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
> + false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -705,7 +916,7 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error
> **errp)
> const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops;
>
> memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops,
> - FW_CFG(s), "fwcfg.ctl", FW_CFG_SIZE);
> + FW_CFG(s), "fwcfg.ctl", FW_CFG_CTL_SIZE);
> sysbus_init_mmio(sbd, &s->ctl_iomem);
>
> if (s->data_width > data_ops->valid.max_access_size) {
> @@ -723,6 +934,13 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error
> **errp)
> memory_region_init_io(&s->data_iomem, OBJECT(s), data_ops, FW_CFG(s),
> "fwcfg.data", data_ops->valid.max_access_size);
> sysbus_init_mmio(sbd, &s->data_iomem);
> +
> + if (FW_CFG(s)->dma_enabled) {
> + memory_region_init_io(&FW_CFG(s)->dma_iomem, OBJECT(s),
> + &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
> + sizeof(dma_addr_t));
> + sysbus_init_mmio(sbd, &FW_CFG(s)->dma_iomem);
> + }
> }
>
> static void fw_cfg_mem_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index e60d3ca..ee0cd8a 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -61,6 +61,15 @@ typedef struct FWCfgFiles {
> FWCfgFile f[];
> } FWCfgFiles;
>
> +/* Control as first field allows for different structures selected by this
> + * field, which might be useful in the future
> + */
> +typedef struct FWCfgDmaAccess {
> + uint32_t control;
> + uint32_t length;
> + uint64_t address;
> +} QEMU_PACKED FWCfgDmaAccess;
> +
> typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
> typedef void (*FWCfgReadCallback)(void *opaque, uint32_t offset);
>
> @@ -77,10 +86,13 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char
> *filename,
> void *data, size_t len);
> void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
> size_t len);
> +FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
> + AddressSpace *dma_as);
> FWCfgState *fw_cfg_init_io(uint32_t iobase);
> FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr data_addr);
> -FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, hwaddr data_addr,
> - uint32_t data_width);
> +FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> + hwaddr data_addr, uint32_t data_width,
> + hwaddr dma_addr, AddressSpace *dma_as);
>
> FWCfgState *fw_cfg_find(void);
>
>
- [Qemu-devel] [cross-post] QEMU fw_cfg DMA interface, Marc Marí, 2015/10/08
- [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface, Marc Marí, 2015/10/08
- [Qemu-devel] [PATCH v5 5/6] Enable fw_cfg DMA interface for x86, Marc Marí, 2015/10/08
- [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface, Marc Marí, 2015/10/08
- Re: [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface,
Laszlo Ersek <=
- [Qemu-devel] [PATCH v5 2/6] fw_cfg DMA interface documentation, Marc Marí, 2015/10/08
- Re: [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface, Gerd Hoffmann, 2015/10/09
- [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions, Marc Marí, 2015/10/09
- [Qemu-devel] [PATCH v5 4/6] Enable fw_cfg DMA interface for ARM, Marc Marí, 2015/10/09
- [Qemu-devel] [PATCH v5 6/6] fw_cfg: Define a static signature to be returned on DMA port reads, Marc Marí, 2015/10/09