[Qemu-devel] [PATCH 0/2] VGA fixes

[Qemu-devel] [PATCH 0/2] VGA fixes

[Juan Quintela]

Hi

This includes two fixes:
- also write gr0/gr1 on cirrus_vga
  this was a bug introduced on my vga_cleanup series
- move dirty_log_functions to vga.c
  this is needed by qemu-kvm

Please, apply, Juan.

Juan Quintela (2):
  cirrus_vga: also assign gr0/1 when writting shadow_gr0/1
  vga: move back dirty_log functions to vga.c

 hw/cirrus_vga.c |    2 ++
 hw/vga-pci.c    |   12 ------------
 hw/vga.c        |   12 ++++++++++++
 3 files changed, 14 insertions(+), 12 deletions(-)

[Qemu-devel] [PATCH 1/2] cirrus_vga: also assign gr0/1 when writting shadow_gr0/1

[Juan Quintela]

This is a regression/bug caused by previous vga_cleanup.  This fixes
Ubuntu installer reported by:
	 Pierre Riteau

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/cirrus_vga.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 7e95f10..ae4be82 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -1490,9 +1490,11 @@ cirrus_vga_write_gr(CirrusVGAState * s, unsigned reg_index, int reg_value)
 #endif
     switch (reg_index) {
     case 0x00:			// Standard VGA, BGCOLOR 0x000000ff
+	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
 	s->cirrus_shadow_gr0 = reg_value;
 	break;
     case 0x01:			// Standard VGA, FGCOLOR 0x000000ff
+	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
 	s->cirrus_shadow_gr1 = reg_value;
 	break;
     case 0x02:			// Standard VGA
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 1/2] cirrus_vga: also assign gr0/1 when writting shadow_gr0/1

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> This is a regression/bug caused by previous vga_cleanup.  This fixes
> Ubuntu installer reported by:
> 	 Pierre Riteau
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Broken in commit 22286bc6.  Patch restores old behavior of
cirrus_vga_ioport_write() for address 0x3cf.

But what about cirrus_mmio_blt_write() for address
CIRRUS_MMIO_BLTBGCOLOR + 0?  There we had

    cirrus_hook_write_gr(s, 0x00, value);

which did not update s->vga.gr[0x00].  It does with your patch, doesn't
it?

Same for CIRRUS_MMIO_BLTFGCOLOR + 0.

[Qemu-devel] Re: [PATCH 1/2] cirrus_vga: also assign gr0/1 when writting shadow_gr0/1

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> This is a regression/bug caused by previous vga_cleanup.  This fixes
>> Ubuntu installer reported by:
>> 	 Pierre Riteau
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Broken in commit 22286bc6.  Patch restores old behavior of
> cirrus_vga_ioport_write() for address 0x3cf.
>
> But what about cirrus_mmio_blt_write() for address
> CIRRUS_MMIO_BLTBGCOLOR + 0?  There we had
>
>     cirrus_hook_write_gr(s, 0x00, value);
>
> which did not update s->vga.gr[0x00].  It does with your patch, doesn't
> it?
>
> Same for CIRRUS_MMIO_BLTFGCOLOR + 0.

Yeap :(
My VGA expertise is inexistent, I don't know what to do here.  I will
also fix that.

Thanks for the review, Juan.

[Qemu-devel] [PATCH 2/2] vga: move back dirty_log functions to vga.c

[Juan Quintela]

They are needed there on qemu-kvm.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/vga-pci.c |   12 ------------
 hw/vga.c     |   12 ++++++++++++
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 6038cec..ccc78e5 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -28,7 +28,6 @@
 #include "vga_int.h"
 #include "pixel_ops.h"
 #include "qemu-timer.h"
-#include "kvm.h"

 typedef struct PCIVGAState {
     PCIDevice dev;
@@ -59,17 +58,6 @@ static int pci_vga_load(QEMUFile *f, void *opaque, int version_id)
     return vga_common_load(f, &s->vga, version_id);
 }

-void vga_dirty_log_start(VGACommonState *s)
-{
-    if (kvm_enabled() && s->map_addr)
-        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
-
-    if (kvm_enabled() && s->lfb_vram_mapped) {
-        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
-        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
-    }
-}
-
 static void vga_map(PCIDevice *pci_dev, int region_num,
                     uint32_t addr, uint32_t size, int type)
 {
diff --git a/hw/vga.c b/hw/vga.c
index 514371c..6cbd3d3 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -28,6 +28,7 @@
 #include "vga_int.h"
 #include "pixel_ops.h"
 #include "qemu-timer.h"
+#include "kvm.h"

 //#define DEBUG_VGA
 //#define DEBUG_VGA_MEM
@@ -1576,6 +1577,17 @@ static void vga_sync_dirty_bitmap(VGACommonState *s)
     }
 }

+void vga_dirty_log_start(VGACommonState *s)
+{
+    if (kvm_enabled() && s->map_addr)
+        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
+
+    if (kvm_enabled() && s->lfb_vram_mapped) {
+        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
+        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
+    }
+}
+
 /*
  * graphic modes
  */
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 2/2] vga: move back dirty_log functions to vga.c

[Jan Kiszka]

Juan Quintela wrote:
> They are needed there on qemu-kvm.

Well, my real argument still is that they are not PCI-specific, even if
they are currently only used by PCI-based adapters.

Acked-by: Jan Kiszka <jan.kiszka@siemens.com>

Thanks,
Jan

> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/vga-pci.c |   12 ------------
>  hw/vga.c     |   12 ++++++++++++
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/vga-pci.c b/hw/vga-pci.c
> index 6038cec..ccc78e5 100644
> --- a/hw/vga-pci.c
> +++ b/hw/vga-pci.c
> @@ -28,7 +28,6 @@
>  #include "vga_int.h"
>  #include "pixel_ops.h"
>  #include "qemu-timer.h"
> -#include "kvm.h"
> 
>  typedef struct PCIVGAState {
>      PCIDevice dev;
> @@ -59,17 +58,6 @@ static int pci_vga_load(QEMUFile *f, void *opaque, int version_id)
>      return vga_common_load(f, &s->vga, version_id);
>  }
> 
> -void vga_dirty_log_start(VGACommonState *s)
> -{
> -    if (kvm_enabled() && s->map_addr)
> -        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
> -
> -    if (kvm_enabled() && s->lfb_vram_mapped) {
> -        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
> -        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
> -    }
> -}
> -
>  static void vga_map(PCIDevice *pci_dev, int region_num,
>                      uint32_t addr, uint32_t size, int type)
>  {
> diff --git a/hw/vga.c b/hw/vga.c
> index 514371c..6cbd3d3 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -28,6 +28,7 @@
>  #include "vga_int.h"
>  #include "pixel_ops.h"
>  #include "qemu-timer.h"
> +#include "kvm.h"
> 
>  //#define DEBUG_VGA
>  //#define DEBUG_VGA_MEM
> @@ -1576,6 +1577,17 @@ static void vga_sync_dirty_bitmap(VGACommonState *s)
>      }
>  }
> 
> +void vga_dirty_log_start(VGACommonState *s)
> +{
> +    if (kvm_enabled() && s->map_addr)
> +        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
> +
> +    if (kvm_enabled() && s->lfb_vram_mapped) {
> +        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
> +        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
> +    }
> +}
> +
>  /*
>   * graphic modes
>   */

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux