[Qemu-devel] [RFC] Writeable files in fw_cfg

[David Woodhouse]

For OVMF we really want to have a way to store non-volatile variables,
other than the dirty hack that currently puts them on a file in the EFI
system partition.

It looks like we've supported writing to fw_cfg items fairly much since
they were introduced, but we've never actually made use of that.

This WIP patch kills the existing fw_cfg_add_callback() because I can't
see how it would ever be useful, and it doesn't seem to have been used
for years, if ever. And adds something slightly more useful.

Other then the obvious bits that need finishing, any objections?

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 3b31d77..1318a2e 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -33,6 +33,9 @@
 #define FW_CFG_SIZE 2
 #define FW_CFG_DATA_SIZE 1
 
+struct FWCfgEntry;
+typedef void (*FWCfgCallback)(struct FWCfgState *s, struct FWCfgEntry *e, int 
value);
+
 typedef struct FWCfgEntry {
     uint32_t len;
     uint8_t *data;
@@ -206,20 +209,19 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value)
 
     trace_fw_cfg_write(s, value);
 
-    if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback &&
-        s->cur_offset < e->len) {
-        e->data[s->cur_offset++] = value;
-        if (s->cur_offset == e->len) {
-            e->callback(e->callback_opaque, e->data);
-            s->cur_offset = 0;
-        }
-    }
+    if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback)
+        e->callback(s, e, value);
 }
 
 static int fw_cfg_select(FWCfgState *s, uint16_t key)
 {
+    int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
+    FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
     int ret;
 
+    if (e->callback)
+        e->callback(s, e, -1);
+
     s->cur_offset = 0;
     if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) {
         s->cur_entry = FW_CFG_INVALID;
@@ -419,8 +421,8 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t 
value)
     fw_cfg_add_bytes(s, key, copy, sizeof(value));
 }
 
-void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
-                         void *callback_opaque, void *data, size_t len)
+static void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback 
callback,
+                                void *callback_opaque, void *data, size_t len)
 {
     int arch = !!(key & FW_CFG_ARCH_LOCAL);
 
@@ -436,8 +438,9 @@ void fw_cfg_add_callback(FWCfgState *s, uint16_t key, 
FWCfgCallback callback,
     s->entries[arch][key].callback = callback;
 }
 
-void fw_cfg_add_file(FWCfgState *s,  const char *filename,
-                     void *data, size_t len)
+static void fw_cfg_add_file_writeable(FWCfgState *s,  const char *filename,
+                                      void *data, size_t len,
+                                     FWCfgCallback callback, void *cb_opaque)
 {
     int i, index;
     size_t dsize;
@@ -451,7 +454,8 @@ void fw_cfg_add_file(FWCfgState *s,  const char *filename,
     index = be32_to_cpu(s->files->count);
     assert(index < FW_CFG_FILE_SLOTS);
 
-    fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
+    fw_cfg_add_callback(s, FW_CFG_WRITE_CHANNEL + FW_CFG_FILE_FIRST + index,
+                        callback, cb_opaque, data, len);
 
     pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
             filename);
@@ -464,11 +468,74 @@ void fw_cfg_add_file(FWCfgState *s,  const char *filename,
 
     s->files->f[index].size   = cpu_to_be32(len);
     s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
+    if (callback)
+           s->files->f[index].select |= cpu_to_be16(FW_CFG_WRITE_CHANNEL);
     trace_fw_cfg_add_file(s, index, s->files->f[index].name, len);
 
     s->files->count = cpu_to_be32(index+1);
 }
 
+void fw_cfg_add_file(FWCfgState *s,  const char *filename,
+                     void *data, size_t len)
+{
+    fw_cfg_add_file_writeable(s, filename, data, len, NULL, NULL);
+}
+
+// Lifted from pc.c
+static long get_file_size(FILE *f)
+{
+    long where, size;
+
+    /* XXX: on Unix systems, using fstat() probably makes more sense */
+
+    where = ftell(f);
+    fseek(f, 0, SEEK_END);
+    size = ftell(f);
+    fseek(f, where, SEEK_SET);
+
+    return size;
+}
+
+static void nvstorage_callback(struct FWCfgState *s, struct FWCfgEntry *e, int 
value)
+
+{
+    if (value == -1) {
+        FILE *f = fopen(e->callback_opaque, "wb");
+        if (f) {
+            fwrite(e->data, e->len, 1, f);
+            fclose(f);
+        }
+       return;
+    }
+    if (s->cur_offset == e->len)
+        e->data = realloc(e->data, ++e->len);
+    e->data[s->cur_offset++] = value;
+}
+
+void fw_cfg_add_nvstorage(FWCfgState *s, const char *filename)
+{
+    FILE *f;
+    int file_size = 0;
+    int f2;
+    uint8_t *data = NULL;
+
+    f = fopen(filename, "rb");
+    if (f) {
+        file_size = get_file_size(f);
+       if (file_size) {
+            data = malloc(file_size);
+            if ((f2=fread(data, 1, file_size, f)) != file_size) {
+                fprintf(stderr, "qemu: Could not load non-volatile storage 
file '%s' %d %d: %s\n",
+                        filename, file_size, f2, strerror(errno));
+                exit(1);
+            }
+        }
+        fclose(f);
+    }
+    fw_cfg_add_file_writeable(s, "etc/nvstorage", data, file_size,
+                              nvstorage_callback, strdup(filename));
+}
+
 static void fw_cfg_machine_ready(struct Notifier *n, void *data)
 {
     size_t len;
@@ -507,6 +574,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t 
data_port,
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
     fw_cfg_bootsplash(s);
     fw_cfg_reboot(s);
+    fw_cfg_add_nvstorage(s, "/tmp/nvstorage");
 
     s->machine_ready.notify = fw_cfg_machine_ready;
     qemu_add_machine_init_done_notifier(&s->machine_ready);
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 05c8df1..298bc47 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -51,20 +51,17 @@ typedef struct FWCfgFiles {
     FWCfgFile f[];
 } FWCfgFiles;
 
-typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
-
 typedef struct FWCfgState FWCfgState;
 void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
 void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value);
 void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
 void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
 void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
-void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
-                         void *callback_opaque, void *data, size_t len);
 void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
                      size_t len);
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
                         hwaddr crl_addr, hwaddr data_addr);
+void fw_cfg_add_nvstorage(FWCfgState *s, const char *filename);
 
 #endif /* NO_QEMU_PROTOS */
 

-- 
dwmw2

smime.p7s
Description: S/MIME cryptographic signature