[libvirt] [PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks
These have been on the deprecation list since a year now, so it's time to finally remove the pc-0.x machine types. We then can also remove some compatibility hacks in the devices, i.e. the "use_broken_id" in ac97, the "command_serr_enable" in PCI devices and the "rombar" stuff in VGA devices. v2: - Minor updates to the first patch (fix comment, add deprecation_reason message for the pc-1.x machines) - Keep the QEMU_PCI_CAP_SERR enum in the third patch - Added fourth patch to remove the "rombar" hacks from the VGA devices Thomas Huth (4): hw/i386: Remove the deprecated machines 0.12 up to 0.15 hw/audio: Remove the "use_broken_id" hack from the AC97 device hw/pci: Remove the "command_serr_enable" property hw/display: Remove "rombar" hack from vga-pci and vmware_vga hw/audio/ac97.c | 9 ----- hw/display/vga-pci.c | 5 --- hw/display/vga.c | 4 +- hw/display/vmware_vga.c | 5 --- hw/i386/pc_piix.c | 85 +---------------------------------------- hw/pci/pci.c | 6 +-- include/hw/pci/pci.h | 2 +- qemu-deprecated.texi | 2 +- tests/cpu-plug-test.c | 6 +-- 9 files changed, 7 insertions(+), 117 deletions(-) -- 2.18.1
These machines can't be used reliably for migration anymore, quoting https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html : " due to the introduction of the memory API, the firmware is not migrated correctly from source to destination. On QEMU <1.3 the 0xf0000-0xfffff area is basically a copy of the higher 0xffff0000-0xffffffff area, while on more recent versions it is initialized with zeroes and the firmware copies from 0xffff0000 to 0xf0000. When you migrate from old to new QEMU, after reboot there's nothing at 0xf0000 and bugs ensue. " The pc-0.x machines have been marked as deprecated since QEMU v4.0, so it is time to remove them now. And while we're at it, mark the remaining pc-1.x machine types as deprecated now, too, so that we finally only have "pc-i440fx" and "pc-q35" machine types left (apart from the non-versioned "isapc" and "microvm") once we remove them in a couple of releases. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- hw/i386/pc_piix.c | 85 +------------------------------------------ qemu-deprecated.texi | 2 +- tests/cpu-plug-test.c | 6 +-- 3 files changed, 4 insertions(+), 89 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 1bd70d1abb..c0292f1793 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -357,19 +357,13 @@ static void pc_compat_1_3(MachineState *machine) pc_compat_1_4_fn(machine); } -/* PC compat function for pc-0.14 to pc-1.2 */ +/* PC compat function for pc-1.0 to pc-1.2 */ static void pc_compat_1_2(MachineState *machine) { pc_compat_1_3(machine); x86_cpu_change_kvm_default("kvm-pv-eoi", NULL); } -/* PC compat function for pc-0.12 and pc-0.13 */ -static void pc_compat_0_13(MachineState *machine) -{ - pc_compat_1_2(machine); -} - static void pc_init_isa(MachineState *machine) { pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE); @@ -732,6 +726,7 @@ static void pc_i440fx_1_3_machine_options(MachineClass *m) pc_i440fx_1_4_machine_options(m); m->hw_version = "1.3.0"; + m->deprecation_reason = "use a newer machine type instead"; x86mc->compat_apic_id_mode = true; compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat)); } @@ -800,82 +795,6 @@ DEFINE_I440FX_MACHINE(v1_0, "pc-1.0", pc_compat_1_2, pc_i440fx_1_0_machine_options); -static void pc_i440fx_0_15_machine_options(MachineClass *m) -{ - static GlobalProperty compat[] = { - PC_CPU_MODEL_IDS("0.15") - }; - - pc_i440fx_1_0_machine_options(m); - m->hw_version = "0.15"; - m->deprecation_reason = "use a newer machine type instead"; - compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat)); -} - -DEFINE_I440FX_MACHINE(v0_15, "pc-0.15", pc_compat_1_2, - pc_i440fx_0_15_machine_options); - - -static void pc_i440fx_0_14_machine_options(MachineClass *m) -{ - static GlobalProperty compat[] = { - PC_CPU_MODEL_IDS("0.14") - { "virtio-blk-pci", "event_idx", "off" }, - { "virtio-serial-pci", "event_idx", "off" }, - { "virtio-net-pci", "event_idx", "off" }, - { "virtio-balloon-pci", "event_idx", "off" }, - { "qxl", "revision", "2" }, - { "qxl-vga", "revision", "2" }, - }; - - pc_i440fx_0_15_machine_options(m); - m->hw_version = "0.14"; - compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat)); -} - -DEFINE_I440FX_MACHINE(v0_14, "pc-0.14", pc_compat_1_2, - pc_i440fx_0_14_machine_options); - -static void pc_i440fx_0_13_machine_options(MachineClass *m) -{ - PCMachineClass *pcmc = PC_MACHINE_CLASS(m); - static GlobalProperty compat[] = { - PC_CPU_MODEL_IDS("0.13") - { TYPE_PCI_DEVICE, "command_serr_enable", "off" }, - { "AC97", "use_broken_id", "1" }, - { "virtio-9p-pci", "vectors", "0" }, - { "VGA", "rombar", "0" }, - { "vmware-svga", "rombar", "0" }, - }; - - pc_i440fx_0_14_machine_options(m); - m->hw_version = "0.13"; - compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat)); - pcmc->kvmclock_enabled = false; -} - -DEFINE_I440FX_MACHINE(v0_13, "pc-0.13", pc_compat_0_13, - pc_i440fx_0_13_machine_options); - -static void pc_i440fx_0_12_machine_options(MachineClass *m) -{ - static GlobalProperty compat[] = { - PC_CPU_MODEL_IDS("0.12") - { "virtio-serial-pci", "max_ports", "1" }, - { "virtio-serial-pci", "vectors", "0" }, - { "usb-mouse", "serial", "1" }, - { "usb-tablet", "serial", "1" }, - { "usb-kbd", "serial", "1" }, - }; - - pc_i440fx_0_13_machine_options(m); - m->hw_version = "0.12"; - compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat)); -} - -DEFINE_I440FX_MACHINE(v0_12, "pc-0.12", pc_compat_0_13, - pc_i440fx_0_12_machine_options); - typedef struct { uint16_t gpu_device_id; uint16_t pch_device_id; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index b177c0fa0e..2f9efb45ba 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -256,7 +256,7 @@ The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or @section System emulator machines -@subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0) +@subsection pc-1.0, pc-1.1, pc-1.2 and pc-1.3 (since 5.0) These machine types are very old and likely can not be used for live migration from old QEMU versions anymore. A newer machine type should be used instead. diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c index 30e514bbfb..e8ffbbce4b 100644 --- a/tests/cpu-plug-test.c +++ b/tests/cpu-plug-test.c @@ -148,11 +148,7 @@ static void add_pc_test_case(const char *mname) (strcmp(mname, "pc-1.3") == 0) || (strcmp(mname, "pc-1.2") == 0) || (strcmp(mname, "pc-1.1") == 0) || - (strcmp(mname, "pc-1.0") == 0) || - (strcmp(mname, "pc-0.15") == 0) || - (strcmp(mname, "pc-0.14") == 0) || - (strcmp(mname, "pc-0.13") == 0) || - (strcmp(mname, "pc-0.12") == 0)) { + (strcmp(mname, "pc-1.0") == 0)) { path = g_strdup_printf("cpu-plug/%s/init/%ux%ux%u&maxcpus=%u", mname, data->sockets, data->cores, data->threads, data->maxcpus); -- 2.18.1
Now that the old pc-0.x machine types are gone, we do not need the "use_broken_id" hack anymore. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- hw/audio/ac97.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c index a136b97f68..78cda88333 100644 --- a/hw/audio/ac97.c +++ b/hw/audio/ac97.c @@ -161,7 +161,6 @@ typedef struct AC97BusMasterRegs { typedef struct AC97LinkState { PCIDevice dev; QEMUSoundCard card; - uint32_t use_broken_id; uint32_t glob_cnt; uint32_t glob_sta; uint32_t cas; @@ -1373,13 +1372,6 @@ static void ac97_realize(PCIDevice *dev, Error **errp) c[PCI_BASE_ADDRESS_0 + 6] = 0x00; c[PCI_BASE_ADDRESS_0 + 7] = 0x00; - if (s->use_broken_id) { - c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; - c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; - c[PCI_SUBSYSTEM_ID] = 0x00; - c[PCI_SUBSYSTEM_ID + 1] = 0x00; - } - c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */ c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */ @@ -1411,7 +1403,6 @@ static int ac97_init (PCIBus *bus) static Property ac97_properties[] = { DEFINE_AUDIO_PROPERTIES(AC97LinkState, card), - DEFINE_PROP_UINT32 ("use_broken_id", AC97LinkState, use_broken_id, 0), DEFINE_PROP_END_OF_LIST (), }; -- 2.18.1
Now that the old pc-0.x machine types have been removed, this config knob is not required anymore. Signed-off-by: Thomas Huth <thuth@redhat.com> --- hw/pci/pci.c | 6 +----- include/hw/pci/pci.h | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index cbc7a32568..e3d310365d 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -69,8 +69,6 @@ static Property pci_props[] = { DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), - DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present, - QEMU_PCI_CAP_SERR_BITNR, true), DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present, QEMU_PCIE_LNKSTA_DLLLA_BITNR, true), DEFINE_PROP_BIT("x-pcie-extcap-init", PCIDevice, cap_present, @@ -751,9 +749,7 @@ static void pci_init_wmask(PCIDevice *dev) pci_set_word(dev->wmask + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_INTX_DISABLE); - if (dev->cap_present & QEMU_PCI_CAP_SERR) { - pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR); - } + pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR); memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff, config_size - PCI_CONFIG_HEADER_SIZE); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index db75c6dfd0..2acd8321af 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -174,7 +174,7 @@ enum { #define QEMU_PCI_CAP_MULTIFUNCTION_BITNR 3 QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR), - /* command register SERR bit enabled */ + /* command register SERR bit enabled - unused since QEMU v5.0 */ #define QEMU_PCI_CAP_SERR_BITNR 4 QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR), /* Standard hot plug controller. */ -- 2.18.1
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
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 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
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/19 13:52, Thomas Huth wrote:
These have been on the deprecation list since a year now, so it's time to finally remove the pc-0.x machine types.
We then can also remove some compatibility hacks in the devices, i.e. the "use_broken_id" in ac97, the "command_serr_enable" in PCI devices and the "rombar" stuff in VGA devices.
v2: - Minor updates to the first patch (fix comment, add deprecation_reason message for the pc-1.x machines) - Keep the QEMU_PCI_CAP_SERR enum in the third patch - Added fourth patch to remove the "rombar" hacks from the VGA devices
Thomas Huth (4): hw/i386: Remove the deprecated machines 0.12 up to 0.15 hw/audio: Remove the "use_broken_id" hack from the AC97 device hw/pci: Remove the "command_serr_enable" property hw/display: Remove "rombar" hack from vga-pci and vmware_vga
hw/audio/ac97.c | 9 ----- hw/display/vga-pci.c | 5 --- hw/display/vga.c | 4 +- hw/display/vmware_vga.c | 5 --- hw/i386/pc_piix.c | 85 +---------------------------------------- hw/pci/pci.c | 6 +-- include/hw/pci/pci.h | 2 +- qemu-deprecated.texi | 2 +- tests/cpu-plug-test.c | 6 +-- 9 files changed, 7 insertions(+), 117 deletions(-)
Queued, thanks. Paolo
Patchew URL: https://patchew.org/QEMU/20191209125248.5849-1-thuth@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Cloning into 'dtc'... remote: Counting objects: 5280, done. error: RPC failed; result=18, HTTP code = 200 fatal: The remote end hung up unexpectedly fatal: protocol error: bad pack header Clone of 'https://git.qemu.org/git/dtc.git' into submodule path 'dtc' failed failed to update submodule dtc Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc' make[1]: *** [/var/tmp/patchew-tester-tmp-x_fnqco1/src/docker-src.2019-12-09-13.13.54.13964] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-x_fnqco1/src' make: *** [docker-run-test-quick@centos7] Error 2 real 4m48.507s user 0m2.457s The full log is available at http://patchew.org/logs/20191209125248.5849-1-thuth@redhat.com/testing.docke.... --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20191209125248.5849-1-thuth@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Cloning into 'dtc'... remote: Counting objects: 5280, done. error: RPC failed; result=18, HTTP code = 200 fatal: The remote end hung up unexpectedly fatal: protocol error: bad pack header Clone of 'https://git.qemu.org/git/dtc.git' into submodule path 'dtc' failed failed to update submodule dtc Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc' make[1]: *** [/var/tmp/patchew-tester-tmp-9lcnuxat/src/docker-src.2019-12-09-13.19.18.7229] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-9lcnuxat/src' make: *** [docker-run-test-mingw@fedora] Error 2 real 4m35.183s user 0m2.657s The full log is available at http://patchew.org/logs/20191209125248.5849-1-thuth@redhat.com/testing.docke.... --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
participants (5)
-
Gerd Hoffmann -
no-reply＠patchew.org -
Paolo Bonzini -
Philippe Mathieu-Daudé -
Thomas Huth