Re: [Qemu-devel] [PATCH 1/3] loader: add support for resizeable blobs

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (address@hidden) wrote:
> Support resizeable blobs: we allocate more memory than currently
> available in the blob, which can later be filled in.
> 
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  include/hw/loader.h       | 14 +++++++--
>  include/hw/nvram/fw_cfg.h |  2 +-
>  hw/core/loader.c          | 15 +++++----
>  hw/nvram/fw_cfg.c         | 79 
> ++++++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 96 insertions(+), 14 deletions(-)
> 


> +static bool fw_cfg_len_needed(void *opaque, int version_id)
> +{
> +    FWCfgState *s = FW_CFG(opaque);
> +    int i, j;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->entries); ++i) {
> +        for (j = 0; j < ARRAY_SIZE(s->entries[0]); ++j) {
> +            if (s->entries[i][j].len != s->entries[i][j].max_len) {
> +                return true;
> +            }
> +        }
> +    }
> +
> +    return false;
> +}

This feels a bit delicate; it means that increasing the 'max_len' changes
the expected migration stream, which seems odd for a parameter whose
job is to make things easier to migrate.

Dave

> +
>  static const VMStateDescription vmstate_fw_cfg = {
>      .name = "fw_cfg",
>      .version_id = 2,
>      .minimum_version_id = 1,
> +    .post_load = fw_cfg_post_load,
> +    .pre_save = fw_cfg_pre_save,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT16(cur_entry, FWCfgState),
>          VMSTATE_UINT16_HACK(cur_offset, FWCfgState, is_version_1),
>          VMSTATE_UINT32_V(cur_offset, FWCfgState, 2),
> +        VMSTATE_ARRAY_TEST(len, FWCfgState, FW_CFG_LEN_ENTRIES,
> +                           fw_cfg_len_needed,
> +                           vmstate_info_uint32, uint32_t),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -388,23 +449,28 @@ static const VMStateDescription vmstate_fw_cfg = {
>  static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key,
>                                             FWCfgReadCallback callback,
>                                             void *callback_opaque,
> -                                           void *data, size_t len)
> +                                           void *data, size_t len,
> +                                           size_t max_len)
>  {
>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>  
> +    assert(len <= max_len);
> +
>      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].reset_len = (uint32_t)len;
> +    s->entries[arch][key].max_len = (uint32_t)max_len;
>      s->entries[arch][key].read_callback = callback;
>      s->entries[arch][key].callback_opaque = callback_opaque;
>  }
>  
>  void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
>  {
> -    fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len);
> +    fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len, len);
>  }
>  
>  void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value)
> @@ -454,13 +520,15 @@ void fw_cfg_add_callback(FWCfgState *s, uint16_t key, 
> FWCfgCallback callback,
>  
>      s->entries[arch][key].data = data;
>      s->entries[arch][key].len = (uint32_t)len;
> +    s->entries[arch][key].reset_len = (uint32_t)len;
> +    s->entries[arch][key].max_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)
> +                              void *data, size_t len, size_t max_len)
>  {
>      int i, index;
>      size_t dsize;
> @@ -475,7 +543,8 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char 
> *filename,
>      assert(index < FW_CFG_FILE_SLOTS);
>  
>      fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index,
> -                                   callback, callback_opaque, data, len);
> +                                   callback, callback_opaque, data, len,
> +                                   max_len);
>  
>      pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
>              filename);
> @@ -496,7 +565,7 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char 
> *filename,
>  void fw_cfg_add_file(FWCfgState *s,  const char *filename,
>                       void *data, size_t len)
>  {
> -    fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len);
> +    fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, len);
>  }
>  
>  static void fw_cfg_machine_ready(struct Notifier *n, void *data)
> -- 
> MST
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK