On 11/05/15 15:32, Gabriel L. Somlo wrote: > IOPort read access is limited to one byte at a time by > fw_cfg_comb_valid(). As such, fw_cfg_comb_read() may safely > ignore its size argument (which will always be 1), and simply > call its fw_cfg_read() helper function once, returning 8 bits > via the least significant byte of a 64-bit return value. > > This patch replaces fw_cfg_comb_read() with the generic method > fw_cfg_data_read(), and removes the unused fw_cfg_read() helper. > > When called with size = 1, fw_cfg_data_read() acts exactly like > fw_cfg_read(), performing the same set of sanity checks, and > executing the while loop at most once (subject to the current > read offset being within range). > > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Marc MarĂ <mar...@redhat.com> > Signed-off-by: Gabriel Somlo <so...@cmu.edu> > Reviewd-by: Laszlo Ersek <ler...@redhat.com>
Not sure who will pick up this series, but he please fix the typo in the last tag above, at commit time. Thanks! Laszlo > --- > hw/nvram/fw_cfg.c | 25 +------------------------ > 1 file changed, 1 insertion(+), 24 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 8f566f6..a1d650d 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -304,23 +304,6 @@ static uint64_t fw_cfg_data_read(void *opaque, hwaddr > addr, unsigned size) > return value; > } > > -static uint8_t fw_cfg_read(FWCfgState *s) > -{ > - int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > - FWCfgEntry *e = (s->cur_entry == FW_CFG_INVALID) ? NULL : > - &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK]; > - uint8_t ret; > - > - if (s->cur_entry == FW_CFG_INVALID || !e->data || s->cur_offset >= > e->len) > - ret = 0; > - else { > - ret = e->data[s->cur_offset++]; > - } > - > - trace_fw_cfg_read(s, ret); > - return ret; > -} > - > static void fw_cfg_data_mem_write(void *opaque, hwaddr addr, > uint64_t value, unsigned size) > { > @@ -470,12 +453,6 @@ static bool fw_cfg_ctl_mem_valid(void *opaque, hwaddr > addr, > return is_write && size == 2; > } > > -static uint64_t fw_cfg_comb_read(void *opaque, hwaddr addr, > - unsigned size) > -{ > - return fw_cfg_read(opaque); > -} > - > static void fw_cfg_comb_write(void *opaque, hwaddr addr, > uint64_t value, unsigned size) > { > @@ -513,7 +490,7 @@ static const MemoryRegionOps fw_cfg_data_mem_ops = { > }; > > static const MemoryRegionOps fw_cfg_comb_mem_ops = { > - .read = fw_cfg_comb_read, > + .read = fw_cfg_data_read, > .write = fw_cfg_comb_write, > .endianness = DEVICE_LITTLE_ENDIAN, > .valid.accepts = fw_cfg_comb_valid, >