Re: [Qemu-devel] [PATCH v2 2/5] fw_cfg: remove support for guest-side data writes

[Laszlo Ersek]

On 03/19/15 01:18, Gabriel L. Somlo wrote:
> From this point forward, any guest-side writes to the fw_cfg
> data register will be treated as no-ops. This patch also removes
> the unused host-side API function fw_cfg_add_callback(), which
> allowed the registration of a callback to be executed each time
> the guest completed a full overwrite of a given fw_cfg data item.
> 
> Signed-off-by: Gabriel Somlo <address@hidden>
> ---
>  hw/nvram/fw_cfg.c         | 33 +--------------------------------
>  include/hw/nvram/fw_cfg.h |  2 --
>  trace-events              |  1 -
>  3 files changed, 1 insertion(+), 35 deletions(-)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 78a37be..2f609b4 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;
>  
> @@ -232,19 +231,7 @@ static void fw_cfg_reboot(FWCfgState *s)
>  
>  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;
> -        }
> -    }
> +    /* nothing, write support removed in QEMU v2.4+ */
>  }
>  
>  static int fw_cfg_select(FWCfgState *s, uint16_t key)
> @@ -458,7 +445,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 +488,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,
> diff --git a/trace-events b/trace-events
> index 30eba92..1275b70 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -193,7 +193,6 @@ ecc_diag_mem_writeb(uint64_t addr, uint32_t val) "Write 
> diagnostic %"PRId64" = %
>  ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= 
> %02x"
>  
>  # hw/nvram/fw_cfg.c
> -fw_cfg_write(void *s, uint8_t value) "%p %d"
>  fw_cfg_select(void *s, uint16_t key, int ret) "%p key %d = %d"
>  fw_cfg_read(void *s, uint8_t ret) "%p = %d"
>  fw_cfg_add_file_dupe(void *s, char *name) "%p %s"
> 

Reviewed-by: Laszlo Ersek <address@hidden>