[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] loader: add support for resizeable blobs
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] loader: add support for resizeable blobs |
Date: |
Mon, 28 Jul 2014 21:07:57 +0200 |
On Mon, Jul 28, 2014 at 05:52:27PM +0100, Dr. David Alan Gilbert wrote:
> * 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
It's an API issue - so you are more comfortable with
an explicit flag?
> > +
> > 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