Re: [PATCH 1/2] pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7

[Thomas Huth <thuth@redhat.com>]

On 21/09/2023 14.10, Paolo Bonzini wrote:
> These are the last users of the 128K SeaBIOS blob in the i440FX family.
> Removing them allows us to drop PCI support from the 128K blob,
> thus making it easier to update SeaBIOS to newer versions.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   docs/about/deprecated.rst           |  8 ----
>   docs/about/removed-features.rst     |  2 +-
>   hw/i386/pc.c                        | 54 ---------------------
>   hw/i386/pc_piix.c                   | 73 -----------------------------
>   tests/qtest/test-x86-cpuid-compat.c | 10 +---
>   5 files changed, 2 insertions(+), 145 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 694a165a54a..d59bcf36230 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -261,14 +261,6 @@ deprecated; use the new name ``dtb-randomness`` instead. The new name
>   better reflects the way this property affects all random data within
>   the device tree blob, not just the ``kaslr-seed`` node.
>   
> -``pc-i440fx-1.4`` up to ``pc-i440fx-1.7`` (since 7.0)
> -'''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -These old machine types are quite neglected nowadays and thus might have
> -various pitfalls with regards to live migration. Use a newer machine type
> -instead.

While you're at it ... do we maybe want to start deprecating the next batch 
of machine types already? (Say pc-i440fx-2.0 up to pc-i440fx-2.2 maybe?)

> -
>   Backend options
>   ---------------
>   
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 39468b6e926..56e078ad126 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -730,7 +730,7 @@ mips ``fulong2e`` machine alias (removed in 6.0)
>   
>   This machine has been renamed ``fuloong2e``.
>   
> -``pc-0.10`` up to ``pc-1.3`` (removed in 4.0 up to 6.0)
> +``pc-0.10`` up to ``pc-1.7`` (removed in 4.0 up to 8.2)
>   '''''''''''''''''''''''''''''''''''''''''''''''''''''''

The names started to change with version 1.4, so it's "pc-i440fx-1.7" and 
not "pc-1.7".

>   These machine types were very old and likely could not be used for live
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 54838c0c411..1c7898a2d34 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -359,60 +359,6 @@ GlobalProperty pc_compat_2_0[] = {
>   };
>   const size_t pc_compat_2_0_len = G_N_ELEMENTS(pc_compat_2_0);
>   
> -GlobalProperty pc_compat_1_7[] = {
> -    PC_CPU_MODEL_IDS("1.7.0")
> -    { TYPE_USB_DEVICE, "msos-desc", "no" },
> -    { "PIIX4_PM", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
> -    { "hpet", HPET_INTCAP, "4" },
> -};
> -const size_t pc_compat_1_7_len = G_N_ELEMENTS(pc_compat_1_7);
> -
> -GlobalProperty pc_compat_1_6[] = {
> -    PC_CPU_MODEL_IDS("1.6.0")
> -    { "e1000", "mitigation", "off" },
> -    { "qemu64-" TYPE_X86_CPU, "model", "2" },
> -    { "qemu32-" TYPE_X86_CPU, "model", "3" },
> -    { "i440FX-pcihost", "short_root_bus", "1" },
> -    { "q35-pcihost", "short_root_bus", "1" },
> -};
> -const size_t pc_compat_1_6_len = G_N_ELEMENTS(pc_compat_1_6);
> -
> -GlobalProperty pc_compat_1_5[] = {
> -    PC_CPU_MODEL_IDS("1.5.0")
> -    { "Conroe-" TYPE_X86_CPU, "model", "2" },
> -    { "Conroe-" TYPE_X86_CPU, "min-level", "2" },
> -    { "Penryn-" TYPE_X86_CPU, "model", "2" },
> -    { "Penryn-" TYPE_X86_CPU, "min-level", "2" },
> -    { "Nehalem-" TYPE_X86_CPU, "model", "2" },
> -    { "Nehalem-" TYPE_X86_CPU, "min-level", "2" },
> -    { "virtio-net-pci", "any_layout", "off" },
> -    { TYPE_X86_CPU, "pmu", "on" },
> -    { "i440FX-pcihost", "short_root_bus", "0" },
> -    { "q35-pcihost", "short_root_bus", "0" },
> -};
> -const size_t pc_compat_1_5_len = G_N_ELEMENTS(pc_compat_1_5);
> -
> -GlobalProperty pc_compat_1_4[] = {
> -    PC_CPU_MODEL_IDS("1.4.0")
> -    { "scsi-hd", "discard_granularity", "0" },
> -    { "scsi-cd", "discard_granularity", "0" },
> -    { "ide-hd", "discard_granularity", "0" },
> -    { "ide-cd", "discard_granularity", "0" },
> -    { "virtio-blk-pci", "discard_granularity", "0" },
> -    /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string: */
> -    { "virtio-serial-pci", "vectors", "0xFFFFFFFF" },
> -    { "virtio-net-pci", "ctrl_guest_offloads", "off" },
> -    { "e1000", "romfile", "pxe-e1000.rom" },
> -    { "ne2k_pci", "romfile", "pxe-ne2k_pci.rom" },
> -    { "pcnet", "romfile", "pxe-pcnet.rom" },
> -    { "rtl8139", "romfile", "pxe-rtl8139.rom" },
> -    { "virtio-net-pci", "romfile", "pxe-virtio.rom" },
> -    { "486-" TYPE_X86_CPU, "model", "0" },
> -    { "n270" "-" TYPE_X86_CPU, "movbe", "off" },
> -    { "Westmere" "-" TYPE_X86_CPU, "pclmulqdq", "off" },
> -};
> -const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);

It might be worth to have a closer look at the above settings in the various 
devices - maybe we can get rid of some compatibility handling code in the 
devices now, in case the properties are not set by other targets as well.

  Thomas