On Feb 8, 2017, at 4:13 AM, Laszlo Ersek <lersek@redhat.com> wrote:

On 02/05/17 18:09, ben@skyportsystems.com wrote:
From: Ben Warren <ben@skyportsystems.com>

This allows BIOS to write data back to QEMU using the DMA interface and
provides a higher-level abstraction to write to a fw_cfg file

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
src/fw/paravirt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
src/fw/paravirt.h |  3 +++
2 files changed, 52 insertions(+)

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 6de70f6..75cb992 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -253,6 +253,20 @@ qemu_cfg_read(void *buf, int len)
}

static void
+qemu_cfg_write(void *buf, int len)
+{
+    if (len == 0) {
+        return;
+    }
+
+    if (qemu_cfg_dma_enabled()) {
+        qemu_cfg_dma_transfer(buf, len, QEMU_CFG_DMA_CTL_WRITE);
+    } else {
+        warn_internalerror();
+    }
+}
+
+static void
qemu_cfg_skip(int len)
{
    if (len == 0) {
@@ -280,6 +294,18 @@ qemu_cfg_read_entry(void *buf, int e, int len)
    }
}

+static void
+qemu_cfg_write_entry(void *buf, int e, int len)
+{
+    if (qemu_cfg_dma_enabled()) {
+        u32 control = (e << 16) | QEMU_CFG_DMA_CTL_SELECT
+                        | QEMU_CFG_DMA_CTL_WRITE;
+        qemu_cfg_dma_transfer(buf, len, control);
+    } else {
+        warn_internalerror();
+    }
+}
+
struct qemu_romfile_s {
    struct romfile_s file;
    int select, skip;
@@ -303,6 +329,29 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
    return file->size;
}

+int
+qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
+{
+    if ((file->size + offset) < len)
+        return -1;

I'm confused about this; mathematically, we want to require

 offset + len <= size

If we want to avoid any possibility for overflow in the addition, we can
subtract "len":

 offset <= size - len

However, in this case the subtraction needs to be protected, and we end
up with the following condition to proceed:

 size >= len && offset <= size - len

If we want a condition for bailing out early, we got to negate the above:

 size < len || offset > size - len

The rest looks good to me.

This was dumb.  I meant to do:

if ((offset + len) > file->size) {
    return -1;
}

but got mixed up.  Will fix.
Thanks
Laszlo

+
+    if (!qemu_cfg_dma_enabled() || (file->copy != qemu_cfg_read_file)) {
+        warn_internalerror();
+        return -1;
+    }
+    struct qemu_romfile_s *qfile;
+    qfile = container_of(file, struct qemu_romfile_s, file);
+    if (offset == 0) {
+        /* Do it in one transfer */
+        qemu_cfg_write_entry(src, qfile->select, len);
+    } else {
+        qemu_cfg_select(qfile->select);
+        qemu_cfg_skip(offset);
+        qemu_cfg_write(src, len);
+    }
+    return len;
+}
+
static void
qemu_romfile_add(char *name, int select, int skip, int size)
{
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index d8eb7c4..fb220d8 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -3,6 +3,7 @@

#include "config.h" // CONFIG_*
#include "biosvar.h" // GET_GLOBAL
+#include "romfile.h" // struct romfile_s

// Types of paravirtualized platforms.
#define PF_QEMU     (1<<0)
@@ -43,6 +44,7 @@ static inline int runningOnKVM(void) {
#define QEMU_CFG_DMA_CTL_READ    0x02
#define QEMU_CFG_DMA_CTL_SKIP    0x04
#define QEMU_CFG_DMA_CTL_SELECT  0x08
+#define QEMU_CFG_DMA_CTL_WRITE   0x10

// QEMU_CFG_DMA ID bit
#define QEMU_CFG_VERSION_DMA    2
@@ -53,5 +55,6 @@ void qemu_platform_setup(void);
void qemu_cfg_init(void);

u16 qemu_get_present_cpus_count(void);
+int qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len);

#endif