[Qemu-devel] [PATCH 1/2] Make sure to enable dirty tracking of VBE vram mapping

[Qemu-devel] [PATCH 1/2] Make sure to enable dirty tracking of VBE vram mapping

[Anthony Liguori]

Apparently, VBE maps the VGA vram to a fixed physical location.  KVM requires
that all mappings of the VGA vram have dirty tracking enabled on them.  Any
access to the VGA vram through the VBE mapping currently fails to result in
dirty page tracking updates causing a black screen.

This is the true root cause of VMware VGA not working correctly under KVM and
likely also an issue with some of the std-vga black screen issues too.

Cirrus does not enable VBE so it would not be a problem when using Cirrus.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/vga.c        |   35 +++++++++++++++++++++++++----------
 hw/vga_int.h    |    5 ++++-
 hw/vmware_vga.c |    7 +------
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 013ff10..cb4b750 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1577,6 +1577,13 @@ static void vga_sync_dirty_bitmap(VGAState *s)
         cpu_physical_sync_dirty_bitmap(isa_mem_base + 0xa0000, 0xa8000);
         cpu_physical_sync_dirty_bitmap(isa_mem_base + 0xa8000, 0xb0000);
     }
+
+#ifdef CONFIG_BOCHS_VBE
+    if (s->vbe_mapped) {
+        cpu_physical_sync_dirty_bitmap(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
+                                       VBE_DISPI_LFB_PHYSICAL_ADDRESS + s->vram_size);
+    }
+#endif
 }
 
 /*
@@ -2233,6 +2240,12 @@ void vga_dirty_log_start(VGAState *s)
         kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
         kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
     }
+
+#ifdef CONFIG_BOCHS_VBE
+    if (kvm_enabled() && s->vbe_mapped) {
+        kvm_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
+    }
+#endif
 }
 
 static void vga_map(PCIDevice *pci_dev, int region_num,
@@ -2428,6 +2441,16 @@ static void vga_mm_init(VGAState *s, target_phys_addr_t vram_base,
     qemu_register_coalesced_mmio(vram_base + 0x000a0000, 0x20000);
 }
 
+void vga_init_vbe(VGAState *s)
+{
+#ifdef CONFIG_BOCHS_VBE
+    /* XXX: use optimized standard vga accesses */
+    cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
+                                 VGA_RAM_SIZE, s->vram_offset);
+    s->vbe_mapped = 1;
+#endif
+}
+
 int isa_vga_init(void)
 {
     VGAState *s;
@@ -2439,12 +2462,8 @@ int isa_vga_init(void)
 
     s->ds = graphic_console_init(s->update, s->invalidate,
                                  s->screen_dump, s->text_update, s);
+    vga_init_vbe(s);
 
-#ifdef CONFIG_BOCHS_VBE
-    /* XXX: use optimized standard vga accesses */
-    cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
-                                 VGA_RAM_SIZE, s->vram_offset);
-#endif
     return 0;
 }
 
@@ -2460,12 +2479,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
 
     s->ds = graphic_console_init(s->update, s->invalidate,
                                  s->screen_dump, s->text_update, s);
+    vga_init_vbe(s);
 
-#ifdef CONFIG_BOCHS_VBE
-    /* XXX: use optimized standard vga accesses */
-    cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
-                                 VGA_RAM_SIZE, s->vram_offset);
-#endif
     return 0;
 }
 
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 631b1b0..0b41254 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -68,7 +68,8 @@
     uint16_t vbe_regs[VBE_DISPI_INDEX_NB];      \
     uint32_t vbe_start_addr;                    \
     uint32_t vbe_line_offset;                   \
-    uint32_t vbe_bank_mask;
+    uint32_t vbe_bank_mask;                     \
+    int vbe_mapped;
 
 #else
 
@@ -193,6 +194,8 @@ void vga_common_init(VGAState *s, int vga_ram_size);
 void vga_init(VGAState *s);
 void vga_reset(void *s);
 
+void vga_init_vbe(VGAState *s);
+
 void vga_dirty_log_start(VGAState *s);
 
 uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 79da1ff..bb17698 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1141,12 +1141,7 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size)
                                      vmsvga_invalidate_display,
                                      vmsvga_screen_dump,
                                      vmsvga_text_update, &s->vga);
-
-#ifdef CONFIG_BOCHS_VBE
-    /* XXX: use optimized standard vga accesses */
-    cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
-                                 vga_ram_size, s->vga.vram_offset);
-#endif
+    vga_init_vbe((VGAState *)s);
 }
 
 static void pci_vmsvga_save(QEMUFile *f, void *opaque)
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Anthony Liguori]

This is needed for VMware VGA to work properly under KVM.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/vmware_vga.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index bb17698..246011b 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev, int region_num,
 #endif
     cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
                     iomemtype);
+
+    s->vga.map_addr = addr;
+    s->vga.map_end = addr + s->vga.vram_size;
+    vga_dirty_log_start(&s->vga);
 }
 
 void pci_vmsvga_init(PCIBus *bus)
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Anthony Liguori]

Anthony Liguori wrote:
> This is needed for VMware VGA to work properly under KVM.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/vmware_vga.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index bb17698..246011b 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev, int region_num,
>  #endif
>      cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
>                      iomemtype);
> +
> +    s->vga.map_addr = addr;
> +    s->vga.map_end = addr + s->vga.vram_size;
> +    vga_dirty_log_start(&s->vga);
>  }
>
>  void pci_vmsvga_init(PCIBus *bus)
>   
N.B. depth is still screwed up with VMware vga but the fix for that 
looks a bit more non-obvious.  This VBE issue has been a long standing 
problem though and I suspect it effects other guests though.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Jordan Justen]

On Mon, Jun 8, 2009 at 2:53 PM, Anthony Liguori<aliguori@us.ibm.com> wrote:
> Anthony Liguori wrote:
>>
>> This is needed for VMware VGA to work properly under KVM.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  hw/vmware_vga.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>> index bb17698..246011b 100644
>> --- a/hw/vmware_vga.c
>> +++ b/hw/vmware_vga.c
>> @@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev,
>> int region_num,
>>  #endif
>>     cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
>>                     iomemtype);
>> +
>> +    s->vga.map_addr = addr;
>> +    s->vga.map_end = addr + s->vga.vram_size;
>> +    vga_dirty_log_start(&s->vga);
>>  }
>>
>>  void pci_vmsvga_init(PCIBus *bus)
>>
>
> N.B. depth is still screwed up with VMware vga but the fix for that looks a
> bit more non-obvious.  This VBE issue has been a long standing problem
> though and I suspect it effects other guests though.
>

I've tried booting Ubuntu 8.10/9.04 using '-vga vmware' on various versions of
kvm and qemu.  The result is always ok video during BIOS post, but garbage
during pre and post X.  Would this be a symptom of the 'VBE' issue that
you mentioned?

> --
> Regards,
>
> Anthony Liguori
>
>
>
>

Re: [Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Anthony Liguori]

Jordan Justen wrote:
> On Mon, Jun 8, 2009 at 2:53 PM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>   
>> Anthony Liguori wrote:
>>     
>>> This is needed for VMware VGA to work properly under KVM.
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> ---
>>>  hw/vmware_vga.c |    4 ++++
>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>>> index bb17698..246011b 100644
>>> --- a/hw/vmware_vga.c
>>> +++ b/hw/vmware_vga.c
>>> @@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev,
>>> int region_num,
>>>  #endif
>>>     cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
>>>                     iomemtype);
>>> +
>>> +    s->vga.map_addr = addr;
>>> +    s->vga.map_end = addr + s->vga.vram_size;
>>> +    vga_dirty_log_start(&s->vga);
>>>  }
>>>
>>>  void pci_vmsvga_init(PCIBus *bus)
>>>
>>>       
>> N.B. depth is still screwed up with VMware vga but the fix for that looks a
>> bit more non-obvious.  This VBE issue has been a long standing problem
>> though and I suspect it effects other guests though.
>>
>>     
>
> I've tried booting Ubuntu 8.10/9.04 using '-vga vmware' on various versions of
> kvm and qemu.  The result is always ok video during BIOS post, but garbage
> during pre and post X.  Would this be a symptom of the 'VBE' issue that
> you mentioned?
>   

Try the latest qemu git.  vmware vga should work there.

I'm not sure about qemu-kvm.  It depends on whether the changes survived 
the merge.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Jordan Justen]

On Wed, Jul 29, 2009 at 10:15 AM, Anthony Liguori<anthony@codemonkey.ws> wrote:
> Jordan Justen wrote:
>>
>> On Mon, Jun 8, 2009 at 2:53 PM, Anthony Liguori<aliguori@us.ibm.com>
>> wrote:
>>
>>>
>>> Anthony Liguori wrote:
>>>
>>>>
>>>> This is needed for VMware VGA to work properly under KVM.
>>>>
>>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>>> ---
>>>>  hw/vmware_vga.c |    4 ++++
>>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>>>> index bb17698..246011b 100644
>>>> --- a/hw/vmware_vga.c
>>>> +++ b/hw/vmware_vga.c
>>>> @@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice
>>>> *pci_dev,
>>>> int region_num,
>>>>  #endif
>>>>    cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
>>>>                    iomemtype);
>>>> +
>>>> +    s->vga.map_addr = addr;
>>>> +    s->vga.map_end = addr + s->vga.vram_size;
>>>> +    vga_dirty_log_start(&s->vga);
>>>>  }
>>>>
>>>>  void pci_vmsvga_init(PCIBus *bus)
>>>>
>>>>
>>>
>>> N.B. depth is still screwed up with VMware vga but the fix for that looks
>>> a
>>> bit more non-obvious.  This VBE issue has been a long standing problem
>>> though and I suspect it effects other guests though.
>>>
>>>
>>
>> I've tried booting Ubuntu 8.10/9.04 using '-vga vmware' on various
>> versions of
>> kvm and qemu.  The result is always ok video during BIOS post, but garbage
>> during pre and post X.  Would this be a symptom of the 'VBE' issue that
>> you mentioned?
>>
>
> Try the latest qemu git.  vmware vga should work there.
>
> I'm not sure about qemu-kvm.  It depends on whether the changes survived the
> merge.
>

Yes, this is one of the combinations I tried.  Latest qemu from git with your
two patches applied.

Do you have another distro which you might recommend trying?  (Although,
I'm not too confident this would be the problem.)

> Regards,
>
> Anthony Liguori
>

Re: [Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Anthony Liguori]

Jordan Justen wrote:
> Yes, this is one of the combinations I tried.  Latest qemu from git with your
> two patches applied.
>   

Shouldn't need any patches.  I'll try to give it a try in the near future.


-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 2/2] Make sure to enable dirty log tracking for VMware VGA

[Avi Kivity]

(redirecting)

Nolan wrote:
> Anthony Liguori <aliguori <at> us.ibm.com> writes:
>   
>>  
>> This is needed for VMware VGA to work properly under KVM.
>>
>> Signed-off-by: Anthony Liguori <aliguori <at> us.ibm.com>
>> ---
>>  hw/vmware_vga.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>> index bb17698..246011b 100644
>> --- a/hw/vmware_vga.c
>> +++ b/hw/vmware_vga.c
>> @@ -1203,6 +1203,10 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev, int
>>     
> region_num,
>   
>>  #endif
>>      cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
>>                      iomemtype);
>> +
>> +    s->vga.map_addr = addr;
>> +    s->vga.map_end = addr + s->vga.vram_size;
>> +    vga_dirty_log_start(&s->vga);
>>  }
>>
>>  void pci_vmsvga_init(PCIBus *bus)
>>     
>
> Using the MMU to track writes to VRAM should only be needed when the FIFO is
> disabled.  When the FIFO is enabled (SVGA_REG_CONFIG_DONE = 1), the guest will
> notify the host via the FIFO when it has written to a region of VRAM directly,
> using the SVGA_CMD_UPDATE FIFO command.
>
> Unless I'm confused, this change appears to use the MMU to track VRAM updates
> unconditionally.
>   


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.