[PATCH] Don't explicitly set BAR values for VMware VGA

[PATCH] Don't explicitly set BAR values for VMware VGA

[Anthony Liguori]

Right now we set explict base addresses for the PCI IO regions in the VMware
VGA device.  We don't register the second region at all and instead directly
map the physical memory.

The problem is, the addresses we're setting in the BAR is not taken into
account in the e820 mapping.

This patch removes the explicit BARs and registers the second region through
the normal PCI code.

I've only tested with a Linux guest and the open source VMware VGA driver.

This patch needs -p2 to apply against the QEMU CVS tree.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/vmware_vga.c b/qemu/hw/vmware_vga.c
index bd96e6b..ec7b1cd 100644
--- a/qemu/hw/vmware_vga.c
+++ b/qemu/hw/vmware_vga.c
@@ -60,6 +60,8 @@ struct vmsvga_state_s {
     int vram_size;
 #endif
     uint8_t *vram;
+    uint32_t vram_addr;
+    int iomemtype;
 
     int index;
     int scratch_size;
@@ -644,7 +646,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
         return ((s->depth + 7) >> 3) * s->new_width;
 
     case SVGA_REG_FB_START:
-        return SVGA_MEM_BASE;
+        return s->vram_addr;
 
     case SVGA_REG_FB_OFFSET:
         return 0x0;
@@ -671,7 +673,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
         return caps;
 
     case SVGA_REG_MEM_START:
-        return SVGA_MEM_BASE + s->vram_size - SVGA_FIFO_SIZE;
+        return s->vram_addr + s->vram_size - SVGA_FIFO_SIZE;
 
     case SVGA_REG_MEM_SIZE:
         return SVGA_FIFO_SIZE;
@@ -953,7 +955,7 @@ static void vmsvga_screen_dump(void *opaque, const char *filename)
 static uint32_t vmsvga_vram_readb(void *opaque, target_phys_addr_t addr)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         return *(uint8_t *) (s->ds->data + addr);
     else
@@ -963,7 +965,7 @@ static uint32_t vmsvga_vram_readb(void *opaque, target_phys_addr_t addr)
 static uint32_t vmsvga_vram_readw(void *opaque, target_phys_addr_t addr)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         return *(uint16_t *) (s->ds->data + addr);
     else
@@ -973,7 +975,7 @@ static uint32_t vmsvga_vram_readw(void *opaque, target_phys_addr_t addr)
 static uint32_t vmsvga_vram_readl(void *opaque, target_phys_addr_t addr)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         return *(uint32_t *) (s->ds->data + addr);
     else
@@ -984,7 +986,7 @@ static void vmsvga_vram_writeb(void *opaque, target_phys_addr_t addr,
                 uint32_t value)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         *(uint8_t *) (s->ds->data + addr) = value;
     else
@@ -995,7 +997,7 @@ static void vmsvga_vram_writew(void *opaque, target_phys_addr_t addr,
                 uint32_t value)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         *(uint16_t *) (s->ds->data + addr) = value;
     else
@@ -1006,7 +1008,7 @@ static void vmsvga_vram_writel(void *opaque, target_phys_addr_t addr,
                 uint32_t value)
 {
     struct vmsvga_state_s *s = (struct vmsvga_state_s *) opaque;
-    addr -= SVGA_MEM_BASE;
+    addr -= s->vram_addr;
     if (addr < s->fb_size)
         *(uint32_t *) (s->ds->data + addr) = value;
     else
@@ -1081,10 +1083,10 @@ static void vmsvga_init(struct vmsvga_state_s *s, DisplayState *ds,
                 uint8_t *vga_ram_base, unsigned long vga_ram_offset,
                 int vga_ram_size)
 {
-    int iomemtype;
     s->ds = ds;
     s->vram = vga_ram_base;
     s->vram_size = vga_ram_size;
+    s->vram_addr = 0;
 
     s->scratch_size = SVGA_SCRATCH_SIZE;
     s->scratch = (uint32_t *) qemu_malloc(s->scratch_size * 4);
@@ -1092,13 +1094,11 @@ static void vmsvga_init(struct vmsvga_state_s *s, DisplayState *ds,
     vmsvga_reset(s);
 
 #ifdef DIRECT_VRAM
-    iomemtype = cpu_register_io_memory(0, vmsvga_vram_read,
-                    vmsvga_vram_write, s);
+    s->iomemtype = cpu_register_io_memory(0, vmsvga_vram_read,
+					  vmsvga_vram_write, s);
 #else
-    iomemtype = vga_ram_offset | IO_MEM_RAM;
+    s->iomemtype = vga_ram_offset | IO_MEM_RAM;
 #endif
-    cpu_register_physical_memory(SVGA_MEM_BASE, vga_ram_size,
-                    iomemtype);
 
     graphic_console_init(ds, vmsvga_update_display,
                     vmsvga_invalidate_display, vmsvga_screen_dump, s);
@@ -1133,24 +1133,30 @@ static int pci_vmsvga_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static void pci_vmsvga_map_ioport(PCIDevice *pci_dev, int region_num,
+static void pci_vmsvga_map(PCIDevice *pci_dev, int region_num,
                 uint32_t addr, uint32_t size, int type)
 {
     struct pci_vmsvga_state_s *d = (struct pci_vmsvga_state_s *) pci_dev;
     struct vmsvga_state_s *s = &d->chip;
 
-    register_ioport_read(addr + SVGA_IO_MUL * SVGA_INDEX_PORT,
-                    1, 4, vmsvga_index_read, s);
-    register_ioport_write(addr + SVGA_IO_MUL * SVGA_INDEX_PORT,
-                    1, 4, vmsvga_index_write, s);
-    register_ioport_read(addr + SVGA_IO_MUL * SVGA_VALUE_PORT,
-                    1, 4, vmsvga_value_read, s);
-    register_ioport_write(addr + SVGA_IO_MUL * SVGA_VALUE_PORT,
-                    1, 4, vmsvga_value_write, s);
-    register_ioport_read(addr + SVGA_IO_MUL * SVGA_BIOS_PORT,
-                    1, 4, vmsvga_bios_read, s);
-    register_ioport_write(addr + SVGA_IO_MUL * SVGA_BIOS_PORT,
-                    1, 4, vmsvga_bios_write, s);
+    if (region_num == 0) {
+	register_ioport_read(addr + SVGA_IO_MUL * SVGA_INDEX_PORT,
+			     1, 4, vmsvga_index_read, s);
+	register_ioport_write(addr + SVGA_IO_MUL * SVGA_INDEX_PORT,
+			      1, 4, vmsvga_index_write, s);
+	register_ioport_read(addr + SVGA_IO_MUL * SVGA_VALUE_PORT,
+			     1, 4, vmsvga_value_read, s);
+	register_ioport_write(addr + SVGA_IO_MUL * SVGA_VALUE_PORT,
+			      1, 4, vmsvga_value_write, s);
+	register_ioport_read(addr + SVGA_IO_MUL * SVGA_BIOS_PORT,
+			     1, 4, vmsvga_bios_read, s);
+	register_ioport_write(addr + SVGA_IO_MUL * SVGA_BIOS_PORT,
+			      1, 4, vmsvga_bios_write, s);
+    } else {
+        cpu_register_physical_memory(addr, s->vram_size, s->iomemtype);
+	s->vram_addr = addr;
+	s->vram = s->vram_ptr;
+    }
 }
 
 #define PCI_VENDOR_ID_VMWARE		0x15ad
@@ -1182,14 +1188,6 @@ void pci_vmsvga_init(PCIBus *bus, DisplayState *ds, uint8_t *vga_ram_base,
     s->card.config[0x0c]		= 0x08;		/* Cache line size */
     s->card.config[0x0d]		= 0x40;		/* Latency timer */
     s->card.config[0x0e]		= PCI_CLASS_HEADERTYPE_00h;
-    s->card.config[0x10]		= ((SVGA_IO_BASE >>  0) & 0xff) | 1;
-    s->card.config[0x11]		=  (SVGA_IO_BASE >>  8) & 0xff;
-    s->card.config[0x12]		=  (SVGA_IO_BASE >> 16) & 0xff;
-    s->card.config[0x13]		=  (SVGA_IO_BASE >> 24) & 0xff;
-    s->card.config[0x18]		= (SVGA_MEM_BASE >>  0) & 0xff;
-    s->card.config[0x19]		= (SVGA_MEM_BASE >>  8) & 0xff;
-    s->card.config[0x1a]		= (SVGA_MEM_BASE >> 16) & 0xff;
-    s->card.config[0x1b]		= (SVGA_MEM_BASE >> 24) & 0xff;
     s->card.config[0x2c]		= PCI_VENDOR_ID_VMWARE & 0xff;
     s->card.config[0x2d]		= PCI_VENDOR_ID_VMWARE >> 8;
     s->card.config[0x2e]		= SVGA_PCI_DEVICE_ID & 0xff;
@@ -1197,7 +1195,10 @@ void pci_vmsvga_init(PCIBus *bus, DisplayState *ds, uint8_t *vga_ram_base,
     s->card.config[0x3c]		= 0xff;		/* End */
 
     pci_register_io_region(&s->card, 0, 0x10,
-                    PCI_ADDRESS_SPACE_IO, pci_vmsvga_map_ioport);
+			   PCI_ADDRESS_SPACE_IO, pci_vmsvga_map);
+
+    pci_register_io_region(&s->card, 1, vga_ram_size,
+			   PCI_ADDRESS_SPACE_MEM, pci_vmsvga_map);
 
     vmsvga_init(&s->chip, ds, vga_ram_base, vga_ram_offset, vga_ram_size);
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 1/3] Move common VGAState attributes to VGA_STATE_COMMON

[Anthony Liguori]

vmware_vga.c uses functions in vga.c to do some things. They
need to agree on which parts of their state struct is common
and which aren't, otherwise they'll overwrite parts of each
other's state. This patch makes it so.

Signed-off-by: Soren Hansen <soren@ubuntu.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
index 1915c73..35cee6b 100644
--- a/qemu/hw/cirrus_vga.c
+++ b/qemu/hw/cirrus_vga.c
@@ -240,9 +240,6 @@ typedef struct CirrusVGAState {
     int cirrus_mmio_io_addr;
     unsigned long cirrus_lfb_addr;
     unsigned long cirrus_lfb_end;
-    int aliases_enabled;
-    uint32_t aliased_bank_base[2];
-    uint32_t aliased_bank_limit[2];
     uint32_t cirrus_addr_mask;
     uint32_t linear_mmio_mask;
     uint8_t cirrus_shadow_gr0;
diff --git a/qemu/hw/vga_int.h b/qemu/hw/vga_int.h
index 912d977..196f8df 100644
--- a/qemu/hw/vga_int.h
+++ b/qemu/hw/vga_int.h
@@ -147,18 +147,14 @@
     uint32_t last_palette[256];                                         \
     uint32_t last_ch_attr[CH_ATTR_SIZE]; /* XXX: make it dynamic */	\
     unsigned long map_addr;						\
-    unsigned long map_end;
+    unsigned long map_end;						\
+    int aliases_enabled;						\
+    uint32_t aliased_bank_base[2];					\
+    uint32_t aliased_bank_limit[2];
 
 
 typedef struct VGAState {
     VGA_STATE_COMMON
-
-    int32_t  aliases_enabled;
-    int32_t  pad1;
-    uint32_t aliased_bank_base[2];
-    uint32_t aliased_bank_limit[2];
-
-
 } VGAState;
 
 static inline int c6_to_8(int v)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 2/3] Factor out the VGA vram mapping updating routine

[Anthony Liguori]

This function is useful for enabling KVM support in VMware VGA.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/vga.c b/qemu/hw/vga.c
index 222a39c..1cfc154 100644
--- a/qemu/hw/vga.c
+++ b/qemu/hw/vga.c
@@ -1813,6 +1813,36 @@ typedef struct PCIVGAState {
     VGAState vga_state;
 } PCIVGAState;
 
+void vga_update_vram_mapping(VGAState *s, unsigned long vga_ram_begin,
+			     unsigned long vga_ram_end)
+{
+    void *vram_pointer, *old_vram;
+
+    if (vga_ram_begin == s->map_addr &&
+	vga_ram_end   == s->map_end) {
+	return;
+    }
+
+    if (s->map_addr && s->map_end)
+	unset_vram_mapping(s->map_addr, s->map_end);
+    
+    vram_pointer = set_vram_mapping(vga_ram_begin, vga_ram_end);
+    if (!vram_pointer) {
+	fprintf(stderr, "set_vram_mapping failed\n");
+	s->map_addr = s->map_end = 0;
+    }
+    else {
+	old_vram = vga_update_vram((VGAState *)s, vram_pointer,
+				   VGA_RAM_SIZE);
+	if (s->map_addr && s->map_end)
+	    munmap(old_vram, s->map_end - s->map_addr);
+	else
+	    qemu_free(old_vram);
+	s->map_addr = vga_ram_begin;
+	s->map_end  = vga_ram_end;
+    }
+}
+
 static void vga_map(PCIDevice *pci_dev, int region_num,
                     uint32_t addr, uint32_t size, int type)
 {
@@ -1822,37 +1852,8 @@ static void vga_map(PCIDevice *pci_dev, int region_num,
         cpu_register_physical_memory(addr, s->bios_size, s->bios_offset);
     } else {
         cpu_register_physical_memory(addr, s->vram_size, s->vram_offset);
-        if (kvm_enabled()) {
-            unsigned long vga_ram_begin, vga_ram_end;
-            void *vram_pointer, *old_vram;
-
-            vga_ram_begin = addr;
-            vga_ram_end   = addr + VGA_RAM_SIZE;
-
-            if (vga_ram_begin == s->map_addr &&
-                vga_ram_end   == s->map_end) {
-                return;
-            }
-
-            if (s->map_addr && s->map_end)
-                unset_vram_mapping(s->map_addr, s->map_end);
-
-            vram_pointer = set_vram_mapping(vga_ram_begin, vga_ram_end);
-            if (!vram_pointer) {
-                fprintf(stderr, "set_vram_mapping failed\n");
-                s->map_addr = s->map_end = 0;
-            }
-            else {
-                old_vram = vga_update_vram((VGAState *)s, vram_pointer,
-                                           VGA_RAM_SIZE);
-                if (s->map_addr && s->map_end)
-                    munmap(old_vram, s->map_end - s->map_addr);
-                else
-                    qemu_free(old_vram);
-                s->map_addr = vga_ram_begin;
-                s->map_end  = vga_ram_end;
-            }
-        }
+	if (kvm_enabled())
+	    vga_update_vram_mapping(s, addr, addr + VGA_RAM_SIZE);
     }
 }
 
diff --git a/qemu/hw/vga_int.h b/qemu/hw/vga_int.h
index 196f8df..8d29205 100644
--- a/qemu/hw/vga_int.h
+++ b/qemu/hw/vga_int.h
@@ -192,5 +192,7 @@ void *set_vram_mapping(unsigned long begin, unsigned long end);
 int unset_vram_mapping(unsigned long begin, unsigned long end);
 
 void *vga_update_vram(VGAState *s, void *vga_ram_base, int vga_ram_size);
+void vga_update_vram_mapping(VGAState *s, unsigned long vga_ram_begin,
+			     unsigned long vga_ram_end);
 extern const uint8_t sr_mask[8];
 extern const uint8_t gr_mask[16];

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH 3/3] Enable VGA optimization for VMware VGA

[Anthony Liguori]

After the previous patches, this patch is pretty straight forward.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/vmware_vga.c b/qemu/hw/vmware_vga.c
index ec7b1cd..f2a298e 100644
--- a/qemu/hw/vmware_vga.c
+++ b/qemu/hw/vmware_vga.c
@@ -36,6 +36,9 @@
 # include "vga_int.h"
 #endif
 
+#include "qemu-kvm.h"
+#include "pc.h"
+
 struct vmsvga_state_s {
 #ifdef EMBED_STDVGA
     VGA_STATE_COMMON
@@ -1155,6 +1158,8 @@ static void pci_vmsvga_map(PCIDevice *pci_dev, int region_num,
     } else {
         cpu_register_physical_memory(addr, s->vram_size, s->iomemtype);
 	s->vram_addr = addr;
+        if (kvm_enabled())
+	    vga_update_vram_mapping((VGAState *)s, addr, addr + VGA_RAM_SIZE);
 	s->vram = s->vram_ptr;
     }
 }

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[Anthony Liguori]

Anthony Liguori wrote:
> Right now we set explict base addresses for the PCI IO regions in the VMware
> VGA device.  We don't register the second region at all and instead directly
> map the physical memory.
>
> The problem is, the addresses we're setting in the BAR is not taken into
> account in the e820 mapping.
>   

Actually, it looks like the e820 map is okay, but I do get some errors 
in the gust about remapping that range so I do think this patch is correct.

Regards,

Anthony Liguori

> This patch removes the explicit BARs and registers the second region through
> the normal PCI code.
>
> I've only tested with a Linux guest and the open source VMware VGA driver.
>
> This patch needs -p2 to apply against the QEMU CVS tree.
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[andrzej zaborowski]

On 22/02/2008, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Right now we set explict base addresses for the PCI IO regions in the VMware
>  VGA device.  We don't register the second region at all and instead directly
>  map the physical memory.
>
>  The problem is, the addresses we're setting in the BAR is not taken into
>  account in the e820 mapping.
>
>  This patch removes the explicit BARs and registers the second region through
>  the normal PCI code.
>
>  I've only tested with a Linux guest and the open source VMware VGA driver.

I have a very similar patch on my HD but I haven't included it because
it causes my testing Ms Windows install to stop detecting the card.  I
just tested your patch and the same thing happens, i.e. with the patch
it works as a vga card but is detected as an Unknown adapter and only
the 640x480x8 mode can be used.  I can't explain this.

Currently the io port numbers can be set by the guest and the memory
io regions are fixed.  Earlier both settings were hardcoded.  This is
because in one version of the X driver (which was/is the only
documentation available) these settings were metioned as *the* correct
values for this card.  This may of course cause different types of
breakage but so far worked ok, except when it was found that in
various combinations qemu segfaulted due to different PCI cards
registering the same port range as our default.  When this happened I
tried to make these settings settable through PCI registers but found
that this broke Ms Windows.  Luckily Ms Windows still worked if only
port ranges were assigned dynamically and the segfault went away so I
left it at this but it perhaps needs better looking at.
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[Anthony Liguori]

andrzej zaborowski wrote:
> On 22/02/2008, Anthony Liguori <aliguori@us.ibm.com> wrote:
>   
>> Right now we set explict base addresses for the PCI IO regions in the VMware
>>  VGA device.  We don't register the second region at all and instead directly
>>  map the physical memory.
>>
>>  The problem is, the addresses we're setting in the BAR is not taken into
>>  account in the e820 mapping.
>>
>>  This patch removes the explicit BARs and registers the second region through
>>  the normal PCI code.
>>
>>  I've only tested with a Linux guest and the open source VMware VGA driver.
>>     
>
> I have a very similar patch on my HD but I haven't included it because
> it causes my testing Ms Windows install to stop detecting the card.  I
> just tested your patch and the same thing happens, i.e. with the patch
> it works as a vga card but is detected as an Unknown adapter and only
> the 640x480x8 mode can be used.  I can't explain this.
>   

Can you describe how you setup your Windows install?  I'll try to 
reproduce it and dig into it.

Regards,

Anthony Liguori

> Currently the io port numbers can be set by the guest and the memory
> io regions are fixed.  Earlier both settings were hardcoded.  This is
> because in one version of the X driver (which was/is the only
> documentation available) these settings were metioned as *the* correct
> values for this card.  This may of course cause different types of
> breakage but so far worked ok, except when it was found that in
> various combinations qemu segfaulted due to different PCI cards
> registering the same port range as our default.  When this happened I
> tried to make these settings settable through PCI registers but found
> that this broke Ms Windows.  Luckily Ms Windows still worked if only
> port ranges were assigned dynamically and the segfault went away so I
> left it at this but it perhaps needs better looking at.
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[andrzej zaborowski]

On 23/02/2008, Anthony Liguori <aliguori@us.ibm.com> wrote:
> andrzej zaborowski wrote:
>  > I have a very similar patch on my HD but I haven't included it because
>  > it causes my testing Ms Windows install to stop detecting the card.  I
>  > just tested your patch and the same thing happens, i.e. with the patch
>  > it works as a vga card but is detected as an Unknown adapter and only
>  > the 640x480x8 mode can be used.  I can't explain this.
>  >
>
>
> Can you describe how you setup your Windows install?  I'll try to
>  reproduce it and dig into it.

Oh, good question, and I think the answer be the reason why it's not
working here (*slaps self*).  I'll apply the patch if you can confirm
that it works with some Ms Windows install.

I just launched the VM and checked what kind of Ms Windows set up this
is and it's a "Windows XP professional 2002" with VMware Tools
installed, and all the files on it have last modification date in 2004
or earlier.  Apparently I stole the already set up VM from my dad's
computer on which he used VMware, somewhere in 2004.  I then converted
the image to raw and always performed my tests with -snapshot on.
This may be why the system is unwilling to use different base
addresses.
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]

andrzej zaborowski wrote:
> Oh, good question, and I think the answer be the reason why it's not
> working here (*slaps self*).  I'll apply the patch if you can confirm
> that it works with some Ms Windows install.
>   

I just tried with Windows XP and I have no problem detecting the card 
with this patch.

FWIW, I needed to use the following patch to avoid SEGVs as it seems 
there's a bug in the emulation where the update region is larger than 
the screen.  My first impression is that it's related to cursor drawing 
as it only happens when I click on the start button and the update 
region height is 36 pixels which looks like a cursor size to me.  This 
is true with or without the BAR patch though.  I'll look into it a 
little more and see what's going on.

Regards,

Anthony Liguori

> I just launched the VM and checked what kind of Ms Windows set up this
> is and it's a "Windows XP professional 2002" with VMware Tools
> installed, and all the files on it have last modification date in 2004
> or earlier.  Apparently I stole the already set up VM from my dad's
> computer on which he used VMware, somewhere in 2004.  I then converted
> the image to raw and always performed my tests with -snapshot on.
> This may be why the system is unwilling to use different base
> addresses.
>   


[-- Attachment #2: qemu-vmware-check-update.patch --]
[-- Type: text/x-diff, Size: 1193 bytes --]

diff --git a/qemu/hw/vmware_vga.c b/qemu/hw/vmware_vga.c
index f2a298e..0204d88 100644
--- a/qemu/hw/vmware_vga.c
+++ b/qemu/hw/vmware_vga.c
@@ -295,12 +295,31 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
                 int x, int y, int w, int h)
 {
 #ifndef DIRECT_VRAM
-    int line = h;
-    int bypl = s->bypp * s->width;
-    int width = s->bypp * w;
-    int start = s->bypp * x + bypl * y;
-    uint8_t *src = s->vram + start;
-    uint8_t *dst = s->ds->data + start;
+    int line;
+    int bypl;
+    int width;
+    int start;
+    uint8_t *src;
+    uint8_t *dst;
+
+    if ((x + w) > s->ds->width) {
+	fprintf(stderr, "update width too large x: %d, w: %d\n", x, w);
+	x = MIN(x, s->ds->width);
+	w = s->ds->width - x;
+    }
+
+    if ((y + h) > s->ds->height) {
+	fprintf(stderr, "update height too large y: %d, h: %d\n", y, h);
+	y = MIN(y, s->ds->height);
+	h = s->ds->height - y;
+    }
+
+    line = h;
+    bypl = s->bypp * s->width;
+    width = s->bypp * w;
+    start = s->bypp * x + bypl * y;
+    src = s->vram + start;
+    dst = s->ds->data + start;
 
     for (; line > 0; line --, src += bypl, dst += bypl)
         memcpy(dst, src, width);

[-- Attachment #3: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] Don't explicitly set BAR values for VMware VGA

[Avi Kivity]

Anthony Liguori wrote:
> Right now we set explict base addresses for the PCI IO regions in the VMware
> VGA device.  We don't register the second region at all and instead directly
> map the physical memory.
>
> The problem is, the addresses we're setting in the BAR is not taken into
> account in the e820 mapping.
>
> This patch removes the explicit BARs and registers the second region through
> the normal PCI code.
>
> I've only tested with a Linux guest and the open source VMware VGA driver.
>
> This patch needs -p2 to apply against the QEMU CVS tree.
>
>   

Since it seems like qemu will apply this soon, I'll pick it up via the 
next merge.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/3] Move common VGAState attributes to VGA_STATE_COMMON

[Avi Kivity]

Anthony Liguori wrote:
> vmware_vga.c uses functions in vga.c to do some things. They
> need to agree on which parts of their state struct is common
> and which aren't, otherwise they'll overwrite parts of each
> other's state. This patch makes it so.
>
> Signed-off-by: Soren Hansen <soren@ubuntu.com>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>
>   

The trick to passing through patches is to have a

From: Original Author <email@example.com>

line in the beginning, which git picks up and uses to maintain 
authorship information. Also, sign-off normally. Acked-by means "I know 
the author" while Reviewed-by means "I know the author well".

Anyway I applied all these patches, including the vmware BAR patch since 
it's a dependency.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/3] Move common VGAState attributes to VGA_STATE_COMMON

[Anthony Liguori]

Avi Kivity wrote:
> The trick to passing through patches is to have a
>
> From: Original Author <email@example.com>
>
> line in the beginning, which git picks up and uses to maintain 
> authorship information. Also, sign-off normally. Acked-by means "I know 
> the author" while Reviewed-by means "I know the author well".
>   

Does From have to be the first line?  git-send-email never seems to do 
anything with From for me.

Regards,

Anthony Liguori

> Anyway I applied all these patches, including the vmware BAR patch since 
> it's a dependency.
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/3] Move common VGAState attributes to VGA_STATE_COMMON

[Avi Kivity]

Anthony Liguori wrote:
> Avi Kivity wrote:
>   
>> The trick to passing through patches is to have a
>>
>> From: Original Author <email@example.com>
>>
>> line in the beginning, which git picks up and uses to maintain 
>> authorship information. Also, sign-off normally. Acked-by means "I know 
>> the author" while Reviewed-by means "I know the author well".
>>   
>>     
>
> Does From have to be the first line?  

Yes (but after the regular email headers).

> git-send-email never seems to do 
> anything with From for me.
>   

It's 'git am' which is the target of this.  When applying such a patch 
from email, it will set the author automatically.

The whole thing should work automatically, so if the patch has the 
correct authorship in your repo, it should come out fine without further 
effort.  Not sure if format-patch or send-email is responsible.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/