| Message ID | 1257962966-22902-2-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
Alexander Graf wrote: > The fw_cfg interface can only handle up to 16 bits of data for its streams. > While that isn't too much of a problem when handling integers, we would > like to stream full kernel images over that interface! > > So let's extend it to 32 bit length variables. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > hw/fw_cfg.c | 8 ++++---- > hw/fw_cfg.h | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c > index a6d811b..3a3f694 100644 > --- a/hw/fw_cfg.c > +++ b/hw/fw_cfg.c > @@ -39,7 +39,7 @@ > #define FW_CFG_SIZE 2 > > typedef struct _FWCfgEntry { > - uint16_t len; > + uint32_t len; > uint8_t *data; > void *callback_opaque; > FWCfgCallback callback; > @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry { > typedef struct _FWCfgState { > FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; > uint16_t cur_entry; > - uint16_t cur_offset; > + uint32_t cur_offset; > } FWCfgState; > > static void fw_cfg_write(FWCfgState *s, uint8_t value) > @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = { > .minimum_version_id_old = 1, > .fields = (VMStateField []) { > VMSTATE_UINT16(cur_entry, FWCfgState), > - VMSTATE_UINT16(cur_offset, FWCfgState), > + VMSTATE_UINT32(cur_offset, FWCfgState), > VMSTATE_END_OF_LIST() > } > }; > > -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len) > +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len) > { > FWCfgState *s = opaque; > int arch = !!(key & FW_CFG_ARCH_LOCAL); > We need to bump a version here. > diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h > index 30dfec7..359d45a 100644 > --- a/hw/fw_cfg.h > +++ b/hw/fw_cfg.h > @@ -28,7 +28,7 @@ > #ifndef NO_QEMU_PROTOS > typedef void (*FWCfgCallback)(void *opaque, uint8_t *data); > > -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len); > +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len); > int fw_cfg_add_i16(void *opaque, uint16_t key, uint16_t value); > int fw_cfg_add_i32(void *opaque, uint16_t key, uint32_t value); > int fw_cfg_add_i64(void *opaque, uint16_t key, uint64_t value); >
Anthony Liguori wrote: > Alexander Graf wrote: >> The fw_cfg interface can only handle up to 16 bits of data for its >> streams. >> While that isn't too much of a problem when handling integers, we would >> like to stream full kernel images over that interface! >> >> So let's extend it to 32 bit length variables. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> hw/fw_cfg.c | 8 ++++---- >> hw/fw_cfg.h | 2 +- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c >> index a6d811b..3a3f694 100644 >> --- a/hw/fw_cfg.c >> +++ b/hw/fw_cfg.c >> @@ -39,7 +39,7 @@ >> #define FW_CFG_SIZE 2 >> >> typedef struct _FWCfgEntry { >> - uint16_t len; >> + uint32_t len; >> uint8_t *data; >> void *callback_opaque; >> FWCfgCallback callback; >> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry { >> typedef struct _FWCfgState { >> FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; >> uint16_t cur_entry; >> - uint16_t cur_offset; >> + uint32_t cur_offset; >> } FWCfgState; >> >> static void fw_cfg_write(FWCfgState *s, uint8_t value) >> @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = { >> .minimum_version_id_old = 1, >> .fields = (VMStateField []) { >> VMSTATE_UINT16(cur_entry, FWCfgState), >> - VMSTATE_UINT16(cur_offset, FWCfgState), >> + VMSTATE_UINT32(cur_offset, FWCfgState), >> VMSTATE_END_OF_LIST() >> } >> }; >> >> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >> uint16_t len) >> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >> uint32_t len) >> { >> FWCfgState *s = opaque; >> int arch = !!(key & FW_CFG_ARCH_LOCAL); >> > > We need to bump a version here. Sure - which one? Alex
Alexander Graf wrote: > Anthony Liguori wrote: > >> Alexander Graf wrote: >> >>> The fw_cfg interface can only handle up to 16 bits of data for its >>> streams. >>> While that isn't too much of a problem when handling integers, we would >>> like to stream full kernel images over that interface! >>> >>> So let's extend it to 32 bit length variables. >>> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> --- >>> hw/fw_cfg.c | 8 ++++---- >>> hw/fw_cfg.h | 2 +- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c >>> index a6d811b..3a3f694 100644 >>> --- a/hw/fw_cfg.c >>> +++ b/hw/fw_cfg.c >>> @@ -39,7 +39,7 @@ >>> #define FW_CFG_SIZE 2 >>> >>> typedef struct _FWCfgEntry { >>> - uint16_t len; >>> + uint32_t len; >>> uint8_t *data; >>> void *callback_opaque; >>> FWCfgCallback callback; >>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry { >>> typedef struct _FWCfgState { >>> FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; >>> uint16_t cur_entry; >>> - uint16_t cur_offset; >>> + uint32_t cur_offset; >>> } FWCfgState; >>> >>> static void fw_cfg_write(FWCfgState *s, uint8_t value) >>> @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = { >>> .minimum_version_id_old = 1, >>> .fields = (VMStateField []) { >>> VMSTATE_UINT16(cur_entry, FWCfgState), >>> - VMSTATE_UINT16(cur_offset, FWCfgState), >>> + VMSTATE_UINT32(cur_offset, FWCfgState), >>> VMSTATE_END_OF_LIST() >>> } >>> }; >>> >>> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >>> uint16_t len) >>> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >>> uint32_t len) >>> { >>> FWCfgState *s = opaque; >>> int arch = !!(key & FW_CFG_ARCH_LOCAL); >>> >>> >> We need to bump a version here. >> > > Sure - which one? > The version_id field in vmstate_fw_cfg. You also have to try to support older versions which means you may want to either split cur_offset into a high and low or ask Juan what the appropriate vodoo would be. Regards, Anthony Liguori > Alex >
On 11.11.2009, at 23:22, Anthony Liguori wrote: > Alexander Graf wrote: >> Anthony Liguori wrote: >> >>> Alexander Graf wrote: >>> >>>> The fw_cfg interface can only handle up to 16 bits of data for its >>>> streams. >>>> While that isn't too much of a problem when handling integers, we >>>> would >>>> like to stream full kernel images over that interface! >>>> >>>> So let's extend it to 32 bit length variables. >>>> >>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>>> --- >>>> hw/fw_cfg.c | 8 ++++---- >>>> hw/fw_cfg.h | 2 +- >>>> 2 files changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c >>>> index a6d811b..3a3f694 100644 >>>> --- a/hw/fw_cfg.c >>>> +++ b/hw/fw_cfg.c >>>> @@ -39,7 +39,7 @@ >>>> #define FW_CFG_SIZE 2 >>>> typedef struct _FWCfgEntry { >>>> - uint16_t len; >>>> + uint32_t len; >>>> uint8_t *data; >>>> void *callback_opaque; >>>> FWCfgCallback callback; >>>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry { >>>> typedef struct _FWCfgState { >>>> FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; >>>> uint16_t cur_entry; >>>> - uint16_t cur_offset; >>>> + uint32_t cur_offset; >>>> } FWCfgState; >>>> static void fw_cfg_write(FWCfgState *s, uint8_t value) >>>> @@ -171,12 +171,12 @@ static const VMStateDescription >>>> vmstate_fw_cfg = { >>>> .minimum_version_id_old = 1, >>>> .fields = (VMStateField []) { >>>> VMSTATE_UINT16(cur_entry, FWCfgState), >>>> - VMSTATE_UINT16(cur_offset, FWCfgState), >>>> + VMSTATE_UINT32(cur_offset, FWCfgState), >>>> VMSTATE_END_OF_LIST() >>>> } >>>> }; >>>> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >>>> uint16_t len) >>>> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, >>>> uint32_t len) >>>> { >>>> FWCfgState *s = opaque; >>>> int arch = !!(key & FW_CFG_ARCH_LOCAL); >>>> >>> We need to bump a version here. >>> >> >> Sure - which one? >> > > The version_id field in vmstate_fw_cfg. You also have to try to > support older versions which means you may want to either split > cur_offset into a high and low or ask Juan what the appropriate > vodoo would be. Juan, I'd really love to learn some new voodoo :-). This whole new qdev whatever based save format was supposed to make things like this easy, right? I would've known what to do with the old code ... Alex
Alexander Graf wrote: > Juan, I'd really love to learn some new voodoo :-). > This whole new qdev whatever based save format was supposed to make > things like this easy, right? I would've known what to do with the old > code ... I think Juan's mentioned something about writing a doc explaining how to use VMState correctly. I think it would certainly be helpful for situations like this. But the most important part of VMState is that it converts something that was previously open coded and opaque to something that is data-driven and introspectable. I think it's done an extremely good job of achieving those goals. As we get everything converted, we can potentially figure out some ways to make this all a bit easier to understand. Right now, I think how we support backwards compatibility is admittedly awkward. Regards, Anthony Liguori > Alex
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c index a6d811b..3a3f694 100644 --- a/hw/fw_cfg.c +++ b/hw/fw_cfg.c @@ -39,7 +39,7 @@ #define FW_CFG_SIZE 2 typedef struct _FWCfgEntry { - uint16_t len; + uint32_t len; uint8_t *data; void *callback_opaque; FWCfgCallback callback; @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry { typedef struct _FWCfgState { FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; uint16_t cur_entry; - uint16_t cur_offset; + uint32_t cur_offset; } FWCfgState; static void fw_cfg_write(FWCfgState *s, uint8_t value) @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = { .minimum_version_id_old = 1, .fields = (VMStateField []) { VMSTATE_UINT16(cur_entry, FWCfgState), - VMSTATE_UINT16(cur_offset, FWCfgState), + VMSTATE_UINT32(cur_offset, FWCfgState), VMSTATE_END_OF_LIST() } }; -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len) +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len) { FWCfgState *s = opaque; int arch = !!(key & FW_CFG_ARCH_LOCAL); diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h index 30dfec7..359d45a 100644 --- a/hw/fw_cfg.h +++ b/hw/fw_cfg.h @@ -28,7 +28,7 @@ #ifndef NO_QEMU_PROTOS typedef void (*FWCfgCallback)(void *opaque, uint8_t *data); -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len); +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len); int fw_cfg_add_i16(void *opaque, uint16_t key, uint16_t value); int fw_cfg_add_i32(void *opaque, uint16_t key, uint32_t value); int fw_cfg_add_i64(void *opaque, uint16_t key, uint64_t value);
The fw_cfg interface can only handle up to 16 bits of data for its streams. While that isn't too much of a problem when handling integers, we would like to stream full kernel images over that interface! So let's extend it to 32 bit length variables. Signed-off-by: Alexander Graf <agraf@suse.de> --- hw/fw_cfg.c | 8 ++++---- hw/fw_cfg.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)