| Message ID | 1433787045-6215-2-git-send-email-somlo@cmu.edu |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 08, 2015 at 02:10:44PM -0400, Gabriel L. Somlo wrote: > Allow the ability to modify the value of an existing 16-bit integer > fw_cfg item. > > Signed-off-by: Gabriel Somlo <somlo@cmu.edu> > --- > > Couple of thoughts: > > 1. I'm thinking about pre-emptively creating _i16, _i32, and _i64 > versions, but right now (for fixing sparc and ppc) we only need > the _i16 version. What to do ? Presumably, I'll update the documentation to mention these update functions as well (one more reason to do all three versions rather than just _i16) :) > > 2. Part of me wants to find the memory location containing the previous > value and simply overwrite it, but I'll need to somehow ensure the > blob being replaced was of the same size, etc., which could get hairy. > So for now I'm going with the paranoid/safe version which allocates > a new blob and frees the old one. > > Thanks, > Gabriel > > hw/nvram/fw_cfg.c | 10 ++++++++++ > include/hw/nvram/fw_cfg.h | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 68eff77..08b5cc3 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -484,6 +484,16 @@ void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value) > fw_cfg_add_bytes(s, key, copy, sizeof(value)); > } > > +void fw_cfg_modify_i16(FWCfgState *s, uint16_t key, uint16_t value) > +{ > + uint16_t *copy, *old; > + > + copy = g_malloc(sizeof(value)); > + *copy = cpu_to_le16(value); > + old = fw_cfg_modify_bytes_read(s, key, copy, sizeof(value)); > + g_free(old); > +} > + > void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value) > { > uint32_t *copy; > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index 6d8a8ac..bc6c4a0 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -67,6 +67,7 @@ typedef void (*FWCfgReadCallback)(void *opaque, uint32_t offset); > void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len); > 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_modify_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, > -- > 2.1.0 >
On Mo, 2015-06-08 at 14:26 -0400, Gabriel L. Somlo wrote: > On Mon, Jun 08, 2015 at 02:10:44PM -0400, Gabriel L. Somlo wrote: > > Allow the ability to modify the value of an existing 16-bit integer > > fw_cfg item. > > > > Signed-off-by: Gabriel Somlo <somlo@cmu.edu> > > --- > > > > Couple of thoughts: > > > > 1. I'm thinking about pre-emptively creating _i16, _i32, and _i64 > > versions, but right now (for fixing sparc and ppc) we only need > > the _i16 version. What to do ? > > Presumably, I'll update the documentation to mention these update > functions as well (one more reason to do all three versions rather > than just _i16) :) Should be easy to add this once they are actually needed. Documentation update is fine, can go incremental though. cheers, Gerd
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 68eff77..08b5cc3 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -484,6 +484,16 @@ void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value) fw_cfg_add_bytes(s, key, copy, sizeof(value)); } +void fw_cfg_modify_i16(FWCfgState *s, uint16_t key, uint16_t value) +{ + uint16_t *copy, *old; + + copy = g_malloc(sizeof(value)); + *copy = cpu_to_le16(value); + old = fw_cfg_modify_bytes_read(s, key, copy, sizeof(value)); + g_free(old); +} + void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value) { uint32_t *copy; diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 6d8a8ac..bc6c4a0 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -67,6 +67,7 @@ typedef void (*FWCfgReadCallback)(void *opaque, uint32_t offset); void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len); 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_modify_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,
Allow the ability to modify the value of an existing 16-bit integer fw_cfg item. Signed-off-by: Gabriel Somlo <somlo@cmu.edu> --- Couple of thoughts: 1. I'm thinking about pre-emptively creating _i16, _i32, and _i64 versions, but right now (for fixing sparc and ppc) we only need the _i16 version. What to do ? 2. Part of me wants to find the memory location containing the previous value and simply overwrite it, but I'll need to somehow ensure the blob being replaced was of the same size, etc., which could get hairy. So for now I'm going with the paranoid/safe version which allocates a new blob and frees the old one. Thanks, Gabriel hw/nvram/fw_cfg.c | 10 ++++++++++ include/hw/nvram/fw_cfg.h | 1 + 2 files changed, 11 insertions(+)