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

2011-08-05 Thread 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 Hendersonr...@twiddle.net
Signed-off-by: Avi Kivitya...@redhat.com


Reviewed-by: Anthony Liguori aligu...@us.ibm.com

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  0x1) {
@@ -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 0xa and 0xc */
-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 0xa and 0xc */
-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

[PATCH v3 08/39] vga: simplify vga window mmio access functions

2011-08-04 Thread Avi Kivity 
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 r...@twiddle.net
Signed-off-by: Avi Kivity a...@redhat.com
---
 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  0x1) {
@@ -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 0xa and 0xc */
-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 0xa and 0xc */
-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