Re: [Qemu-devel] [PATCH v3 08/39] vga: simplify vga window mmio access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:
> Make use of the memory API's ability to satisfy multi-byte accesses via
> multiple single-byte accesses.
>
> We have to keep vga_mem_{read,write}b() since they're used by cirrus.
>
> Reviewed-by: Richard Henderson<address@hidden>
> Signed-off-by: Avi Kivity<address@hidden>

Reviewed-by: Anthony Liguori <address@hidden>

Regards,

Anthony Liguori

> ---
>   hw/cirrus_vga.c |    4 +-
>   hw/vga.c        |   56 +++++++-----------------------------------------------
>   hw/vga_int.h    |    4 +-
>   3 files changed, 12 insertions(+), 52 deletions(-)
>
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index 92696d9..3db15bf 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -1965,7 +1965,7 @@ static uint64_t cirrus_vga_mem_read(void *opaque,
>       uint32_t val;
>
>       if ((s->vga.sr[0x07]&  0x01) == 0) {
> -       return vga_mem_readb(s, addr);
> +        return vga_mem_readb(&s->vga, addr);
>       }
>
>       if (addr<  0x10000) {
> @@ -2010,7 +2010,7 @@ static void cirrus_vga_mem_write(void *opaque,
>       unsigned mode;
>
>       if ((s->vga.sr[0x07]&  0x01) == 0) {
> -       vga_mem_writeb(s, addr, mem_value);
> +        vga_mem_writeb(&s->vga, addr, mem_value);
>           return;
>       }
>
> diff --git a/hw/vga.c b/hw/vga.c
> index cdd8255..f5dd519 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -707,9 +707,8 @@ static void vbe_ioport_write_data(void *opaque, uint32_t 
> addr, uint32_t val)
>   #endif
>
>   /* called for accesses between 0xa0000 and 0xc0000 */
> -uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr)
> +uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr)
>   {
> -    VGACommonState *s = opaque;
>       int memory_map_mode, plane;
>       uint32_t ret;
>
> @@ -763,28 +762,9 @@ uint32_t vga_mem_readb(void *opaque, target_phys_addr_t 
> addr)
>       return ret;
>   }
>
> -static uint32_t vga_mem_readw(void *opaque, target_phys_addr_t addr)
> -{
> -    uint32_t v;
> -    v = vga_mem_readb(opaque, addr);
> -    v |= vga_mem_readb(opaque, addr + 1)<<  8;
> -    return v;
> -}
> -
> -static uint32_t vga_mem_readl(void *opaque, target_phys_addr_t addr)
> -{
> -    uint32_t v;
> -    v = vga_mem_readb(opaque, addr);
> -    v |= vga_mem_readb(opaque, addr + 1)<<  8;
> -    v |= vga_mem_readb(opaque, addr + 2)<<  16;
> -    v |= vga_mem_readb(opaque, addr + 3)<<  24;
> -    return v;
> -}
> -
>   /* called for accesses between 0xa0000 and 0xc0000 */
> -void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> +void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val)
>   {
> -    VGACommonState *s = opaque;
>       int memory_map_mode, plane, write_mode, b, func_select, mask;
>       uint32_t write_mask, bit_mask, set_mask;
>
> @@ -916,20 +896,6 @@ void vga_mem_writeb(void *opaque, target_phys_addr_t addr, 
> uint32_t val)
>       }
>   }
>
> -static void vga_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    vga_mem_writeb(opaque, addr, val&  0xff);
> -    vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
> -}
> -
> -static void vga_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    vga_mem_writeb(opaque, addr, val&  0xff);
> -    vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
> -    vga_mem_writeb(opaque, addr + 2, (val>>  16)&  0xff);
> -    vga_mem_writeb(opaque, addr + 3, (val>>  24)&  0xff);
> -}
> -
>   typedef void vga_draw_glyph8_func(uint8_t *d, int linesize,
>                                const uint8_t *font_ptr, int h,
>                                uint32_t fgcol, uint32_t bgcol);
> @@ -2104,12 +2070,7 @@ static uint64_t vga_mem_read(void *opaque, 
> target_phys_addr_t addr,
>   {
>       VGACommonState *s = opaque;
>
> -    switch (size) {
> -    case 1: return vga_mem_readb(s, addr);
> -    case 2: return vga_mem_readw(s, addr);
> -    case 4: return vga_mem_readl(s, addr);
> -    default: abort();
> -    }
> +    return vga_mem_readb(s, addr);
>   }
>
>   static void vga_mem_write(void *opaque, target_phys_addr_t addr,
> @@ -2117,18 +2078,17 @@ static void vga_mem_write(void *opaque, 
> target_phys_addr_t addr,
>   {
>       VGACommonState *s = opaque;
>
> -    switch (size) {
> -    case 1: return vga_mem_writeb(s, addr, data);
> -    case 2: return vga_mem_writew(s, addr, data);
> -    case 4: return vga_mem_writel(s, addr, data);
> -    default: abort();
> -    }
> +    return vga_mem_writeb(s, addr, data);
>   }
>
>   const MemoryRegionOps vga_mem_ops = {
>       .read = vga_mem_read,
>       .write = vga_mem_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
>   };
>
>   static int vga_common_post_load(void *opaque, int version_id)
> diff --git a/hw/vga_int.h b/hw/vga_int.h
> index 4592d2c..100d98c 100644
> --- a/hw/vga_int.h
> +++ b/hw/vga_int.h
> @@ -198,8 +198,8 @@ void vga_dirty_log_restart(VGACommonState *s);
>   extern const VMStateDescription vmstate_vga_common;
>   uint32_t vga_ioport_read(void *opaque, uint32_t addr);
>   void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val);
> -uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr);
> -void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val);
> +uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
> +void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
>   void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
>   int ppm_save(const char *filename, struct DisplaySurface *ds);