Re: [Qemu-devel] fw_cfg specification ?

[Gabriel L. Somlo]

On Thu, Mar 12, 2015 at 04:42:25PM +0100, Laszlo Ersek wrote:
> On 03/12/15 15:17, Paolo Bonzini wrote:
> > Let's nuke it. :)
> 
> Okay... Gabriel, do you want to include such a patch in your upcoming
> series, or should I do the nuking sooner?

I was working on it (good for practice :) and so far I have the patch
below.

Still trying to understand how to update .valid.accepts in the various
mem_ops structures, but I think I'm on the right track. Will send a
proper patch once I wrap my head around that :)

Thanks,
--Gabriel


diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 78a37be..0a9eaf6 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -46,7 +46,6 @@ typedef struct FWCfgEntry {
     uint32_t len;
     uint8_t *data;
     void *callback_opaque;
-    FWCfgCallback callback;
     FWCfgReadCallback read_callback;
 } FWCfgEntry;
 
@@ -230,23 +229,6 @@ static void fw_cfg_reboot(FWCfgState *s)
     fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&reboot_timeout, 4), 4);
 }
 
-static void fw_cfg_write(FWCfgState *s, uint8_t value)
-{
-    int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
-    FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
-
-    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;
-        }
-    }
-}
-
 static int fw_cfg_select(FWCfgState *s, uint16_t key)
 {
     int ret;
@@ -299,17 +281,13 @@ static uint64_t fw_cfg_data_mem_read(void *opaque, hwaddr 
addr,
 static void fw_cfg_data_mem_write(void *opaque, hwaddr addr,
                                   uint64_t value, unsigned size)
 {
-    FWCfgState *s = opaque;
-    unsigned i = size;
-
-    do {
-        fw_cfg_write(s, value >> (8 * --i));
-    } while (i);
+//FIXME: unused, remove from fw_cfg_data_mem_ops
 }
 
 static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
                                   unsigned size, bool is_write)
 {
+//FIXME: additional checks before we nuke fw_cfg_data_mem_write() ?
     return addr == 0;
 }
 
@@ -336,7 +314,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr,
 {
     switch (size) {
     case 1:
-        fw_cfg_write(opaque, (uint8_t)value);
+        //FIXME: unused, fix fw_cfg_comb_mem_ops then remove case
         break;
     case 2:
         fw_cfg_select(opaque, (uint16_t)value);
@@ -347,6 +325,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr,
 static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
                                   unsigned size, bool is_write)
 {
+//FIXME: update checks before removing size==1 fw_cfg_comb_write() case
     return (size == 1) || (is_write && size == 2);
 }
 
@@ -358,6 +337,7 @@ static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
 
 static const MemoryRegionOps fw_cfg_data_mem_ops = {
     .read = fw_cfg_data_mem_read,
+//FIXME: nuke this after updating fw_cfg_data_mem_valid()
     .write = fw_cfg_data_mem_write,
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
@@ -458,7 +438,6 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, 
uint16_t key,
     s->entries[arch][key].data = data;
     s->entries[arch][key].len = len;
     s->entries[arch][key].callback_opaque = NULL;
-    s->entries[arch][key].callback = NULL;
 
     return ptr;
 }
@@ -502,23 +481,6 @@ 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)
-{
-    int arch = !!(key & FW_CFG_ARCH_LOCAL);
-
-    assert(key & FW_CFG_WRITE_CHANNEL);
-
-    key &= FW_CFG_ENTRY_MASK;
-
-    assert(key < FW_CFG_MAX_ENTRY && len <= UINT32_MAX);
-
-    s->entries[arch][key].data = data;
-    s->entries[arch][key].len = (uint32_t)len;
-    s->entries[arch][key].callback_opaque = callback_opaque;
-    s->entries[arch][key].callback = callback;
-}
-
 void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
                               FWCfgReadCallback callback, void 
*callback_opaque,
                               void *data, size_t len)
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 6d8a8ac..b2e10c2 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -69,8 +69,6 @@ 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);
 void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,