Patchwork [2/3,v2] vmware_vga: Return a value for FB_SIZE before the device is enabled

login
register
mail settings
Submitter BALATON Zoltan
Date Oct. 3, 2012, 9:30 a.m.
Message ID <alpine.GSO.2.00.1210031130270.19707@mono>
Download mbox | patch
Permalink /patch/188740/
State New
Headers show

Comments

BALATON Zoltan - Oct. 3, 2012, 9:30 a.m.
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
  hw/vmware_vga.c |   13 ++++++++-----
  1 file changed, 8 insertions(+), 5 deletions(-)

  v2: Rebase to apply to current
Paolo Bonzini - Oct. 3, 2012, 9:43 a.m.
Il 03/10/2012 11:30, BALATON Zoltan ha scritto:
> According to the documentation drivers using this device should read
> FB_SIZE before enabling the device to know what memory to map. This
> would not work if we return 0 before enabled.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/vmware_vga.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
>  v2: Rebase to apply to current
> 
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 20f4fb8..5e4786f 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -39,8 +39,12 @@
>  struct vmsvga_state_s {
>      VGACommonState vga;
> 
> +/* -*- The members marked are now unused and could be removed but they are
> + *     contained in the VMState thus need special handling. Maybe they
> could
> + *     be removed the next time a new machine type is added.
> + */

To fix this, you can bump the version number and replace the depth and
fb_size VMState fields with VMSTATE_UNUSED.

Paolo

>      int invalidated;
> -    int depth;
> +    int depth;  /* -*- */
>      int enable;
>      int config;
>      struct {
> @@ -58,7 +62,7 @@ struct vmsvga_state_s {
>      uint32_t guest;
>      uint32_t svgaid;
>      int syncing;
> -    int fb_size;
> +    int fb_size;  /* -*- */
> 
>      MemoryRegion fifo_ram;
>      uint8_t *fifo_ptr;
> @@ -733,10 +737,10 @@ static uint32_t vmsvga_value_read(void *opaque,
> uint32_t address)
>          return 0x0;
> 
>      case SVGA_REG_VRAM_SIZE:
> -        return s->vga.vram_size;
> +        return s->vga.vram_size; /* No physical VRAM besides the
> framebuffer */
> 
>      case SVGA_REG_FB_SIZE:
> -        return s->fb_size;
> +        return s->vga.vram_size;
> 
>      case SVGA_REG_CAPABILITIES:
>          caps = SVGA_CAP_NONE;
> @@ -821,7 +825,6 @@ static void vmsvga_value_write(void *opaque,
> uint32_t address, uint32_t value)
>          s->invalidated = 1;
>          s->vga.invalidate(&s->vga);
>          if (s->enable) {
> -            s->fb_size = ((s->depth + 7) >> 3) * s->new_width *
> s->new_height;
>              vga_dirty_log_stop(&s->vga);
>          } else {
>              vga_dirty_log_start(&s->vga);

Patch

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 20f4fb8..5e4786f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,8 +39,12 @@ 
  struct vmsvga_state_s {
      VGACommonState vga;

+/* -*- The members marked are now unused and could be removed but they are
+ *     contained in the VMState thus need special handling. Maybe they could
+ *     be removed the next time a new machine type is added.
+ */
      int invalidated;
-    int depth;
+    int depth;  /* -*- */
      int enable;
      int config;
      struct {
@@ -58,7 +62,7 @@  struct vmsvga_state_s {
      uint32_t guest;
      uint32_t svgaid;
      int syncing;
-    int fb_size;
+    int fb_size;  /* -*- */

      MemoryRegion fifo_ram;
      uint8_t *fifo_ptr;
@@ -733,10 +737,10 @@  static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
          return 0x0;

      case SVGA_REG_VRAM_SIZE:
-        return s->vga.vram_size;
+        return s->vga.vram_size; /* No physical VRAM besides the framebuffer */

      case SVGA_REG_FB_SIZE:
-        return s->fb_size;
+        return s->vga.vram_size;

      case SVGA_REG_CAPABILITIES:
          caps = SVGA_CAP_NONE;
@@ -821,7 +825,6 @@  static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value)
          s->invalidated = 1;
          s->vga.invalidate(&s->vga);
          if (s->enable) {
-            s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
              vga_dirty_log_stop(&s->vga);
          } else {
              vga_dirty_log_start(&s->vga);