On Feb 20, 2017, at 1:06 PM, Laszlo Ersek <lersek@redhat.com> wrote:

On 02/20/17 21:14, ben@skyportsystems.com wrote:
From: Ben Warren <ben@skyportsystems.com>

Due to memory contraints, when resuming from S3 the fw_cfg "files" API
isn't available.  This adds a simple API to get a file 'key', and to
write to the file using the key as a reference.

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

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 4618647..225b08b 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -329,6 +329,17 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
    return file->size;
}

+// Bare-bones function for writing a file knowing only its unique
+// identifying key (select)
+int
+qemu_cfg_write_file_simple(void *src, u16 key, u32 offset, u32 len)
+{
+    qemu_cfg_select(key);
+    qemu_cfg_skip(offset);
+    qemu_cfg_write(src, len);
+    return len;
+}
+
int
qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
{
@@ -339,15 +350,12 @@ qemu_cfg_write_file(void *src, struct romfile_s *file, u32 offset, u32 len)
        warn_internalerror();
        return -1;
    }
-    struct qemu_romfile_s *qfile;
-    qfile = container_of(file, struct qemu_romfile_s, file);
+    u16 key = qemu_get_romfile_key(file);
    if (offset == 0) {
        /* Do it in one transfer */
-        qemu_cfg_write_entry(src, qfile->select, len);
+        qemu_cfg_write_entry(src, key, len);
    } else {
-        qemu_cfg_select(qfile->select);
-        qemu_cfg_skip(offset);
-        qemu_cfg_write(src, len);
+        qemu_cfg_write_file_simple(src, key, offset, len);
    }
    return len;
}

One of the ideas that I mentioned here was to move not just the second
branch of the "if" to qemu_cfg_write_file_simple(), but the entire "if"
-- both branches. Because, qemu_cfg_write_entry() looks suitable for S3
too, and if that kind of optimization makes sense for normal boot, then
it makes sense for S3 resume as well.

Anyway, this is not a functional problem, I won't obsess about it.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Sorry, I misinterpreted your idea.  It makes a lot of sense, so we may as well do it now.  New patch coming.
Thanks
Laszlo


—Ben
<snip>