1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks
  4. View patch

[libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga

Thomas Huth posted 4 patches 6 years, 2 months ago
[libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga
Posted by Thomas Huth 6 years, 2 months ago 
Now that the old pc-0.x machine types have been removed, we do not need
the old "rombar" hacks anymore.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/display/vga-pci.c    | 5 -----
 hw/display/vga.c        | 4 +---
 hw/display/vmware_vga.c | 5 -----
 3 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index a27b88122d..cfe095713e 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -264,11 +264,6 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
 
         pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
     }
-
-    if (!dev->rom_bar) {
-        /* compatibility with pc-0.13 and older */
-        vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
-    }
 }
 
 static void pci_std_vga_init(Object *obj)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 82ebe53610..636586a476 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2304,9 +2304,7 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
 
 void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *system_memory)
 {
-    /* With pc-0.12 and below we map both the PCI BAR and the fixed VBE region,
-     * so use an alias to avoid double-mapping the same region.
-     */
+    /* Use an alias to avoid double-mapping the same region */
     memory_region_init_alias(&s->vram_vbe, obj, "vram.vbe",
                              &s->vram, 0, memory_region_size(&s->vram));
     /* XXX: use optimized standard vga accesses */
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 23dc8910cc..ead754eccf 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1312,11 +1312,6 @@ static void pci_vmsvga_realize(PCIDevice *dev, Error **errp)
                      &s->chip.vga.vram);
     pci_register_bar(dev, 2, PCI_BASE_ADDRESS_MEM_PREFETCH,
                      &s->chip.fifo_ram);
-
-    if (!dev->rom_bar) {
-        /* compatibility with pc-0.13 and older */
-        vga_init_vbe(&s->chip.vga, OBJECT(dev), pci_address_space(dev));
-    }
 }
 
 static Property vga_vmware_properties[] = {
-- 
2.18.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga
Posted by Philippe Mathieu-Daudé 6 years, 2 months ago 
On 12/9/19 1:52 PM, Thomas Huth wrote:
> Now that the old pc-0.x machine types have been removed, we do not need
> the old "rombar" hacks anymore.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/display/vga-pci.c    | 5 -----
>   hw/display/vga.c        | 4 +---
>   hw/display/vmware_vga.c | 5 -----
>   3 files changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> index a27b88122d..cfe095713e 100644
> --- a/hw/display/vga-pci.c
> +++ b/hw/display/vga-pci.c
> @@ -264,11 +264,6 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
>   
>           pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
>       }
> -
> -    if (!dev->rom_bar) {
> -        /* compatibility with pc-0.13 and older */
> -        vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
> -    }
>   }
>   
>   static void pci_std_vga_init(Object *obj)
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 82ebe53610..636586a476 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2304,9 +2304,7 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>   
>   void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *system_memory)
>   {
> -    /* With pc-0.12 and below we map both the PCI BAR and the fixed VBE region,
> -     * so use an alias to avoid double-mapping the same region.
> -     */
> +    /* Use an alias to avoid double-mapping the same region */

If I understand the comment correctly, we can now revert commit 
8294a64d7f9 and and remove the alias, isn't it?

>       memory_region_init_alias(&s->vram_vbe, obj, "vram.vbe",
>                                &s->vram, 0, memory_region_size(&s->vram));
>       /* XXX: use optimized standard vga accesses */
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 23dc8910cc..ead754eccf 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -1312,11 +1312,6 @@ static void pci_vmsvga_realize(PCIDevice *dev, Error **errp)
>                        &s->chip.vga.vram);
>       pci_register_bar(dev, 2, PCI_BASE_ADDRESS_MEM_PREFETCH,
>                        &s->chip.fifo_ram);
> -
> -    if (!dev->rom_bar) {
> -        /* compatibility with pc-0.13 and older */
> -        vga_init_vbe(&s->chip.vga, OBJECT(dev), pci_address_space(dev));
> -    }
>   }
>   
>   static Property vga_vmware_properties[] = {
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga
Posted by Gerd Hoffmann 6 years, 2 months ago 
On Mon, Dec 09, 2019 at 02:12:35PM +0100, Philippe Mathieu-Daudé wrote:
> On 12/9/19 1:52 PM, Thomas Huth wrote:
> > Now that the old pc-0.x machine types have been removed, we do not need
> > the old "rombar" hacks anymore.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   hw/display/vga-pci.c    | 5 -----
> >   hw/display/vga.c        | 4 +---
> >   hw/display/vmware_vga.c | 5 -----
> >   3 files changed, 1 insertion(+), 13 deletions(-)
> > 
> > diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> > index a27b88122d..cfe095713e 100644
> > --- a/hw/display/vga-pci.c
> > +++ b/hw/display/vga-pci.c
> > @@ -264,11 +264,6 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
> >           pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
> >       }
> > -
> > -    if (!dev->rom_bar) {
> > -        /* compatibility with pc-0.13 and older */
> > -        vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
> > -    }
> >   }
> >   static void pci_std_vga_init(Object *obj)
> > diff --git a/hw/display/vga.c b/hw/display/vga.c
> > index 82ebe53610..636586a476 100644
> > --- a/hw/display/vga.c
> > +++ b/hw/display/vga.c
> > @@ -2304,9 +2304,7 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
> >   void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *system_memory)
> >   {
> > -    /* With pc-0.12 and below we map both the PCI BAR and the fixed VBE region,
> > -     * so use an alias to avoid double-mapping the same region.
> > -     */
> > +    /* Use an alias to avoid double-mapping the same region */
> 
> If I understand the comment correctly, we can now revert commit 8294a64d7f9
> and and remove the alias, isn't it?

Hmm, yes, I think so given that only isa-vga calls vga_init_vbe().

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga
Posted by Paolo Bonzini 6 years, 2 months ago 
On 09/12/19 14:12, Philippe Mathieu-Daudé wrote:
> On 12/9/19 1:52 PM, Thomas Huth wrote:
>> Now that the old pc-0.x machine types have been removed, we do not need
>> the old "rombar" hacks anymore.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   hw/display/vga-pci.c    | 5 -----
>>   hw/display/vga.c        | 4 +---
>>   hw/display/vmware_vga.c | 5 -----
>>   3 files changed, 1 insertion(+), 13 deletions(-)
>>
>> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
>> index a27b88122d..cfe095713e 100644
>> --- a/hw/display/vga-pci.c
>> +++ b/hw/display/vga-pci.c
>> @@ -264,11 +264,6 @@ static void pci_std_vga_realize(PCIDevice *dev,
>> Error **errp)
>>             pci_register_bar(&d->dev, 2,
>> PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
>>       }
>> -
>> -    if (!dev->rom_bar) {
>> -        /* compatibility with pc-0.13 and older */
>> -        vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
>> -    }
>>   }
>>     static void pci_std_vga_init(Object *obj)
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index 82ebe53610..636586a476 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -2304,9 +2304,7 @@ void vga_init(VGACommonState *s, Object *obj,
>> MemoryRegion *address_space,
>>     void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion
>> *system_memory)
>>   {
>> -    /* With pc-0.12 and below we map both the PCI BAR and the fixed
>> VBE region,
>> -     * so use an alias to avoid double-mapping the same region.
>> -     */
>> +    /* Use an alias to avoid double-mapping the same region */
> 
> If I understand the comment correctly, we can now revert commit
> 8294a64d7f9 and and remove the alias, isn't it?

Yes, even inline vga_init_vbe and remove vbe_mapped.

----------------- 8< ----------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] vga: cleanup mapping of VRAM for non-PCI VGA

vga_init_vbe is now used only from ISA VGA cards.  Since the alias is
not needed anymore, remove it (effectively reverting commit 8294a64d7f,
"vga: fix vram double-mapping with -vga std and -M pc-0.12", 2012-05-29)
and the new unused vbe_mapped field of VGACommonState.  The function now
consists of a single memory_region_add_subregion call, so we can inline
it and avoid incorrect usage from PCI cards.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
index e9c43e5530..06f93c4ef5 100644
--- a/hw/display/vga-isa-mm.c
+++ b/hw/display/vga-isa-mm.c
@@ -106,6 +106,9 @@ int isa_vga_mm_init(hwaddr vram_base,
 
     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
 
-    vga_init_vbe(&s->vga, NULL, address_space);
+    memory_region_add_subregion(system_memory,
+                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
+                                &s->vram);
+
     return 0;
 }
diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 873e5e9706..5b0b567835 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -76,7 +76,9 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     memory_region_set_coalescing(vga_io_memory);
     s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s);
 
-    vga_init_vbe(s, OBJECT(dev), isa_address_space(isadev));
+    memory_region_add_subregion(system_memory,
+                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
+                                &s->vram);
     /* ROM BIOS */
     rom_add_vga(VGABIOS_FILENAME);
 }
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 636586a476..061fd9ab8f 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2301,15 +2301,3 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
         portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
     }
 }
-
-void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *system_memory)
-{
-    /* Use an alias to avoid double-mapping the same region */
-    memory_region_init_alias(&s->vram_vbe, obj, "vram.vbe",
-                             &s->vram, 0, memory_region_size(&s->vram));
-    /* XXX: use optimized standard vga accesses */
-    memory_region_add_subregion(system_memory,
-                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
-                                &s->vram_vbe);
-    s->vbe_mapped = 1;
-}
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 55c418eab5..3081be445d 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -106,7 +106,6 @@ typedef struct VGACommonState {
     uint32_t vbe_start_addr;
     uint32_t vbe_line_offset;
     uint32_t vbe_bank_mask;
-    int vbe_mapped;
     /* display refresh support */
     QemuConsole *con;
     uint32_t font_offsets[2];
@@ -178,7 +177,6 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
 
 int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
 
-void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *address_space);
 uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr);
 void vbe_ioport_write_index(void *opaque, uint32_t addr, uint32_t val);
 void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val);


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] hw/display: Remove "rombar" hack from vga-pci and vmware_vga
Posted by Thomas Huth 6 years, 2 months ago 
On 09/12/2019 14.30, Paolo Bonzini wrote:
> On 09/12/19 14:12, Philippe Mathieu-Daudé wrote:
>> On 12/9/19 1:52 PM, Thomas Huth wrote:
>>> Now that the old pc-0.x machine types have been removed, we do not need
>>> the old "rombar" hacks anymore.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   hw/display/vga-pci.c    | 5 -----
>>>   hw/display/vga.c        | 4 +---
>>>   hw/display/vmware_vga.c | 5 -----
>>>   3 files changed, 1 insertion(+), 13 deletions(-)
>>>
>>> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
>>> index a27b88122d..cfe095713e 100644
>>> --- a/hw/display/vga-pci.c
>>> +++ b/hw/display/vga-pci.c
>>> @@ -264,11 +264,6 @@ static void pci_std_vga_realize(PCIDevice *dev,
>>> Error **errp)
>>>             pci_register_bar(&d->dev, 2,
>>> PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
>>>       }
>>> -
>>> -    if (!dev->rom_bar) {
>>> -        /* compatibility with pc-0.13 and older */
>>> -        vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
>>> -    }
>>>   }
>>>     static void pci_std_vga_init(Object *obj)
>>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>>> index 82ebe53610..636586a476 100644
>>> --- a/hw/display/vga.c
>>> +++ b/hw/display/vga.c
>>> @@ -2304,9 +2304,7 @@ void vga_init(VGACommonState *s, Object *obj,
>>> MemoryRegion *address_space,
>>>     void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion
>>> *system_memory)
>>>   {
>>> -    /* With pc-0.12 and below we map both the PCI BAR and the fixed
>>> VBE region,
>>> -     * so use an alias to avoid double-mapping the same region.
>>> -     */
>>> +    /* Use an alias to avoid double-mapping the same region */
>>
>> If I understand the comment correctly, we can now revert commit
>> 8294a64d7f9 and and remove the alias, isn't it?
> 
> Yes, even inline vga_init_vbe and remove vbe_mapped.

Sounds like a good idea!

> ----------------- 8< ----------------
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] vga: cleanup mapping of VRAM for non-PCI VGA
> 
> vga_init_vbe is now used only from ISA VGA cards.  Since the alias is
> not needed anymore, remove it (effectively reverting commit 8294a64d7f,
> "vga: fix vram double-mapping with -vga std and -M pc-0.12", 2012-05-29)
> and the new unused vbe_mapped field of VGACommonState.  The function now
> consists of a single memory_region_add_subregion call, so we can inline
> it and avoid incorrect usage from PCI cards.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
> index e9c43e5530..06f93c4ef5 100644
> --- a/hw/display/vga-isa-mm.c
> +++ b/hw/display/vga-isa-mm.c
> @@ -106,6 +106,9 @@ int isa_vga_mm_init(hwaddr vram_base,
>  
>      s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>  
> -    vga_init_vbe(&s->vga, NULL, address_space);
> +    memory_region_add_subregion(system_memory,

Use address_space instead of system_memory ?

> +                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
> +                                &s->vram);
> +
>      return 0;
>  }
> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
> index 873e5e9706..5b0b567835 100644
> --- a/hw/display/vga-isa.c
> +++ b/hw/display/vga-isa.c
> @@ -76,7 +76,9 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>      memory_region_set_coalescing(vga_io_memory);
>      s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s);
>  
> -    vga_init_vbe(s, OBJECT(dev), isa_address_space(isadev));
> +    memory_region_add_subregion(system_memory,

isa_address_space(isadev) instead of system_memory ?

> +                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
> +                                &s->vram);
>      /* ROM BIOS */
>      rom_add_vga(VGABIOS_FILENAME);
>  }
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 636586a476..061fd9ab8f 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2301,15 +2301,3 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>          portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
>      }
>  }
> -
> -void vga_init_vbe(VGACommonState *s, Object *obj, MemoryRegion *system_memory)
> -{
> -    /* Use an alias to avoid double-mapping the same region */
> -    memory_region_init_alias(&s->vram_vbe, obj, "vram.vbe",
> -                             &s->vram, 0, memory_region_size(&s->vram));
> -    /* XXX: use optimized standard vga accesses */
> -    memory_region_add_subregion(system_memory,
> -                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
> -                                &s->vram_vbe);
> -    s->vbe_mapped = 1;
> -}
> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> index 55c418eab5..3081be445d 100644
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -106,7 +106,6 @@ typedef struct VGACommonState {
>      uint32_t vbe_start_addr;
>      uint32_t vbe_line_offset;
>      uint32_t vbe_bank_mask;
> -    int vbe_mapped;
I think you can now also remove "vram_vbe" from this file.

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.