[PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks

[PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks

[Thomas Huth]

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

[PATCH v2 1/4] hw/i386: Remove the deprecated machines 0.12 up to 0.15

[Thomas Huth]

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

[PATCH v2 2/4] hw/audio: Remove the "use_broken_id" hack from the AC97 device

[Thomas Huth]

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

[PATCH v2 3/4] hw/pci: Remove the "command_serr_enable" property

[Thomas Huth]

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

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

[Thomas Huth]

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

Re: [PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks

[Paolo Bonzini]

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

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

[Philippe Mathieu-Daudé]

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[] = {
> 

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

[Paolo Bonzini]

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);

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

[Gerd Hoffmann]

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

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

[Thomas Huth]

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

Re: [libvirt] [PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks

[no-reply]

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.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [libvirt] [PATCH v2 0/4] Remove deprecated pc-0.x machine types and related hacks

[no-reply]

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.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com