| Message ID | 20191209125248.5849-5-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove deprecated pc-0.x machine types and related hacks | expand |
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[] = { >
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);
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
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
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[] = {
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(-)