qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [cross-post] QEMU fw_cfg DMA interface
@ 2015-10-08 15:02 Marc Marí
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: linux-kernel, qemu-devel, seabios
  Cc: Mark Rutland, Rob Herring, Drew, Arnd Bergmann, devicetree,
	Stefan Hajnoczi, Alexander Graf, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

Implementation of the FW CFG DMA interface.

When running a Linux guest on top of QEMU, using the -kernel option and with
fw_cfg DMA Linux boot support, this is the timing improvement for x86:

Original QEMU and SeaBIOS
QEMU startup time: .080
BIOS startup time: .060
Kernel setup time: .586
Total time: .726

QEMU and SeaBIOS with this patch series and fw_cfg DMA Linux boot support
QEMU startup time: .080
BIOS startup time: .039
Kernel setup time: .005
Total time: .126

QEMU startup time is the time between the start and the first kvm_entry.
BIOS startup time is the time between the first kvm_entry and the start of
function do_boot, in SeaBIOS.
Kernel setup time is the time between the start of the function do_boot in
SeaBIOS and the jump to the Linux kernel.

As you can see, both the BIOS (because of ACPI tables and other configurations)
and the Linux kernel boot (because of the copy to memory) are greatly
improved with this new interface.

Also, this new interface is an addon to the old interface. Both interfaces
are compatible and interchangeable.

Changes from v1:
 - Take into account order of fields in the FWCfgDmaAccess structure
 - Check and change endianness of FWCfgDmaAccess fields
 - Change order of fields in the FWCfgDmaAccess structure
 - Add FW_CFG_DMA_CTL_SKIP feature for control field
 - Split FW_CFG_SIZE in QEMU
 - Make FW_CFG_ID a bitmap of features
 - Add 64 bit address support for the transfer. Trigger when writing the low
   address, and address is 0 by default and at the end of each transfer.
 - Align ports and addresses.
 - Preserve old fw_cfg_comb_valid behaviour in QEMU
 - Update documentation to reflect all these changes

Changes from v2:
 - Make IOports fw_cfg DMA region a different IO region.
 - Reuse everything for MMIO and IOport DMA regions
 - Make transfer status only based on control field
 - Use DMA helpers instead of direct map/unmap
 - Change ARM fw_cfg DMA address space
 - Change Linux boot process to match linuxboot.S
 - Add select capabilities in the FWCfgDmaAccess struct
 - Update documentation to reflect all these changes

Changes from v3:
 - Set properly fw_cfg DMA fields in ARM
 - Set fw_cfg DMA boot process properly (by Laszlo Ersek)
 - Add signature to fw_cfg DMA address field (by Kevin O'Connor)
 - Minor nitpicks

Changes from v4:
 - Remove Linux fw_cfg boot from this series (will be sent separately)
 - Minor nitpicks

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface
  2015-10-08 15:02 [Qemu-devel] [cross-post] QEMU fw_cfg DMA interface Marc Marí
@ 2015-10-08 15:02 ` Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions Marc Marí
                     ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

Implement host-side of the FW CFG DMA interface both for x86 and ARM.

Based on Gerd Hoffman's initial implementation.

Gabriel L. Somlo (1):
  fw_cfg: document fw_cfg_modify_iXX() update functions

Kevin O'Connor (1):
  fw_cfg: Define a static signature to be returned on DMA port reads

Marc Marí (4):
  fw_cfg DMA interface documentation
  Implement fw_cfg DMA interface
  Enable fw_cfg DMA interface for ARM
  Enable fw_cfg DMA interface for x86

 docs/specs/fw_cfg.txt     |  79 ++++++++++++++-
 hw/arm/virt.c             |   8 +-
 hw/i386/pc.c              |   8 +-
 hw/nvram/fw_cfg.c         | 250 ++++++++++++++++++++++++++++++++++++++++++++--
 include/hw/nvram/fw_cfg.h |  16 ++-
 5 files changed, 337 insertions(+), 24 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 2/6] fw_cfg DMA interface documentation Marc Marí
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Laszlo

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Document the behavior of fw_cfg_modify_iXX() for leak-less updating
of integer-type blobs.

Currently only fw_cfg_modify_i16() is coded, but 32- and 64-bit versions
may be added later if necessary..

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 docs/specs/fw_cfg.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 74351dd..5bc7b96 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -159,6 +159,17 @@ will convert a 16-, 32-, or 64-bit integer to little-endian, then add
 a dynamically allocated copy of the appropriately sized item to fw_cfg
 under the given selector key value.
 
+== fw_cfg_modify_iXX() ==
+
+Modify the value of an XX-bit item (where XX may be 16, 32, or 64).
+Similarly to the corresponding fw_cfg_add_iXX() function set, convert
+a 16-, 32-, or 64-bit integer to little endian, create a dynamically
+allocated copy of the required size, and replace the existing item at
+the given selector key value with the newly allocated one. The previous
+item, assumed to have been allocated during an earlier call to
+fw_cfg_add_iXX() or fw_cfg_modify_iXX() (of the same width XX), is freed
+before the function returns.
+
 == fw_cfg_add_file() ==
 
 Given a filename (i.e., fw_cfg item name), starting pointer, and size,
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 2/6] fw_cfg DMA interface documentation
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface Marc Marí
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

Add fw_cfg DMA interface specification in the documentation.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí <markmb@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 docs/specs/fw_cfg.txt | 65 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 5bc7b96..f217d85 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -76,6 +76,13 @@ increasing address order, similar to memcpy().
 
 Selector Register IOport: 0x510
 Data Register IOport:     0x511
+DMA Address IOport:       0x514
+
+=== ARM Register Locations ===
+
+Selector Register address: Base + 8 (2 bytes)
+Data Register address:     Base + 0 (8 bytes)
+DMA Address address:       Base + 16 (8 bytes)
 
 == Firmware Configuration Items ==
 
@@ -86,11 +93,12 @@ by selecting the "signature" item using key 0x0000 (FW_CFG_SIGNATURE),
 and reading four bytes from the data register. If the fw_cfg device is
 present, the four bytes read will contain the characters "QEMU".
 
-=== Revision (Key 0x0001, FW_CFG_ID) ===
+=== Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
 
-A 32-bit little-endian unsigned int, this item is used as an interface
-revision number, and is currently set to 1 by QEMU when fw_cfg is
-initialized.
+A 32-bit little-endian unsigned int, this item is used to check for enabled
+features.
+ - Bit 0: traditional interface. Always set.
+ - Bit 1: DMA interface.
 
 === File Directory (Key 0x0019, FW_CFG_FILE_DIR) ===
 
@@ -132,6 +140,55 @@ Selector Reg.    Range Usage
 In practice, the number of allowed firmware configuration items is given
 by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h).
 
+= Guest-side DMA Interface =
+
+If bit 1 of the feature bitmap is set, the DMA interface is present. This does
+not replace the existing fw_cfg interface, it is an add-on. This interface
+can be used through the 64-bit wide address register.
+
+The address register is in big-endian format. The value for the register is 0
+at startup and after an operation. A write to the least significant half (at
+offset 4) triggers an operation. This means that operations with 32-bit
+addresses can be triggered with just one write, whereas operations with
+64-bit addresses can be triggered with one 64-bit write or two 32-bit writes,
+starting with the most significant half (at offset 0).
+
+In this register, the physical address of a FWCfgDmaAccess structure in RAM
+should be written. This is the format of the FWCfgDmaAccess structure:
+
+typedef struct FWCfgDmaAccess {
+    uint32_t control;
+    uint32_t length;
+    uint64_t address;
+} FWCfgDmaAccess;
+
+The fields of the structure are in big endian mode, and the field at the lowest
+address is the "control" field.
+
+The "control" field has the following bits:
+ - Bit 0: Error
+ - Bit 1: Read
+ - Bit 2: Skip
+ - Bit 3: Select. The upper 16 bits are the selected index.
+
+When an operation is triggered, if the "control" field has bit 3 set, the
+upper 16 bits are interpreted as an index of a firmware configuration item.
+This has the same effect as writing the selector register.
+
+If the "control" field has bit 1 set, a read operation will be performed.
+"length" bytes for the current selector and offset will be copied into the
+physical RAM address specified by the "address" field.
+
+If the "control" field has bit 2 set (and not bit 1), a skip operation will be
+performed. The offset for the current selector will be advanced "length" bytes.
+
+To check the result, read the "control" field:
+   error bit set        ->  something went wrong.
+   all bits cleared     ->  transfer finished successfully.
+   otherwise            ->  transfer still in progress (doesn't happen
+                            today due to implementation not being async,
+                            but may in the future).
+
 = Host-side API =
 
 The following functions are available to the QEMU programmer for adding
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 2/6] fw_cfg DMA interface documentation Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-08 23:11     ` Laszlo Ersek
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 4/6] Enable fw_cfg DMA interface for ARM Marc Marí
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

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í <markmb@redhat.com>
---
 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(-)

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("/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 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);
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 4/6] Enable fw_cfg DMA interface for ARM
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
                     ` (2 preceding siblings ...)
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 5/6] Enable fw_cfg DMA interface for x86 Marc Marí
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

Enable the fw_cfg DMA interface for the ARM virt machine.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí <markmb@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/arm/virt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7ae984f..0a39087 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -119,7 +119,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_GIC_REDIST] =         { 0x080A0000, 0x00F60000 },
     [VIRT_UART] =               { 0x09000000, 0x00001000 },
     [VIRT_RTC] =                { 0x09010000, 0x00001000 },
-    [VIRT_FW_CFG] =             { 0x09020000, 0x0000000a },
+    [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -677,13 +677,13 @@ static void create_flash(const VirtBoardInfo *vbi)
     g_free(nodename);
 }
 
-static void create_fw_cfg(const VirtBoardInfo *vbi)
+static void create_fw_cfg(const VirtBoardInfo *vbi, AddressSpace *as)
 {
     hwaddr base = vbi->memmap[VIRT_FW_CFG].base;
     hwaddr size = vbi->memmap[VIRT_FW_CFG].size;
     char *nodename;
 
-    fw_cfg_init_mem_wide(base + 8, base, 8, 0, NULL);
+    fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
 
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
     qemu_fdt_add_subnode(vbi->fdt, nodename);
@@ -1026,7 +1026,7 @@ static void machvirt_init(MachineState *machine)
      */
     create_virtio_devices(vbi, pic);
 
-    create_fw_cfg(vbi);
+    create_fw_cfg(vbi, &address_space_memory);
     rom_set_fw(fw_cfg_find());
 
     guest_info->smp_cpus = smp_cpus;
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 5/6] Enable fw_cfg DMA interface for x86
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
                     ` (3 preceding siblings ...)
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 4/6] Enable fw_cfg DMA interface for ARM Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 6/6] fw_cfg: Define a static signature to be returned on DMA port reads Marc Marí
  2015-10-09  8:54   ` [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface Gerd Hoffmann
  6 siblings, 0 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Marc Marí, Laszlo

Enable the fw_cfg DMA interface for all the x86 platforms.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí <markmb@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/i386/pc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9275297..da27553 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -752,14 +752,15 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
     }
 }
 
-static FWCfgState *bochs_bios_init(void)
+static FWCfgState *bochs_bios_init(AddressSpace *as)
 {
     FWCfgState *fw_cfg;
     uint64_t *numa_fw_cfg;
     int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
+
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
      *
      * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
@@ -1389,7 +1390,8 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
                                         option_rom_mr,
                                         1);
 
-    fw_cfg = bochs_bios_init();
+    fw_cfg = bochs_bios_init(&address_space_memory);
+
     rom_set_fw(fw_cfg);
 
     if (guest_info->has_reserved_memory && pcms->hotplug_memory.base) {
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 6/6] fw_cfg: Define a static signature to be returned on DMA port reads
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
                     ` (4 preceding siblings ...)
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 5/6] Enable fw_cfg DMA interface for x86 Marc Marí
@ 2015-10-08 15:02   ` Marc Marí
  2015-10-09  8:54   ` [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface Gerd Hoffmann
  6 siblings, 0 replies; 10+ messages in thread
From: Marc Marí @ 2015-10-08 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, Kevin O'Connor,
	Gerd Hoffmann, Laszlo

From: Kevin O'Connor <kevin@koconnor.net>

Return a static signature ("QEMU CFG") if the guest does a read to the
DMA address io register.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/specs/fw_cfg.txt |  3 +++
 hw/nvram/fw_cfg.c     | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index f217d85..274b53c 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -93,6 +93,9 @@ by selecting the "signature" item using key 0x0000 (FW_CFG_SIGNATURE),
 and reading four bytes from the data register. If the fw_cfg device is
 present, the four bytes read will contain the characters "QEMU".
 
+If the DMA interface is available, then reading the DMA Address
+Register returns 0x51454d5520434647 ("QEMU CFG" in big-endian format).
+
 === Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
 
 A 32-bit little-endian unsigned int, this item is used to check for enabled
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d2d1bca..309490c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -53,6 +53,8 @@
 #define FW_CFG_DMA_CTL_SKIP    0x04
 #define FW_CFG_DMA_CTL_SELECT  0x08
 
+#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
+
 typedef struct FWCfgEntry {
     uint32_t len;
     uint8_t *data;
@@ -397,6 +399,13 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
     trace_fw_cfg_read(s, 0);
 }
 
+static uint64_t fw_cfg_dma_mem_read(void *opaque, hwaddr addr,
+                                    unsigned size)
+{
+    /* Return a signature value (and handle various read sizes) */
+    return extract64(FW_CFG_DMA_SIGNATURE, (8 - addr - size) * 8, size * 8);
+}
+
 static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
                                  uint64_t value, unsigned size)
 {
@@ -420,8 +429,8 @@ static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
 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));
+    return !is_write || ((size == 4 && (addr == 0 || addr == 4)) ||
+                         (size == 8 && addr == 0));
 }
 
 static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
@@ -492,6 +501,7 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 };
 
 static const MemoryRegionOps fw_cfg_dma_mem_ops = {
+    .read = fw_cfg_dma_mem_read,
     .write = fw_cfg_dma_mem_write,
     .endianness = DEVICE_BIG_ENDIAN,
     .valid.accepts = fw_cfg_dma_mem_valid,
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface Marc Marí
@ 2015-10-08 23:11     ` Laszlo Ersek
  0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2015-10-08 23:11 UTC (permalink / raw)
  To: Marc Marí, qemu-devel
  Cc: Stefan Hajnoczi, Drew, Kevin O'Connor, Gabriel L. Somlo,
	Gerd Hoffmann

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í <markmb@redhat.com>
> ---
>  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 <lersek@redhat.com>

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("/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 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);
>  
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface
  2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
                     ` (5 preceding siblings ...)
  2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 6/6] fw_cfg: Define a static signature to be returned on DMA port reads Marc Marí
@ 2015-10-09  8:54   ` Gerd Hoffmann
  6 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2015-10-09  8:54 UTC (permalink / raw)
  To: Marc Marí
  Cc: Drew, Stefan Hajnoczi, Gabriel L. Somlo, qemu-devel,
	Kevin O'Connor, Laszlo

On Do, 2015-10-08 at 17:02 +0200, Marc Marí wrote:
> Implement host-side of the FW CFG DMA interface both for x86 and ARM.
> 
> Based on Gerd Hoffman's initial implementation.
> 
> Gabriel L. Somlo (1):
>   fw_cfg: document fw_cfg_modify_iXX() update functions
> 
> Kevin O'Connor (1):
>   fw_cfg: Define a static signature to be returned on DMA port reads
> 
> Marc Marí (4):
>   fw_cfg DMA interface documentation
>   Implement fw_cfg DMA interface
>   Enable fw_cfg DMA interface for ARM
>   Enable fw_cfg DMA interface for x86

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-10-09  8:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 15:02 [Qemu-devel] [cross-post] QEMU fw_cfg DMA interface Marc Marí
2015-10-08 15:02 ` [Qemu-devel] [PATCH v5 0/6] " Marc Marí
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 1/6] fw_cfg: document fw_cfg_modify_iXX() update functions Marc Marí
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 2/6] fw_cfg DMA interface documentation Marc Marí
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 3/6] Implement fw_cfg DMA interface Marc Marí
2015-10-08 23:11     ` Laszlo Ersek
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 4/6] Enable fw_cfg DMA interface for ARM Marc Marí
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 5/6] Enable fw_cfg DMA interface for x86 Marc Marí
2015-10-08 15:02   ` [Qemu-devel] [PATCH v5 6/6] fw_cfg: Define a static signature to be returned on DMA port reads Marc Marí
2015-10-09  8:54   ` [Qemu-devel] [PATCH v5 0/6] fw_cfg DMA interface Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).