[PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Philippe Mathieu-Daudé]

(series reviewed)

Since v3:
- Fixed 2 issues noticed by Thomas (floppy fallback, e1000)

Since v2:
- Removed qtest in test-x86-cpuid-compat.c

Since v1:
- Fixed issues noticed by Thomas

The versioned 'pc' and 'q35' machines up to 2.12 been marked
as deprecated two releases ago, and are older than 6 years,
so according to our support policy we can remove them.

This series only includes the 2.4 and 2.5 machines removal,
as it is a big enough number of LoC removed. Rest will
follow. Highlight is the legacy fw_cfg API removal :)

Philippe Mathieu-Daudé (19):
  hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines
  hw/i386/pc: Remove PCMachineClass::broken_reserved_end field
  hw/i386/pc: Remove pc_compat_2_4[] array
  target/i386/cpu: Remove X86CPU::check_cpuid field
  hw/core/machine: Remove hw_compat_2_4[] array
  hw/net/e1000: Remove unused E1000_FLAG_MAC flag
  hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition
  hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition
  hw/i386/pc: Remove deprecated pc-q35-2.5 and pc-i440fx-2.5 machines
  hw/i386/x86: Remove X86MachineClass::save_tsc_khz field
  hw/nvram/fw_cfg: Remove legacy FW_CFG_ORDER_OVERRIDE
  hw/core/machine: Remove hw_compat_2_5[] array
  hw/block/fdc-isa: Remove 'fallback' property
  hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION
    definition
  hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition
  hw/scsi/vmw_pvscsi: Convert DeviceRealize -> InstanceInit
  hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS definition
  hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_DISABLE_PCIE definition
  hw/net/vmxnet3: Merge DeviceRealize in InstanceInit

 include/hw/boards.h                 |   9 +--
 include/hw/i386/pc.h                |   7 --
 include/hw/i386/x86.h               |   5 --
 include/hw/loader.h                 |   2 -
 include/hw/nvram/fw_cfg.h           |  10 ---
 include/hw/virtio/virtio-pci.h      |   8 --
 target/i386/cpu.h                   |   1 -
 hw/block/fdc-isa.c                  |   4 +-
 hw/core/loader.c                    |  14 ----
 hw/core/machine.c                   |  18 -----
 hw/i386/pc.c                        |  42 ++---------
 hw/i386/pc_piix.c                   |  26 -------
 hw/i386/pc_q35.c                    |  26 -------
 hw/i386/x86.c                       |   1 -
 hw/net/e1000.c                      |  93 ++++++++++--------------
 hw/net/vmxnet3.c                    |  44 ++---------
 hw/nvram/fw_cfg.c                   | 109 ++--------------------------
 hw/scsi/vmw_pvscsi.c                |  67 +++--------------
 hw/virtio/virtio-pci.c              |  11 +--
 system/vl.c                         |   5 --
 target/i386/cpu.c                   |   3 +-
 target/i386/machine.c               |   5 +-
 tests/qtest/test-x86-cpuid-compat.c |  14 ----
 23 files changed, 77 insertions(+), 447 deletions(-)

-- 
2.47.1

[PATCH v4 01/19] hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines

[Philippe Mathieu-Daudé]

These machines has been supported for a period of more than 6 years.
According to our versioned machine support policy (see commit
ce80c4fa6ff "docs: document special exception for machine type
deprecation & removal") they can now be removed.

Remove the qtest in test-x86-cpuid-compat.c file.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/pc_piix.c                   | 13 -------------
 hw/i386/pc_q35.c                    | 13 -------------
 tests/qtest/test-x86-cpuid-compat.c | 14 --------------
 3 files changed, 40 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0dce512f184..04213b45b44 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -791,19 +791,6 @@ static void pc_i440fx_machine_2_5_options(MachineClass *m)
 
 DEFINE_I440FX_MACHINE(2, 5);
 
-static void pc_i440fx_machine_2_4_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-
-    pc_i440fx_machine_2_5_options(m);
-    m->hw_version = "2.4.0";
-    pcmc->broken_reserved_end = true;
-    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
-    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
-}
-
-DEFINE_I440FX_MACHINE(2, 4);
-
 #ifdef CONFIG_ISAPC
 static void isapc_machine_options(MachineClass *m)
 {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c538b3d05b4..47e12602413 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -685,16 +685,3 @@ static void pc_q35_machine_2_5_options(MachineClass *m)
 }
 
 DEFINE_Q35_MACHINE(2, 5);
-
-static void pc_q35_machine_2_4_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-
-    pc_q35_machine_2_5_options(m);
-    m->hw_version = "2.4.0";
-    pcmc->broken_reserved_end = true;
-    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
-    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
-}
-
-DEFINE_Q35_MACHINE(2, 4);
diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
index c9de47bb269..456e2af6657 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -365,20 +365,6 @@ int main(int argc, char **argv)
                        "level", 10);
     }
 
-    /*
-     * xlevel doesn't have any feature that triggers auto-level
-     * code on old machine-types.  Just check that the compat code
-     * is working correctly:
-     */
-    if (qtest_has_machine("pc-i440fx-2.4")) {
-        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
-                       "SandyBridge", NULL, "pc-i440fx-2.4",
-                       "xlevel", 0x80000008);
-        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
-                       "SandyBridge", "svm=on,npt=on", "pc-i440fx-2.4",
-                       "xlevel", 0x80000008);
-    }
-
     /* Test feature parsing */
     add_feature_test("x86/cpuid/features/plus",
                      "486", "+arat",
-- 
2.47.1

Re: [PATCH v4 01/19] hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> These machines has been supported for a period of more than 6 years.
> According to our versioned machine support policy (see commit
> ce80c4fa6ff "docs: document special exception for machine type
> deprecation & removal") they can now be removed.
> 
> Remove the qtest in test-x86-cpuid-compat.c file.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/i386/pc_piix.c                   | 13 -------------
>   hw/i386/pc_q35.c                    | 13 -------------
>   tests/qtest/test-x86-cpuid-compat.c | 14 --------------
>   3 files changed, 40 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 0dce512f184..04213b45b44 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -791,19 +791,6 @@ static void pc_i440fx_machine_2_5_options(MachineClass *m)
>   
>   DEFINE_I440FX_MACHINE(2, 5);
>   
> -static void pc_i440fx_machine_2_4_options(MachineClass *m)
> -{
> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -
> -    pc_i440fx_machine_2_5_options(m);
> -    m->hw_version = "2.4.0";
> -    pcmc->broken_reserved_end = true;
> -    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
> -    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
> -}
> -
> -DEFINE_I440FX_MACHINE(2, 4);
> -
>   #ifdef CONFIG_ISAPC
>   static void isapc_machine_options(MachineClass *m)
>   {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c538b3d05b4..47e12602413 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -685,16 +685,3 @@ static void pc_q35_machine_2_5_options(MachineClass *m)
>   }
>   
>   DEFINE_Q35_MACHINE(2, 5);
> -
> -static void pc_q35_machine_2_4_options(MachineClass *m)
> -{
> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -
> -    pc_q35_machine_2_5_options(m);
> -    m->hw_version = "2.4.0";
> -    pcmc->broken_reserved_end = true;
> -    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
> -    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
> -}
> -
> -DEFINE_Q35_MACHINE(2, 4);
> diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
> index c9de47bb269..456e2af6657 100644
> --- a/tests/qtest/test-x86-cpuid-compat.c
> +++ b/tests/qtest/test-x86-cpuid-compat.c
> @@ -365,20 +365,6 @@ int main(int argc, char **argv)
>                          "level", 10);
>       }
>   
> -    /*
> -     * xlevel doesn't have any feature that triggers auto-level
> -     * code on old machine-types.  Just check that the compat code
> -     * is working correctly:
> -     */
> -    if (qtest_has_machine("pc-i440fx-2.4")) {
> -        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
> -                       "SandyBridge", NULL, "pc-i440fx-2.4",
> -                       "xlevel", 0x80000008);
> -        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
> -                       "SandyBridge", "svm=on,npt=on", "pc-i440fx-2.4",
> -                       "xlevel", 0x80000008);
> -    }
> -
>       /* Test feature parsing */
>       add_feature_test("x86/cpuid/features/plus",
>                        "486", "+arat",

Re: [PATCH v4 01/19] hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines

[Igor Mammedov]

On Mon, 12 May 2025 10:39:30 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> These machines has been supported for a period of more than 6 years.
> According to our versioned machine support policy (see commit
> ce80c4fa6ff "docs: document special exception for machine type
> deprecation & removal") they can now be removed.
> 
> Remove the qtest in test-x86-cpuid-compat.c file.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc_piix.c                   | 13 -------------
>  hw/i386/pc_q35.c                    | 13 -------------
>  tests/qtest/test-x86-cpuid-compat.c | 14 --------------
>  3 files changed, 40 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 0dce512f184..04213b45b44 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -791,19 +791,6 @@ static void pc_i440fx_machine_2_5_options(MachineClass *m)
>  
>  DEFINE_I440FX_MACHINE(2, 5);
>  
> -static void pc_i440fx_machine_2_4_options(MachineClass *m)
> -{
> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -
> -    pc_i440fx_machine_2_5_options(m);
> -    m->hw_version = "2.4.0";
> -    pcmc->broken_reserved_end = true;
> -    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
> -    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
> -}
> -
> -DEFINE_I440FX_MACHINE(2, 4);
> -
>  #ifdef CONFIG_ISAPC
>  static void isapc_machine_options(MachineClass *m)
>  {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c538b3d05b4..47e12602413 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -685,16 +685,3 @@ static void pc_q35_machine_2_5_options(MachineClass *m)
>  }
>  
>  DEFINE_Q35_MACHINE(2, 5);
> -
> -static void pc_q35_machine_2_4_options(MachineClass *m)
> -{
> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -
> -    pc_q35_machine_2_5_options(m);
> -    m->hw_version = "2.4.0";
> -    pcmc->broken_reserved_end = true;
> -    compat_props_add(m->compat_props, hw_compat_2_4, hw_compat_2_4_len);
> -    compat_props_add(m->compat_props, pc_compat_2_4, pc_compat_2_4_len);
> -}
> -
> -DEFINE_Q35_MACHINE(2, 4);
> diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
> index c9de47bb269..456e2af6657 100644
> --- a/tests/qtest/test-x86-cpuid-compat.c
> +++ b/tests/qtest/test-x86-cpuid-compat.c
> @@ -365,20 +365,6 @@ int main(int argc, char **argv)
>                         "level", 10);
>      }
>  
> -    /*
> -     * xlevel doesn't have any feature that triggers auto-level
> -     * code on old machine-types.  Just check that the compat code
> -     * is working correctly:
> -     */
> -    if (qtest_has_machine("pc-i440fx-2.4")) {
> -        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
> -                       "SandyBridge", NULL, "pc-i440fx-2.4",
> -                       "xlevel", 0x80000008);
> -        add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
> -                       "SandyBridge", "svm=on,npt=on", "pc-i440fx-2.4",
> -                       "xlevel", 0x80000008);
> -    }
> -
>      /* Test feature parsing */
>      add_feature_test("x86/cpuid/features/plus",
>                       "486", "+arat",

[PATCH v4 02/19] hw/i386/pc: Remove PCMachineClass::broken_reserved_end field

[Philippe Mathieu-Daudé]

The PCMachineClass::broken_reserved_end field was only used
by the pc-q35-2.4 and pc-i440fx-2.4 machines, which got removed.
Remove it and simplify pc_memory_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/i386/pc.h |  1 -
 hw/i386/pc.c         | 13 +++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9563674e2da..f4a874b17fc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -107,7 +107,6 @@ struct PCMachineClass {
     /* RAM / address space compat: */
     bool gigabyte_align;
     bool has_reserved_memory;
-    bool broken_reserved_end;
     bool enforce_amd_1tb_hole;
     bool isa_bios_alias;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 70656157ca0..c8bb4a3ee47 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -999,14 +999,13 @@ void pc_memory_init(PCMachineState *pcms,
 
     if (machine->device_memory) {
         uint64_t *val = g_malloc(sizeof(*val));
-        uint64_t res_mem_end = machine->device_memory->base;
-
-        if (!pcmc->broken_reserved_end) {
-            res_mem_end += memory_region_size(&machine->device_memory->mr);
-        }
+        uint64_t res_mem_end;
 
         if (pcms->cxl_devices_state.is_enabled) {
             res_mem_end = cxl_resv_end;
+        } else {
+            res_mem_end = machine->device_memory->base
+                          + memory_region_size(&machine->device_memory->mr);
         }
         *val = cpu_to_le64(ROUND_UP(res_mem_end, 1 * GiB));
         fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
@@ -1044,9 +1043,7 @@ uint64_t pc_pci_hole64_start(void)
         hole64_start = pc_get_cxl_range_end(pcms);
     } else if (pcmc->has_reserved_memory && (ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &hole64_start, &size);
-        if (!pcmc->broken_reserved_end) {
-            hole64_start += size;
-        }
+        hole64_start += size;
     } else {
         hole64_start = pc_above_4g_end(pcms);
     }
-- 
2.47.1

Re: [PATCH v4 02/19] hw/i386/pc: Remove PCMachineClass::broken_reserved_end field

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The PCMachineClass::broken_reserved_end field was only used
> by the pc-q35-2.4 and pc-i440fx-2.4 machines, which got removed.
> Remove it and simplify pc_memory_init().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   include/hw/i386/pc.h |  1 -
>   hw/i386/pc.c         | 13 +++++--------
>   2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 9563674e2da..f4a874b17fc 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -107,7 +107,6 @@ struct PCMachineClass {
>       /* RAM / address space compat: */
>       bool gigabyte_align;
>       bool has_reserved_memory;
> -    bool broken_reserved_end;
>       bool enforce_amd_1tb_hole;
>       bool isa_bios_alias;
>   
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 70656157ca0..c8bb4a3ee47 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -999,14 +999,13 @@ void pc_memory_init(PCMachineState *pcms,
>   
>       if (machine->device_memory) {
>           uint64_t *val = g_malloc(sizeof(*val));
> -        uint64_t res_mem_end = machine->device_memory->base;
> -
> -        if (!pcmc->broken_reserved_end) {
> -            res_mem_end += memory_region_size(&machine->device_memory->mr);
> -        }
> +        uint64_t res_mem_end;
>   
>           if (pcms->cxl_devices_state.is_enabled) {
>               res_mem_end = cxl_resv_end;
> +        } else {
> +            res_mem_end = machine->device_memory->base
> +                          + memory_region_size(&machine->device_memory->mr);
>           }
>           *val = cpu_to_le64(ROUND_UP(res_mem_end, 1 * GiB));
>           fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
> @@ -1044,9 +1043,7 @@ uint64_t pc_pci_hole64_start(void)
>           hole64_start = pc_get_cxl_range_end(pcms);
>       } else if (pcmc->has_reserved_memory && (ms->ram_size < ms->maxram_size)) {
>           pc_get_device_memory_range(pcms, &hole64_start, &size);
> -        if (!pcmc->broken_reserved_end) {
> -            hole64_start += size;
> -        }
> +        hole64_start += size;
>       } else {
>           hole64_start = pc_above_4g_end(pcms);
>       }

Re: [PATCH v4 02/19] hw/i386/pc: Remove PCMachineClass::broken_reserved_end field

[Igor Mammedov]

On Mon, 12 May 2025 10:39:31 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> The PCMachineClass::broken_reserved_end field was only used
> by the pc-q35-2.4 and pc-i440fx-2.4 machines, which got removed.
> Remove it and simplify pc_memory_init().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/i386/pc.h |  1 -
>  hw/i386/pc.c         | 13 +++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 9563674e2da..f4a874b17fc 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -107,7 +107,6 @@ struct PCMachineClass {
>      /* RAM / address space compat: */
>      bool gigabyte_align;
>      bool has_reserved_memory;
> -    bool broken_reserved_end;
>      bool enforce_amd_1tb_hole;
>      bool isa_bios_alias;
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 70656157ca0..c8bb4a3ee47 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -999,14 +999,13 @@ void pc_memory_init(PCMachineState *pcms,
>  
>      if (machine->device_memory) {
>          uint64_t *val = g_malloc(sizeof(*val));
> -        uint64_t res_mem_end = machine->device_memory->base;
> -
> -        if (!pcmc->broken_reserved_end) {
> -            res_mem_end += memory_region_size(&machine->device_memory->mr);
> -        }
> +        uint64_t res_mem_end;
>  
>          if (pcms->cxl_devices_state.is_enabled) {
>              res_mem_end = cxl_resv_end;
> +        } else {
> +            res_mem_end = machine->device_memory->base
> +                          + memory_region_size(&machine->device_memory->mr);
>          }
>          *val = cpu_to_le64(ROUND_UP(res_mem_end, 1 * GiB));
>          fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
> @@ -1044,9 +1043,7 @@ uint64_t pc_pci_hole64_start(void)
>          hole64_start = pc_get_cxl_range_end(pcms);
>      } else if (pcmc->has_reserved_memory && (ms->ram_size < ms->maxram_size)) {
>          pc_get_device_memory_range(pcms, &hole64_start, &size);
> -        if (!pcmc->broken_reserved_end) {
> -            hole64_start += size;
> -        }
> +        hole64_start += size;
>      } else {
>          hole64_start = pc_above_4g_end(pcms);
>      }

[PATCH v4 03/19] hw/i386/pc: Remove pc_compat_2_4[] array

[Philippe Mathieu-Daudé]

The pc_compat_2_4[] array was only used by the pc-q35-2.4
and pc-i440fx-2.4 machines, which got removed. Remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/i386/pc.h |  3 ---
 hw/i386/pc.c         | 19 -------------------
 2 files changed, 22 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f4a874b17fc..b34aa25fdce 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -301,9 +301,6 @@ extern const size_t pc_compat_2_6_len;
 extern GlobalProperty pc_compat_2_5[];
 extern const size_t pc_compat_2_5_len;
 
-extern GlobalProperty pc_compat_2_4[];
-extern const size_t pc_compat_2_4_len;
-
 #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
     static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
                                                  const void *data) \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c8bb4a3ee47..2b46714a5ac 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -262,25 +262,6 @@ const size_t pc_compat_2_6_len = G_N_ELEMENTS(pc_compat_2_6);
 GlobalProperty pc_compat_2_5[] = {};
 const size_t pc_compat_2_5_len = G_N_ELEMENTS(pc_compat_2_5);
 
-GlobalProperty pc_compat_2_4[] = {
-    PC_CPU_MODEL_IDS("2.4.0")
-    { "Haswell-" TYPE_X86_CPU, "abm", "off" },
-    { "Haswell-noTSX-" TYPE_X86_CPU, "abm", "off" },
-    { "Broadwell-" TYPE_X86_CPU, "abm", "off" },
-    { "Broadwell-noTSX-" TYPE_X86_CPU, "abm", "off" },
-    { "host" "-" TYPE_X86_CPU, "host-cache-info", "on" },
-    { TYPE_X86_CPU, "check", "off" },
-    { "qemu64" "-" TYPE_X86_CPU, "sse4a", "on" },
-    { "qemu64" "-" TYPE_X86_CPU, "abm", "on" },
-    { "qemu64" "-" TYPE_X86_CPU, "popcnt", "on" },
-    { "qemu32" "-" TYPE_X86_CPU, "popcnt", "on" },
-    { "Opteron_G2" "-" TYPE_X86_CPU, "rdtscp", "on" },
-    { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "on" },
-    { "Opteron_G4" "-" TYPE_X86_CPU, "rdtscp", "on" },
-    { "Opteron_G5" "-" TYPE_X86_CPU, "rdtscp", "on", }
-};
-const size_t pc_compat_2_4_len = G_N_ELEMENTS(pc_compat_2_4);
-
 /*
  * @PC_FW_DATA:
  * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables
-- 
2.47.1

Re: [PATCH v4 03/19] hw/i386/pc: Remove pc_compat_2_4[] array

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The pc_compat_2_4[] array was only used by the pc-q35-2.4
> and pc-i440fx-2.4 machines, which got removed. Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   include/hw/i386/pc.h |  3 ---
>   hw/i386/pc.c         | 19 -------------------
>   2 files changed, 22 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index f4a874b17fc..b34aa25fdce 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -301,9 +301,6 @@ extern const size_t pc_compat_2_6_len;
>   extern GlobalProperty pc_compat_2_5[];
>   extern const size_t pc_compat_2_5_len;
>   
> -extern GlobalProperty pc_compat_2_4[];
> -extern const size_t pc_compat_2_4_len;
> -
>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
>                                                    const void *data) \
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c8bb4a3ee47..2b46714a5ac 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -262,25 +262,6 @@ const size_t pc_compat_2_6_len = G_N_ELEMENTS(pc_compat_2_6);
>   GlobalProperty pc_compat_2_5[] = {};
>   const size_t pc_compat_2_5_len = G_N_ELEMENTS(pc_compat_2_5);
>   
> -GlobalProperty pc_compat_2_4[] = {
> -    PC_CPU_MODEL_IDS("2.4.0")
> -    { "Haswell-" TYPE_X86_CPU, "abm", "off" },
> -    { "Haswell-noTSX-" TYPE_X86_CPU, "abm", "off" },
> -    { "Broadwell-" TYPE_X86_CPU, "abm", "off" },
> -    { "Broadwell-noTSX-" TYPE_X86_CPU, "abm", "off" },
> -    { "host" "-" TYPE_X86_CPU, "host-cache-info", "on" },
> -    { TYPE_X86_CPU, "check", "off" },
> -    { "qemu64" "-" TYPE_X86_CPU, "sse4a", "on" },
> -    { "qemu64" "-" TYPE_X86_CPU, "abm", "on" },
> -    { "qemu64" "-" TYPE_X86_CPU, "popcnt", "on" },
> -    { "qemu32" "-" TYPE_X86_CPU, "popcnt", "on" },
> -    { "Opteron_G2" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G4" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G5" "-" TYPE_X86_CPU, "rdtscp", "on", }
> -};
> -const size_t pc_compat_2_4_len = G_N_ELEMENTS(pc_compat_2_4);
> -
>   /*
>    * @PC_FW_DATA:
>    * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables

Re: [PATCH v4 03/19] hw/i386/pc: Remove pc_compat_2_4[] array

[Igor Mammedov]

On Mon, 12 May 2025 10:39:32 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> The pc_compat_2_4[] array was only used by the pc-q35-2.4
> and pc-i440fx-2.4 machines, which got removed. Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/i386/pc.h |  3 ---
>  hw/i386/pc.c         | 19 -------------------
>  2 files changed, 22 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index f4a874b17fc..b34aa25fdce 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -301,9 +301,6 @@ extern const size_t pc_compat_2_6_len;
>  extern GlobalProperty pc_compat_2_5[];
>  extern const size_t pc_compat_2_5_len;
>  
> -extern GlobalProperty pc_compat_2_4[];
> -extern const size_t pc_compat_2_4_len;
> -
>  #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>      static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
>                                                   const void *data) \
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c8bb4a3ee47..2b46714a5ac 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -262,25 +262,6 @@ const size_t pc_compat_2_6_len = G_N_ELEMENTS(pc_compat_2_6);
>  GlobalProperty pc_compat_2_5[] = {};
>  const size_t pc_compat_2_5_len = G_N_ELEMENTS(pc_compat_2_5);
>  
> -GlobalProperty pc_compat_2_4[] = {
> -    PC_CPU_MODEL_IDS("2.4.0")
> -    { "Haswell-" TYPE_X86_CPU, "abm", "off" },
> -    { "Haswell-noTSX-" TYPE_X86_CPU, "abm", "off" },
> -    { "Broadwell-" TYPE_X86_CPU, "abm", "off" },
> -    { "Broadwell-noTSX-" TYPE_X86_CPU, "abm", "off" },
> -    { "host" "-" TYPE_X86_CPU, "host-cache-info", "on" },
> -    { TYPE_X86_CPU, "check", "off" },
> -    { "qemu64" "-" TYPE_X86_CPU, "sse4a", "on" },
> -    { "qemu64" "-" TYPE_X86_CPU, "abm", "on" },
> -    { "qemu64" "-" TYPE_X86_CPU, "popcnt", "on" },
> -    { "qemu32" "-" TYPE_X86_CPU, "popcnt", "on" },
> -    { "Opteron_G2" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G4" "-" TYPE_X86_CPU, "rdtscp", "on" },
> -    { "Opteron_G5" "-" TYPE_X86_CPU, "rdtscp", "on", }
> -};
> -const size_t pc_compat_2_4_len = G_N_ELEMENTS(pc_compat_2_4);
> -
>  /*
>   * @PC_FW_DATA:
>   * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables

[PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Philippe Mathieu-Daudé]

The X86CPU::check_cpuid boolean was only set in the
pc_compat_2_4[] array, via the 'check=off' property.
We removed all machines using that array, lets remove
that CPU property and simplify x86_cpu_realizefn().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.h | 1 -
 target/i386/cpu.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4f8ed8868e9..0db70a70439 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2164,7 +2164,6 @@ struct ArchCPU {
     uint8_t hyperv_ver_id_sb;
     uint32_t hyperv_ver_id_sn;
 
-    bool check_cpuid;
     bool enforce_cpuid;
     /*
      * Force features to be enabled even if the host doesn't support them.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1ca6307c72e..cd4361b4227 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8174,7 +8174,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
-    if (x86_cpu_filter_features(cpu, cpu->check_cpuid || cpu->enforce_cpuid)) {
+    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid)) {
         if (cpu->enforce_cpuid) {
             error_setg(&local_err,
                        accel_uses_host_cpuid() ?
@@ -8813,7 +8813,6 @@ static const Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT8("hv-version-id-sbranch", X86CPU, hyperv_ver_id_sb, 0),
     DEFINE_PROP_UINT32("hv-version-id-snumber", X86CPU, hyperv_ver_id_sn, 0),
 
-    DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
-- 
2.47.1

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The X86CPU::check_cpuid boolean was only set in the
> pc_compat_2_4[] array, via the 'check=off' property.
> We removed all machines using that array, lets remove
> that CPU property and simplify x86_cpu_realizefn().

No.

We cannot do this. Because it changes the behavior of QEMU.

'check_cpuid' is true by default while 'enforce_cpuid' is false. So that 
QEMU emits warnings in x86_cpu_filter_features() by default when user 
requests unsupported CPU features. If remove "check" property and the 
internal 'check_cpuid', QEMU will not do it unless user sets 
enforce_cpuid explicitly.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.h | 1 -
>   target/i386/cpu.c | 3 +--
>   2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 4f8ed8868e9..0db70a70439 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2164,7 +2164,6 @@ struct ArchCPU {
>       uint8_t hyperv_ver_id_sb;
>       uint32_t hyperv_ver_id_sn;
>   
> -    bool check_cpuid;
>       bool enforce_cpuid;
>       /*
>        * Force features to be enabled even if the host doesn't support them.
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1ca6307c72e..cd4361b4227 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8174,7 +8174,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>           }
>       }
>   
> -    if (x86_cpu_filter_features(cpu, cpu->check_cpuid || cpu->enforce_cpuid)) {
> +    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid)) {
>           if (cpu->enforce_cpuid) {
>               error_setg(&local_err,
>                          accel_uses_host_cpuid() ?
> @@ -8813,7 +8813,6 @@ static const Property x86_cpu_properties[] = {
>       DEFINE_PROP_UINT8("hv-version-id-sbranch", X86CPU, hyperv_ver_id_sb, 0),
>       DEFINE_PROP_UINT32("hv-version-id-snumber", X86CPU, hyperv_ver_id_sn, 0),
>   
> -    DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>       DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>       DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
>       DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Zhao Liu]

On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:
> Date: Wed, 28 May 2025 10:09:56 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
>  field
> 
> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> > The X86CPU::check_cpuid boolean was only set in the
> > pc_compat_2_4[] array, via the 'check=off' property.
> > We removed all machines using that array, lets remove
> > that CPU property and simplify x86_cpu_realizefn().
> 
> No.
> 
> We cannot do this. Because it changes the behavior of QEMU.
> 
> 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> QEMU emits warnings in x86_cpu_filter_features() by default when user
> requests unsupported CPU features. If remove "check" property and the
> internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> explicitly.

One option would be to have x86_cpu_filter_features() unconditionally
turn on verbose and print warnings, but some people might want to turn
off these warning prints, I don't know if anyone would, but it would be
possible.

The other option is still to keep the “check” property.

IMO, the latter option is the better way to reduce Philippe's burden.

Regards,
Zhao

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Igor Mammedov]

On Wed, 28 May 2025 13:23:49 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:
> > Date: Wed, 28 May 2025 10:09:56 +0800
> > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
> >  field
> > 
> > On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:  
> > > The X86CPU::check_cpuid boolean was only set in the
> > > pc_compat_2_4[] array, via the 'check=off' property.
> > > We removed all machines using that array, lets remove
> > > that CPU property and simplify x86_cpu_realizefn().  
> > 
> > No.
> > 
> > We cannot do this. Because it changes the behavior of QEMU.
> > 
> > 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> > QEMU emits warnings in x86_cpu_filter_features() by default when user
> > requests unsupported CPU features. If remove "check" property and the
> > internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> > explicitly.  
> 
> One option would be to have x86_cpu_filter_features() unconditionally
> turn on verbose and print warnings, but some people might want to turn
> off these warning prints, I don't know if anyone would, but it would be
> possible.
> 
> The other option is still to keep the “check” property.
> 
> IMO, the latter option is the better way to reduce Philippe's burden.

we essentially loose warnings by default when some features aren't available,
qemu still continues to run though.

Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
(it does its has its own cpu model calculation). Likely other mgmt do not care
about it either, and if they do they probably doing something wrong and
should use QMP to get that data.
That leaves us with human users, for that case I'd say one should use
enforce_cpuid if feature availability matters.

so +1 to removal
 
> 
> Regards,
> Zhao
> 
> 

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Xiaoyao Li]

On 6/3/2025 11:02 PM, Igor Mammedov wrote:
> On Wed, 28 May 2025 13:23:49 +0800
> Zhao Liu <zhao1.liu@intel.com> wrote:
> 
>> On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:
>>> Date: Wed, 28 May 2025 10:09:56 +0800
>>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>>> Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
>>>   field
>>>
>>> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
>>>> The X86CPU::check_cpuid boolean was only set in the
>>>> pc_compat_2_4[] array, via the 'check=off' property.
>>>> We removed all machines using that array, lets remove
>>>> that CPU property and simplify x86_cpu_realizefn().
>>>
>>> No.
>>>
>>> We cannot do this. Because it changes the behavior of QEMU.
>>>
>>> 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
>>> QEMU emits warnings in x86_cpu_filter_features() by default when user
>>> requests unsupported CPU features. If remove "check" property and the
>>> internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
>>> explicitly.
>>
>> One option would be to have x86_cpu_filter_features() unconditionally
>> turn on verbose and print warnings, but some people might want to turn
>> off these warning prints, I don't know if anyone would, but it would be
>> possible.
>>
>> The other option is still to keep the “check” property.
>>
>> IMO, the latter option is the better way to reduce Philippe's burden.
> 
> we essentially loose warnings by default when some features aren't available,
> qemu still continues to run though.
> 
> Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
> (it does its has its own cpu model calculation). Likely other mgmt do not care
> about it either, and if they do they probably doing something wrong and
> should use QMP to get that data.
> That leaves us with human users, for that case I'd say one should use
> enforce_cpuid if feature availability matters.

But with "check", it allows the VM to continue running with the 
unsupported bits cleared and warnings to inform users. This is really 
friendly.

> so +1 to removal
>   
>>
>> Regards,
>> Zhao
>>
>>
> 

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Igor Mammedov]

On Tue, 3 Jun 2025 23:54:19 +0800
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> On 6/3/2025 11:02 PM, Igor Mammedov wrote:
> > On Wed, 28 May 2025 13:23:49 +0800
> > Zhao Liu <zhao1.liu@intel.com> wrote:
> >   
> >> On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:  
> >>> Date: Wed, 28 May 2025 10:09:56 +0800
> >>> From: Xiaoyao Li <xiaoyao.li@intel.com>
> >>> Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
> >>>   field
> >>>
> >>> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:  
> >>>> The X86CPU::check_cpuid boolean was only set in the
> >>>> pc_compat_2_4[] array, via the 'check=off' property.
> >>>> We removed all machines using that array, lets remove
> >>>> that CPU property and simplify x86_cpu_realizefn().  
> >>>
> >>> No.
> >>>
> >>> We cannot do this. Because it changes the behavior of QEMU.
> >>>
> >>> 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> >>> QEMU emits warnings in x86_cpu_filter_features() by default when user
> >>> requests unsupported CPU features. If remove "check" property and the
> >>> internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> >>> explicitly.  
> >>
> >> One option would be to have x86_cpu_filter_features() unconditionally
> >> turn on verbose and print warnings, but some people might want to turn
> >> off these warning prints, I don't know if anyone would, but it would be
> >> possible.
> >>
> >> The other option is still to keep the “check” property.
> >>
> >> IMO, the latter option is the better way to reduce Philippe's burden.  
> > 
> > we essentially loose warnings by default when some features aren't available,
> > qemu still continues to run though.
> > 
> > Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
> > (it does its has its own cpu model calculation). Likely other mgmt do not care
> > about it either, and if they do they probably doing something wrong and
> > should use QMP to get that data.
> > That leaves us with human users, for that case I'd say one should use
> > enforce_cpuid if feature availability matters.  
> 
> But with "check", it allows the VM to continue running with the 
> unsupported bits cleared and warnings to inform users. This is really 
> friendly.

it's friendly for human users (mostly developers), but for upper layers
is doesn't make a difference, since it's noise in logs nobody reads until
qemu somehow works.

the 1st category can and should use enforce flag instead to get what they ask for
if they care about it.
If missing feature bits do not matter then it warnings shouldn't matter either.
What I'm advocating for is being more strict/deterministic on QEMU side,
  1. you get what you asked for with enforce or fix you CLI explicitly
     to be clear on what you are missing out.
  2. you don't care if some features are missing, but then you don't really care
     about warnings either. (I'd still get missing features filtered out though,
     just silently). If one cares about missing features, one can use #1

I admit, It's a tiny bit of code but removing it, cleans up code a little bit
and helps readability/in reviews. Doing such small cleanups here and there
have a cumulative effect on the codebase.

Anyways it's not something I'd fight for, so if you insist on keeping it
it's ok as well.

> > so +1 to removal
> >     
> >>
> >> Regards,
> >> Zhao
> >>
> >>  
> >   
> 

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Daniel P. Berrangé]

On Thu, Jun 05, 2025 at 11:09:30AM +0200, Igor Mammedov wrote:
> On Tue, 3 Jun 2025 23:54:19 +0800
> Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> 
> > On 6/3/2025 11:02 PM, Igor Mammedov wrote:
> > > On Wed, 28 May 2025 13:23:49 +0800
> > > Zhao Liu <zhao1.liu@intel.com> wrote:
> > >   
> > >> On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:  
> > >>> Date: Wed, 28 May 2025 10:09:56 +0800
> > >>> From: Xiaoyao Li <xiaoyao.li@intel.com>
> > >>> Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
> > >>>   field
> > >>>
> > >>> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:  
> > >>>> The X86CPU::check_cpuid boolean was only set in the
> > >>>> pc_compat_2_4[] array, via the 'check=off' property.
> > >>>> We removed all machines using that array, lets remove
> > >>>> that CPU property and simplify x86_cpu_realizefn().  
> > >>>
> > >>> No.
> > >>>
> > >>> We cannot do this. Because it changes the behavior of QEMU.
> > >>>
> > >>> 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> > >>> QEMU emits warnings in x86_cpu_filter_features() by default when user
> > >>> requests unsupported CPU features. If remove "check" property and the
> > >>> internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> > >>> explicitly.  
> > >>
> > >> One option would be to have x86_cpu_filter_features() unconditionally
> > >> turn on verbose and print warnings, but some people might want to turn
> > >> off these warning prints, I don't know if anyone would, but it would be
> > >> possible.
> > >>
> > >> The other option is still to keep the “check” property.
> > >>
> > >> IMO, the latter option is the better way to reduce Philippe's burden.  
> > > 
> > > we essentially loose warnings by default when some features aren't available,
> > > qemu still continues to run though.
> > > 
> > > Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
> > > (it does its has its own cpu model calculation). Likely other mgmt do not care
> > > about it either, and if they do they probably doing something wrong and
> > > should use QMP to get that data.
> > > That leaves us with human users, for that case I'd say one should use
> > > enforce_cpuid if feature availability matters.  
> > 
> > But with "check", it allows the VM to continue running with the 
> > unsupported bits cleared and warnings to inform users. This is really 
> > friendly.
> 
> it's friendly for human users (mostly developers), but for upper layers
> is doesn't make a difference, since it's noise in logs nobody reads until
> qemu somehow works.

The latter statement is accurate. In the libvirt case, this info ends up
in the per-VM logfile, which is one of the key files humans look at first
when triaging any QEMU related bug reports.

> 
> the 1st category can and should use enforce flag instead to get what they ask for
> if they care about it.
> If missing feature bits do not matter then it warnings shouldn't matter either.

I don't think that's correct. The mgmt app can decide the missing features
don't matter, but at the same time someone looking at the QEMU command line
doesn't know the mgmt apps' intent and it is easy to overlook that QEMU will
silently drop features. So the precense of the warnings in the log alerts the
person debugging that the actual QEMU guest visible CPU is subtly different
from what's on the command line, and that's useful IMHO for troubleshooting.

> What I'm advocating for is being more strict/deterministic on QEMU side,
>   1. you get what you asked for with enforce or fix you CLI explicitly
>      to be clear on what you are missing out.
>   2. you don't care if some features are missing, but then you don't really care
>      about warnings either. (I'd still get missing features filtered out though,
>      just silently). If one cares about missing features, one can use #1
> 
> I admit, It's a tiny bit of code but removing it, cleans up code a little bit
> and helps readability/in reviews. Doing such small cleanups here and there
> have a cumulative effect on the codebase.
> 
> Anyways it's not something I'd fight for, so if you insist on keeping it
> it's ok as well.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Daniel P. Berrangé]

On Tue, Jun 03, 2025 at 05:02:38PM +0200, Igor Mammedov wrote:
> On Wed, 28 May 2025 13:23:49 +0800
> Zhao Liu <zhao1.liu@intel.com> wrote:
> 
> > On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:
> > > Date: Wed, 28 May 2025 10:09:56 +0800
> > > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > > Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
> > >  field
> > > 
> > > On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:  
> > > > The X86CPU::check_cpuid boolean was only set in the
> > > > pc_compat_2_4[] array, via the 'check=off' property.
> > > > We removed all machines using that array, lets remove
> > > > that CPU property and simplify x86_cpu_realizefn().  
> > > 
> > > No.
> > > 
> > > We cannot do this. Because it changes the behavior of QEMU.
> > > 
> > > 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> > > QEMU emits warnings in x86_cpu_filter_features() by default when user
> > > requests unsupported CPU features. If remove "check" property and the
> > > internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> > > explicitly.  
> > 
> > One option would be to have x86_cpu_filter_features() unconditionally
> > turn on verbose and print warnings, but some people might want to turn
> > off these warning prints, I don't know if anyone would, but it would be
> > possible.
> > 
> > The other option is still to keep the “check” property.
> > 
> > IMO, the latter option is the better way to reduce Philippe's burden.
> 
> we essentially loose warnings by default when some features aren't available,
> qemu still continues to run though.
> 
> Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
> (it does its has its own cpu model calculation). Likely other mgmt do not care
> about it either, and if they do they probably doing something wrong and
> should use QMP to get that data.

Acking it was a mistake on my part - I mis-interpreted the patch and so
didn't notice we were loosing the verbose printing of missing features
by default.

I'm actually curious why we made the 'check' feature tied to machine
types at all. If it doesn't affect guest ABI, just causes verbose
info on stderr, it feels like something we could have just had on
all machine types new & old. Git history brings us back to

  commit 3e68482224129c3ddc061af7c9d438b882ecfdd1
  Author: Eduardo Habkost <ehabkost@redhat.com>
  Date:   Tue Nov 3 17:18:50 2015 -0200

    target-i386: Set "check=off" by default on pc-*-2.4 and older
    
    The default CPU model (qemu64) have some issues today: it enables some
    features (ABM and SSE4a) that are not present in many host CPUs. That
    means many hosts (but not all of them) had those features silently
    disabled in the default configuration in QEMU 2.4 and older.
    
    With the new "check=on" default, this causes warnings to be printed in
    the default configuration, because of the lack of SSE4A on all Intel
    hosts, and the lack of ABM on Sandy Bridge and older hosts:
    
      $ qemu-system-x86_64 -machine pc,accel=kvm
      warning: host doesn't support requested feature: CPUID.80000001H:ECX.abm [bit 5]
      warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
    
    Those issues will be fixed in pc-*-2.5 and newer. But as we can't change
    the guest ABI in pc-*-2.4, disable "check" mode by default in pc-*-2.4
    and older so we don't print spurious warnings.

IOW, we wanted to have 'check' unconditionally on by default, but
had to do a temp hack to avoid spamming all configurations with
the broken 'qemu64' CPU model design.

> That leaves us with human users, for that case I'd say one should use
> enforce_cpuid if feature availability matters.

IMHO even with mgmt apps, it is worth having 'check=on' by default
as the log message has value in debugging scenarios. It could have
the potential to highlight situations where an mgmt app has
unwittingly done something wrong with CPU config. At the very least
though its a warning to humans debugging that they should not trust
the QEMU command line as a expressing the full CPU featureset.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Igor Mammedov]

On Thu, 5 Jun 2025 10:24:34 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, Jun 03, 2025 at 05:02:38PM +0200, Igor Mammedov wrote:
> > On Wed, 28 May 2025 13:23:49 +0800
> > Zhao Liu <zhao1.liu@intel.com> wrote:
> >   
> > > On Wed, May 28, 2025 at 10:09:56AM +0800, Xiaoyao Li wrote:  
> > > > Date: Wed, 28 May 2025 10:09:56 +0800
> > > > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > > > Subject: Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid
> > > >  field
> > > > 
> > > > On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:    
> > > > > The X86CPU::check_cpuid boolean was only set in the
> > > > > pc_compat_2_4[] array, via the 'check=off' property.
> > > > > We removed all machines using that array, lets remove
> > > > > that CPU property and simplify x86_cpu_realizefn().    
> > > > 
> > > > No.
> > > > 
> > > > We cannot do this. Because it changes the behavior of QEMU.
> > > > 
> > > > 'check_cpuid' is true by default while 'enforce_cpuid' is false. So that
> > > > QEMU emits warnings in x86_cpu_filter_features() by default when user
> > > > requests unsupported CPU features. If remove "check" property and the
> > > > internal 'check_cpuid', QEMU will not do it unless user sets enforce_cpuid
> > > > explicitly.    
> > > 
> > > One option would be to have x86_cpu_filter_features() unconditionally
> > > turn on verbose and print warnings, but some people might want to turn
> > > off these warning prints, I don't know if anyone would, but it would be
> > > possible.
> > > 
> > > The other option is still to keep the “check” property.
> > > 
> > > IMO, the latter option is the better way to reduce Philippe's burden.  
> > 
> > we essentially loose warnings by default when some features aren't available,
> > qemu still continues to run though.
> > 
> > Given that Daniel acked it from libvirt side, libvirt doesn't care about warnings
> > (it does its has its own cpu model calculation). Likely other mgmt do not care
> > about it either, and if they do they probably doing something wrong and
> > should use QMP to get that data.  
> 
> Acking it was a mistake on my part - I mis-interpreted the patch and so
> didn't notice we were loosing the verbose printing of missing features
> by default.
> 
> I'm actually curious why we made the 'check' feature tied to machine
> types at all. If it doesn't affect guest ABI, just causes verbose
> info on stderr, it feels like something we could have just had on
> all machine types new & old. Git history brings us back to
> 
>   commit 3e68482224129c3ddc061af7c9d438b882ecfdd1
>   Author: Eduardo Habkost <ehabkost@redhat.com>
>   Date:   Tue Nov 3 17:18:50 2015 -0200
> 
>     target-i386: Set "check=off" by default on pc-*-2.4 and older
>     
>     The default CPU model (qemu64) have some issues today: it enables some
>     features (ABM and SSE4a) that are not present in many host CPUs. That
>     means many hosts (but not all of them) had those features silently
>     disabled in the default configuration in QEMU 2.4 and older.
>     
>     With the new "check=on" default, this causes warnings to be printed in
>     the default configuration, because of the lack of SSE4A on all Intel
>     hosts, and the lack of ABM on Sandy Bridge and older hosts:
>     
>       $ qemu-system-x86_64 -machine pc,accel=kvm
>       warning: host doesn't support requested feature: CPUID.80000001H:ECX.abm [bit 5]
>       warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
>     
>     Those issues will be fixed in pc-*-2.5 and newer. But as we can't change
>     the guest ABI in pc-*-2.4, disable "check" mode by default in pc-*-2.4
>     and older so we don't print spurious warnings.
> 
> IOW, we wanted to have 'check' unconditionally on by default, but
> had to do a temp hack to avoid spamming all configurations with
> the broken 'qemu64' CPU model design.
> 
> > That leaves us with human users, for that case I'd say one should use
> > enforce_cpuid if feature availability matters.  
> 
> IMHO even with mgmt apps, it is worth having 'check=on' by default
> as the log message has value in debugging scenarios. It could have
> the potential to highlight situations where an mgmt app has
> unwittingly done something wrong with CPU config. At the very least
> though its a warning to humans debugging that they should not trust
> the QEMU command line as a expressing the full CPU featureset.

in that line of thought, maybe hardcode 'check=on' and drop conditional/property?

> 
> With regards,
> Daniel

Re: [PATCH v4 04/19] target/i386/cpu: Remove X86CPU::check_cpuid field

[Igor Mammedov]

On Mon, 12 May 2025 10:39:33 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> The X86CPU::check_cpuid boolean was only set in the
> pc_compat_2_4[] array, via the 'check=off' property.
> We removed all machines using that array, lets remove
> that CPU property and simplify x86_cpu_realizefn().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target/i386/cpu.h | 1 -
>  target/i386/cpu.c | 3 +--
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 4f8ed8868e9..0db70a70439 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2164,7 +2164,6 @@ struct ArchCPU {
>      uint8_t hyperv_ver_id_sb;
>      uint32_t hyperv_ver_id_sn;
>  
> -    bool check_cpuid;
>      bool enforce_cpuid;
>      /*
>       * Force features to be enabled even if the host doesn't support them.
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1ca6307c72e..cd4361b4227 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8174,7 +8174,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>      }
>  
> -    if (x86_cpu_filter_features(cpu, cpu->check_cpuid || cpu->enforce_cpuid)) {
> +    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid)) {
>          if (cpu->enforce_cpuid) {
>              error_setg(&local_err,
>                         accel_uses_host_cpuid() ?
> @@ -8813,7 +8813,6 @@ static const Property x86_cpu_properties[] = {
>      DEFINE_PROP_UINT8("hv-version-id-sbranch", X86CPU, hyperv_ver_id_sb, 0),
>      DEFINE_PROP_UINT32("hv-version-id-snumber", X86CPU, hyperv_ver_id_sn, 0),
>  
> -    DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),

[PATCH v4 05/19] hw/core/machine: Remove hw_compat_2_4[] array

[Philippe Mathieu-Daudé]

The hw_compat_2_4[] array was only used by the pc-q35-2.4 and
pc-i440fx-2.4 machines, which got removed. Remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/boards.h | 3 ---
 hw/core/machine.c   | 9 ---------
 2 files changed, 12 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a7b1fcffae3..03e7cbeae82 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -866,7 +866,4 @@ extern const size_t hw_compat_2_6_len;
 extern GlobalProperty hw_compat_2_5[];
 extern const size_t hw_compat_2_5_len;
 
-extern GlobalProperty hw_compat_2_4[];
-extern const size_t hw_compat_2_4_len;
-
 #endif
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ed01798d37c..d66b02b564c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -292,15 +292,6 @@ GlobalProperty hw_compat_2_5[] = {
 };
 const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
 
-GlobalProperty hw_compat_2_4[] = {
-    { "e1000", "extra_mac_registers", "off" },
-    { "virtio-pci", "x-disable-pcie", "on" },
-    { "virtio-pci", "migrate-extra", "off" },
-    { "fw_cfg_mem", "dma_enabled", "off" },
-    { "fw_cfg_io", "dma_enabled", "off" }
-};
-const size_t hw_compat_2_4_len = G_N_ELEMENTS(hw_compat_2_4);
-
 MachineState *current_machine;
 
 static char *machine_get_kernel(Object *obj, Error **errp)
-- 
2.47.1

Re: [PATCH v4 05/19] hw/core/machine: Remove hw_compat_2_4[] array

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The hw_compat_2_4[] array was only used by the pc-q35-2.4 and
> pc-i440fx-2.4 machines, which got removed. Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   include/hw/boards.h | 3 ---
>   hw/core/machine.c   | 9 ---------
>   2 files changed, 12 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index a7b1fcffae3..03e7cbeae82 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -866,7 +866,4 @@ extern const size_t hw_compat_2_6_len;
>   extern GlobalProperty hw_compat_2_5[];
>   extern const size_t hw_compat_2_5_len;
>   
> -extern GlobalProperty hw_compat_2_4[];
> -extern const size_t hw_compat_2_4_len;
> -
>   #endif
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ed01798d37c..d66b02b564c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -292,15 +292,6 @@ GlobalProperty hw_compat_2_5[] = {
>   };
>   const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
>   
> -GlobalProperty hw_compat_2_4[] = {
> -    { "e1000", "extra_mac_registers", "off" },
> -    { "virtio-pci", "x-disable-pcie", "on" },
> -    { "virtio-pci", "migrate-extra", "off" },
> -    { "fw_cfg_mem", "dma_enabled", "off" },
> -    { "fw_cfg_io", "dma_enabled", "off" }
> -};
> -const size_t hw_compat_2_4_len = G_N_ELEMENTS(hw_compat_2_4);
> -
>   MachineState *current_machine;
>   
>   static char *machine_get_kernel(Object *obj, Error **errp)

[PATCH v4 06/19] hw/net/e1000: Remove unused E1000_FLAG_MAC flag

[Philippe Mathieu-Daudé]

E1000_FLAG_MAC was only used by the hw_compat_2_4[] array,
via the 'extra_mac_registers=off' property. We removed all
machines using that array, lets remove all the code around
E1000_FLAG_MAC, including the MAC_ACCESS_FLAG_NEEDED enum,
similarly to commit fa4ec9ffda7 ("e1000: remove old
compatibility code").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/net/e1000.c | 93 ++++++++++++++++++++++----------------------------
 1 file changed, 40 insertions(+), 53 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index cba4999e6d0..a80a7b0cdb4 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -127,10 +127,8 @@ struct E1000State_st {
     QEMUTimer *flush_queue_timer;
 
 /* Compatibility flags for migration to/from qemu 1.3.0 and older */
-#define E1000_FLAG_MAC_BIT 2
 #define E1000_FLAG_TSO_BIT 3
 #define E1000_FLAG_VET_BIT 4
-#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT)
 #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT)
 #define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT)
 
@@ -1212,52 +1210,51 @@ enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
 
 enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 };
 
-#define markflag(x)    ((E1000_FLAG_##x << 2) | MAC_ACCESS_FLAG_NEEDED)
 /* In the array below the meaning of the bits is: [f|f|f|f|f|f|n|p]
  * f - flag bits (up to 6 possible flags)
  * n - flag needed
- * p - partially implenented */
+ * p - partially implemented */
 static const uint8_t mac_reg_access[0x8000] = {
-    [IPAV]    = markflag(MAC),    [WUC]     = markflag(MAC),
-    [IP6AT]   = markflag(MAC),    [IP4AT]   = markflag(MAC),
-    [FFVT]    = markflag(MAC),    [WUPM]    = markflag(MAC),
-    [ECOL]    = markflag(MAC),    [MCC]     = markflag(MAC),
-    [DC]      = markflag(MAC),    [TNCRS]   = markflag(MAC),
-    [RLEC]    = markflag(MAC),    [XONRXC]  = markflag(MAC),
-    [XOFFTXC] = markflag(MAC),    [RFC]     = markflag(MAC),
-    [TSCTFC]  = markflag(MAC),    [MGTPRC]  = markflag(MAC),
-    [WUS]     = markflag(MAC),    [AIT]     = markflag(MAC),
-    [FFLT]    = markflag(MAC),    [FFMT]    = markflag(MAC),
-    [SCC]     = markflag(MAC),    [FCRUC]   = markflag(MAC),
-    [LATECOL] = markflag(MAC),    [COLC]    = markflag(MAC),
-    [SEQEC]   = markflag(MAC),    [CEXTERR] = markflag(MAC),
-    [XONTXC]  = markflag(MAC),    [XOFFRXC] = markflag(MAC),
-    [RJC]     = markflag(MAC),    [RNBC]    = markflag(MAC),
-    [MGTPDC]  = markflag(MAC),    [MGTPTC]  = markflag(MAC),
-    [RUC]     = markflag(MAC),    [ROC]     = markflag(MAC),
-    [GORCL]   = markflag(MAC),    [GORCH]   = markflag(MAC),
-    [GOTCL]   = markflag(MAC),    [GOTCH]   = markflag(MAC),
-    [BPRC]    = markflag(MAC),    [MPRC]    = markflag(MAC),
-    [TSCTC]   = markflag(MAC),    [PRC64]   = markflag(MAC),
-    [PRC127]  = markflag(MAC),    [PRC255]  = markflag(MAC),
-    [PRC511]  = markflag(MAC),    [PRC1023] = markflag(MAC),
-    [PRC1522] = markflag(MAC),    [PTC64]   = markflag(MAC),
-    [PTC127]  = markflag(MAC),    [PTC255]  = markflag(MAC),
-    [PTC511]  = markflag(MAC),    [PTC1023] = markflag(MAC),
-    [PTC1522] = markflag(MAC),    [MPTC]    = markflag(MAC),
-    [BPTC]    = markflag(MAC),
+    [IPAV]    = MAC_ACCESS_FLAG_NEEDED,    [WUC]     = MAC_ACCESS_FLAG_NEEDED,
+    [IP6AT]   = MAC_ACCESS_FLAG_NEEDED,    [IP4AT]   = MAC_ACCESS_FLAG_NEEDED,
+    [FFVT]    = MAC_ACCESS_FLAG_NEEDED,    [WUPM]    = MAC_ACCESS_FLAG_NEEDED,
+    [ECOL]    = MAC_ACCESS_FLAG_NEEDED,    [MCC]     = MAC_ACCESS_FLAG_NEEDED,
+    [DC]      = MAC_ACCESS_FLAG_NEEDED,    [TNCRS]   = MAC_ACCESS_FLAG_NEEDED,
+    [RLEC]    = MAC_ACCESS_FLAG_NEEDED,    [XONRXC]  = MAC_ACCESS_FLAG_NEEDED,
+    [XOFFTXC] = MAC_ACCESS_FLAG_NEEDED,    [RFC]     = MAC_ACCESS_FLAG_NEEDED,
+    [TSCTFC]  = MAC_ACCESS_FLAG_NEEDED,    [MGTPRC]  = MAC_ACCESS_FLAG_NEEDED,
+    [WUS]     = MAC_ACCESS_FLAG_NEEDED,    [AIT]     = MAC_ACCESS_FLAG_NEEDED,
+    [FFLT]    = MAC_ACCESS_FLAG_NEEDED,    [FFMT]    = MAC_ACCESS_FLAG_NEEDED,
+    [SCC]     = MAC_ACCESS_FLAG_NEEDED,    [FCRUC]   = MAC_ACCESS_FLAG_NEEDED,
+    [LATECOL] = MAC_ACCESS_FLAG_NEEDED,    [COLC]    = MAC_ACCESS_FLAG_NEEDED,
+    [SEQEC]   = MAC_ACCESS_FLAG_NEEDED,    [CEXTERR] = MAC_ACCESS_FLAG_NEEDED,
+    [XONTXC]  = MAC_ACCESS_FLAG_NEEDED,    [XOFFRXC] = MAC_ACCESS_FLAG_NEEDED,
+    [RJC]     = MAC_ACCESS_FLAG_NEEDED,    [RNBC]    = MAC_ACCESS_FLAG_NEEDED,
+    [MGTPDC]  = MAC_ACCESS_FLAG_NEEDED,    [MGTPTC]  = MAC_ACCESS_FLAG_NEEDED,
+    [RUC]     = MAC_ACCESS_FLAG_NEEDED,    [ROC]     = MAC_ACCESS_FLAG_NEEDED,
+    [GORCL]   = MAC_ACCESS_FLAG_NEEDED,    [GORCH]   = MAC_ACCESS_FLAG_NEEDED,
+    [GOTCL]   = MAC_ACCESS_FLAG_NEEDED,    [GOTCH]   = MAC_ACCESS_FLAG_NEEDED,
+    [BPRC]    = MAC_ACCESS_FLAG_NEEDED,    [MPRC]    = MAC_ACCESS_FLAG_NEEDED,
+    [TSCTC]   = MAC_ACCESS_FLAG_NEEDED,    [PRC64]   = MAC_ACCESS_FLAG_NEEDED,
+    [PRC127]  = MAC_ACCESS_FLAG_NEEDED,    [PRC255]  = MAC_ACCESS_FLAG_NEEDED,
+    [PRC511]  = MAC_ACCESS_FLAG_NEEDED,    [PRC1023] = MAC_ACCESS_FLAG_NEEDED,
+    [PRC1522] = MAC_ACCESS_FLAG_NEEDED,    [PTC64]   = MAC_ACCESS_FLAG_NEEDED,
+    [PTC127]  = MAC_ACCESS_FLAG_NEEDED,    [PTC255]  = MAC_ACCESS_FLAG_NEEDED,
+    [PTC511]  = MAC_ACCESS_FLAG_NEEDED,    [PTC1023] = MAC_ACCESS_FLAG_NEEDED,
+    [PTC1522] = MAC_ACCESS_FLAG_NEEDED,    [MPTC]    = MAC_ACCESS_FLAG_NEEDED,
+    [BPTC]    = MAC_ACCESS_FLAG_NEEDED,
 
-    [TDFH]  = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [TDFT]  = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [TDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [TDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [TDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [RDFH]  = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [RDFT]  = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [RDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [RDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [RDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL,
-    [PBM]   = markflag(MAC) | MAC_ACCESS_PARTIAL,
+    [TDFH]    = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [TDFT]    = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [TDFHS]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [TDFTS]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [TDFPC]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [RDFH]    = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [RDFT]    = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [RDFHS]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [RDFTS]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [RDFPC]   = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
+    [PBM]     = MAC_ACCESS_FLAG_NEEDED | MAC_ACCESS_PARTIAL,
 };
 
 static void
@@ -1419,13 +1416,6 @@ static int e1000_tx_tso_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static bool e1000_full_mac_needed(void *opaque)
-{
-    E1000State *s = opaque;
-
-    return chkflag(MAC);
-}
-
 static bool e1000_tso_state_needed(void *opaque)
 {
     E1000State *s = opaque;
@@ -1451,7 +1441,6 @@ static const VMStateDescription vmstate_e1000_full_mac_state = {
     .name = "e1000/full_mac_state",
     .version_id = 1,
     .minimum_version_id = 1,
-    .needed = e1000_full_mac_needed,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
         VMSTATE_END_OF_LIST()
@@ -1679,8 +1668,6 @@ static void pci_e1000_realize(PCIDevice *pci_dev, Error **errp)
 
 static const Property e1000_properties[] = {
     DEFINE_NIC_PROPERTIES(E1000State, conf),
-    DEFINE_PROP_BIT("extra_mac_registers", E1000State,
-                    compat_flags, E1000_FLAG_MAC_BIT, true),
     DEFINE_PROP_BIT("migrate_tso_props", E1000State,
                     compat_flags, E1000_FLAG_TSO_BIT, true),
     DEFINE_PROP_BIT("init-vet", E1000State,
-- 
2.47.1

[PATCH v4 07/19] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition

[Philippe Mathieu-Daudé]

VIRTIO_PCI_FLAG_MIGRATE_EXTRA was only used by the
hw_compat_2_4[] array, via the 'migrate-extra=true'
property. We removed all machines using that array,
lets remove all the code around VIRTIO_PCI_FLAG_MIGRATE_EXTRA.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/virtio/virtio-pci.h | 4 ----
 hw/virtio/virtio-pci.c         | 6 +-----
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 31ec144509f..d39161766e0 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -32,7 +32,6 @@ DECLARE_OBJ_CHECKERS(VirtioPCIBusState, VirtioPCIBusClass,
 enum {
     VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
-    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT,
     VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
@@ -57,9 +56,6 @@ enum {
 /* virtio version flags */
 #define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
 
-/* migrate extra state */
-#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT)
-
 /* have pio notification for modern device ? */
 #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
     (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 0fa8fe49556..a3e2e007d6c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -146,9 +146,7 @@ static const VMStateDescription vmstate_virtio_pci = {
 
 static bool virtio_pci_has_extra_state(DeviceState *d)
 {
-    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-
-    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
+    return true;
 }
 
 static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
@@ -2349,8 +2347,6 @@ static void virtio_pci_bus_reset_hold(Object *obj, ResetType type)
 static const Property virtio_pci_properties[] = {
     DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false),
-    DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags,
-                    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true),
     DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
     DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
-- 
2.47.1

[PATCH v4 08/19] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Philippe Mathieu-Daudé]

VIRTIO_PCI_FLAG_DISABLE_PCIE was only used by the hw_compat_2_4[]
array, via the 'x-disable-pcie=false' property. We removed all
machines using that array, lets remove all the code around
VIRTIO_PCI_FLAG_DISABLE_PCIE (see commit 9a4c0e220d8 for similar
VIRTIO_PCI_FLAG_* enum removal).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/virtio/virtio-pci.h | 4 ----
 hw/virtio/virtio-pci.c         | 5 +----
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index d39161766e0..f962c9116c1 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -33,7 +33,6 @@ enum {
     VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
     VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
-    VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
     VIRTIO_PCI_FLAG_ATS_BIT,
     VIRTIO_PCI_FLAG_INIT_DEVERR_BIT,
@@ -53,9 +52,6 @@ enum {
  * vcpu thread using ioeventfd for some devices. */
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
 
-/* virtio version flags */
-#define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
-
 /* have pio notification for modern device ? */
 #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
     (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a3e2e007d6c..8d68e56641a 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2349,8 +2349,6 @@ static const Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false),
     DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
-    DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
-                    VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
     DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
     DEFINE_PROP_BOOL("x-ignore-backend-features", VirtIOPCIProxy,
@@ -2379,8 +2377,7 @@ static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp)
     VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
     PCIDevice *pci_dev = &proxy->pci_dev;
 
-    if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) &&
-        virtio_pci_modern(proxy)) {
+    if (virtio_pci_modern(proxy)) {
         pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
     }
 
-- 
2.47.1

Re: [PATCH v4 08/19] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> VIRTIO_PCI_FLAG_DISABLE_PCIE was only used by the hw_compat_2_4[]
> array, via the 'x-disable-pcie=false' property. We removed all
> machines using that array, lets remove all the code around
> VIRTIO_PCI_FLAG_DISABLE_PCIE (see commit 9a4c0e220d8 for similar
> VIRTIO_PCI_FLAG_* enum removal).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   include/hw/virtio/virtio-pci.h | 4 ----
>   hw/virtio/virtio-pci.c         | 5 +----
>   2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index d39161766e0..f962c9116c1 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -33,7 +33,6 @@ enum {
>       VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
>       VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
>       VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
> -    VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
>       VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
>       VIRTIO_PCI_FLAG_ATS_BIT,
>       VIRTIO_PCI_FLAG_INIT_DEVERR_BIT,
> @@ -53,9 +52,6 @@ enum {
>    * vcpu thread using ioeventfd for some devices. */
>   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
>   
> -/* virtio version flags */
> -#define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
> -
>   /* have pio notification for modern device ? */
>   #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
>       (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index a3e2e007d6c..8d68e56641a 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2349,8 +2349,6 @@ static const Property virtio_pci_properties[] = {
>                       VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false),
>       DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
>                       VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
> -    DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
> -                    VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>       DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
>                       VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
>       DEFINE_PROP_BOOL("x-ignore-backend-features", VirtIOPCIProxy,
> @@ -2379,8 +2377,7 @@ static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp)
>       VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
>       PCIDevice *pci_dev = &proxy->pci_dev;
>   
> -    if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) &&
> -        virtio_pci_modern(proxy)) {
> +    if (virtio_pci_modern(proxy)) {
>           pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>       }
>   

[PATCH v4 09/19] hw/i386/pc: Remove deprecated pc-q35-2.5 and pc-i440fx-2.5 machines

[Philippe Mathieu-Daudé]

These machines has been supported for a period of more than 6 years.
According to our versioned machine support policy (see commit
ce80c4fa6ff "docs: document special exception for machine type
deprecation & removal") they can now be removed.

Remove the now unused empty pc_compat_2_5[] array.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/i386/pc.h |  3 ---
 hw/i386/pc.c         |  3 ---
 hw/i386/pc_piix.c    | 13 -------------
 hw/i386/pc_q35.c     | 13 -------------
 4 files changed, 32 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b34aa25fdce..79b72c54dd3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -298,9 +298,6 @@ extern const size_t pc_compat_2_7_len;
 extern GlobalProperty pc_compat_2_6[];
 extern const size_t pc_compat_2_6_len;
 
-extern GlobalProperty pc_compat_2_5[];
-extern const size_t pc_compat_2_5_len;
-
 #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
     static void pc_machine_##suffix##_class_init(ObjectClass *oc, \
                                                  const void *data) \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2b46714a5ac..cb375aabdc5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -259,9 +259,6 @@ GlobalProperty pc_compat_2_6[] = {
 };
 const size_t pc_compat_2_6_len = G_N_ELEMENTS(pc_compat_2_6);
 
-GlobalProperty pc_compat_2_5[] = {};
-const size_t pc_compat_2_5_len = G_N_ELEMENTS(pc_compat_2_5);
-
 /*
  * @PC_FW_DATA:
  * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 04213b45b44..7a62bb06500 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -778,19 +778,6 @@ static void pc_i440fx_machine_2_6_options(MachineClass *m)
 
 DEFINE_I440FX_MACHINE(2, 6);
 
-static void pc_i440fx_machine_2_5_options(MachineClass *m)
-{
-    X86MachineClass *x86mc = X86_MACHINE_CLASS(m);
-
-    pc_i440fx_machine_2_6_options(m);
-    x86mc->save_tsc_khz = false;
-    m->legacy_fw_cfg_order = 1;
-    compat_props_add(m->compat_props, hw_compat_2_5, hw_compat_2_5_len);
-    compat_props_add(m->compat_props, pc_compat_2_5, pc_compat_2_5_len);
-}
-
-DEFINE_I440FX_MACHINE(2, 5);
-
 #ifdef CONFIG_ISAPC
 static void isapc_machine_options(MachineClass *m)
 {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 47e12602413..33211b1876f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -672,16 +672,3 @@ static void pc_q35_machine_2_6_options(MachineClass *m)
 }
 
 DEFINE_Q35_MACHINE(2, 6);
-
-static void pc_q35_machine_2_5_options(MachineClass *m)
-{
-    X86MachineClass *x86mc = X86_MACHINE_CLASS(m);
-
-    pc_q35_machine_2_6_options(m);
-    x86mc->save_tsc_khz = false;
-    m->legacy_fw_cfg_order = 1;
-    compat_props_add(m->compat_props, hw_compat_2_5, hw_compat_2_5_len);
-    compat_props_add(m->compat_props, pc_compat_2_5, pc_compat_2_5_len);
-}
-
-DEFINE_Q35_MACHINE(2, 5);
-- 
2.47.1

Re: [PATCH v4 09/19] hw/i386/pc: Remove deprecated pc-q35-2.5 and pc-i440fx-2.5 machines

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> These machines has been supported for a period of more than 6 years.
> According to our versioned machine support policy (see commit
> ce80c4fa6ff "docs: document special exception for machine type
> deprecation & removal") they can now be removed.
> 
> Remove the now unused empty pc_compat_2_5[] array.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

[PATCH v4 10/19] hw/i386/x86: Remove X86MachineClass::save_tsc_khz field

[Philippe Mathieu-Daudé]

The X86MachineClass::save_tsc_khz boolean was only used
by the pc-q35-2.5 and pc-i440fx-2.5 machines, which got
removed. Remove it and simplify tsc_khz_needed().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/i386/x86.h | 5 -----
 hw/i386/x86.c         | 1 -
 target/i386/machine.c | 5 ++---
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 258b1343a16..fc460b82f82 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -27,13 +27,8 @@
 #include "qom/object.h"
 
 struct X86MachineClass {
-    /*< private >*/
     MachineClass parent;
 
-    /*< public >*/
-
-    /* TSC rate migration: */
-    bool save_tsc_khz;
     /* use DMA capable linuxboot option rom */
     bool fwcfg_dma_enabled;
     /* CPU and apic information: */
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index e2d04092992..f80533df1c5 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -382,7 +382,6 @@ static void x86_machine_class_init(ObjectClass *oc, const void *data)
     mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
     mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
     mc->kvm_type = x86_kvm_type;
-    x86mc->save_tsc_khz = true;
     x86mc->fwcfg_dma_enabled = true;
     nc->nmi_monitor_handler = x86_nmi;
 
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 6cb561c6322..dd2dac1d443 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1060,9 +1060,8 @@ static bool tsc_khz_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
     CPUX86State *env = &cpu->env;
-    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-    X86MachineClass *x86mc = X86_MACHINE_CLASS(mc);
-    return env->tsc_khz && x86mc->save_tsc_khz;
+
+    return env->tsc_khz;
 }
 
 static const VMStateDescription vmstate_tsc_khz = {
-- 
2.47.1

Re: [PATCH v4 10/19] hw/i386/x86: Remove X86MachineClass::save_tsc_khz field

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The X86MachineClass::save_tsc_khz boolean was only used
> by the pc-q35-2.5 and pc-i440fx-2.5 machines, which got
> removed. Remove it and simplify tsc_khz_needed().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

[PATCH v4 11/19] hw/nvram/fw_cfg: Remove legacy FW_CFG_ORDER_OVERRIDE

[Philippe Mathieu-Daudé]

The MachineClass::legacy_fw_cfg_order boolean was only used
by the pc-q35-2.5 and pc-i440fx-2.5 machines, which got
removed. Remove it along with:

- FW_CFG_ORDER_OVERRIDE_* definitions
- fw_cfg_set_order_override()
- fw_cfg_reset_order_override()
- fw_cfg_order[]
- rom_set_order_override()
- rom_reset_order_override()

Simplify CLI and pc_vga_init() / pc_nic_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/boards.h       |   3 +-
 include/hw/loader.h       |   2 -
 include/hw/nvram/fw_cfg.h |  10 ----
 hw/core/loader.c          |  14 -----
 hw/i386/pc.c              |   7 +--
 hw/nvram/fw_cfg.c         | 109 +++-----------------------------------
 system/vl.c               |   5 --
 7 files changed, 9 insertions(+), 141 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 03e7cbeae82..ab900dacabc 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -286,8 +286,7 @@ struct MachineClass {
         no_parallel:1,
         no_floppy:1,
         no_cdrom:1,
-        pci_allow_0_address:1,
-        legacy_fw_cfg_order:1;
+        pci_allow_0_address:1;
     bool auto_create_sdcard;
     bool is_default;
     const char *default_machine_opts;
diff --git a/include/hw/loader.h b/include/hw/loader.h
index d280dc33e96..c96b5e141c6 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -270,8 +270,6 @@ int rom_add_elf_program(const char *name, GMappedFile *mapped_file, void *data,
                         AddressSpace *as);
 int rom_check_and_register_reset(void);
 void rom_set_fw(FWCfgState *f);
-void rom_set_order_override(int order);
-void rom_reset_order_override(void);
 
 /**
  * rom_transaction_begin:
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 47578ccc7f2..d41b9328fd1 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -42,14 +42,6 @@ struct FWCfgDataGeneratorClass {
 
 typedef struct fw_cfg_file FWCfgFile;
 
-#define FW_CFG_ORDER_OVERRIDE_VGA    70
-#define FW_CFG_ORDER_OVERRIDE_NIC    80
-#define FW_CFG_ORDER_OVERRIDE_USER   100
-#define FW_CFG_ORDER_OVERRIDE_DEVICE 110
-
-void fw_cfg_set_order_override(FWCfgState *fw_cfg, int order);
-void fw_cfg_reset_order_override(FWCfgState *fw_cfg);
-
 typedef struct FWCfgFiles {
     uint32_t  count;
     FWCfgFile f[];
@@ -75,8 +67,6 @@ struct FWCfgState {
     uint32_t cur_offset;
     Notifier machine_ready;
 
-    int fw_cfg_order_override;
-
     bool dma_enabled;
     dma_addr_t dma_addr;
     AddressSpace *dma_as;
diff --git a/hw/core/loader.c b/hw/core/loader.c
index b792a54bb02..e7056ba4bd3 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1333,20 +1333,6 @@ void rom_set_fw(FWCfgState *f)
     fw_cfg = f;
 }
 
-void rom_set_order_override(int order)
-{
-    if (!fw_cfg)
-        return;
-    fw_cfg_set_order_override(fw_cfg, order);
-}
-
-void rom_reset_order_override(void)
-{
-    if (!fw_cfg)
-        return;
-    fw_cfg_reset_order_override(fw_cfg);
-}
-
 void rom_transaction_begin(void)
 {
     Rom *rom;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cb375aabdc5..49632b69d29 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1033,7 +1033,6 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
 {
     DeviceState *dev = NULL;
 
-    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_VGA);
     if (pci_bus) {
         PCIDevice *pcidev = pci_vga_init(pci_bus);
         dev = pcidev ? &pcidev->qdev : NULL;
@@ -1041,7 +1040,7 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
         ISADevice *isadev = isa_vga_init(isa_bus);
         dev = isadev ? DEVICE(isadev) : NULL;
     }
-    rom_reset_order_override();
+
     return dev;
 }
 
@@ -1231,8 +1230,6 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
     bool default_is_ne2k = g_str_equal(mc->default_nic, TYPE_ISA_NE2000);
     NICInfo *nd;
 
-    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_NIC);
-
     while ((nd = qemu_find_nic_info(TYPE_ISA_NE2000, default_is_ne2k, NULL))) {
         pc_init_ne2k_isa(isa_bus, nd, &error_fatal);
     }
@@ -1241,8 +1238,6 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
     if (pci_bus) {
         pci_init_nic_devices(pci_bus, mc->default_nic);
     }
-
-    rom_reset_order_override();
 }
 
 void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 237b9f7d1fe..54cfa07d3f5 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -817,62 +817,6 @@ void fw_cfg_modify_i64(FWCfgState *s, uint16_t key, uint64_t value)
     g_free(old);
 }
 
-void fw_cfg_set_order_override(FWCfgState *s, int order)
-{
-    assert(s->fw_cfg_order_override == 0);
-    s->fw_cfg_order_override = order;
-}
-
-void fw_cfg_reset_order_override(FWCfgState *s)
-{
-    assert(s->fw_cfg_order_override != 0);
-    s->fw_cfg_order_override = 0;
-}
-
-/*
- * This is the legacy order list.  For legacy systems, files are in
- * the fw_cfg in the order defined below, by the "order" value.  Note
- * that some entries (VGA ROMs, NIC option ROMS, etc.) go into a
- * specific area, but there may be more than one and they occur in the
- * order that the user specifies them on the command line.  Those are
- * handled in a special manner, using the order override above.
- *
- * For non-legacy, the files are sorted by filename to avoid this kind
- * of complexity in the future.
- *
- * This is only for x86, other arches don't implement versioning so
- * they won't set legacy mode.
- */
-static struct {
-    const char *name;
-    int order;
-} fw_cfg_order[] = {
-    { "etc/boot-menu-wait", 10 },
-    { "bootsplash.jpg", 11 },
-    { "bootsplash.bmp", 12 },
-    { "etc/boot-fail-wait", 15 },
-    { "etc/smbios/smbios-tables", 20 },
-    { "etc/smbios/smbios-anchor", 30 },
-    { "etc/e820", 40 },
-    { "etc/reserved-memory-end", 50 },
-    { "genroms/kvmvapic.bin", 55 },
-    { "genroms/linuxboot.bin", 60 },
-    { }, /* VGA ROMs from pc_vga_init come here, 70. */
-    { }, /* NIC option ROMs from pc_nic_init come here, 80. */
-    { "etc/system-states", 90 },
-    { }, /* User ROMs come here, 100. */
-    { }, /* Device FW comes here, 110. */
-    { "etc/extra-pci-roots", 120 },
-    { "etc/acpi/tables", 130 },
-    { "etc/table-loader", 140 },
-    { "etc/tpm/log", 150 },
-    { "etc/acpi/rsdp", 160 },
-    { "bootorder", 170 },
-    { "etc/msr_feature_control", 180 },
-
-#define FW_CFG_ORDER_OVERRIDE_LAST 200
-};
-
 /*
  * Any sub-page size update to these table MRs will be lost during migration,
  * as we use aligned size in ram_load_precopy() -> qemu_ram_resize() path.
@@ -890,29 +834,6 @@ static void fw_cfg_acpi_mr_save(FWCfgState *s, const char *filename, size_t len)
     }
 }
 
-static int get_fw_cfg_order(FWCfgState *s, const char *name)
-{
-    int i;
-
-    if (s->fw_cfg_order_override > 0) {
-        return s->fw_cfg_order_override;
-    }
-
-    for (i = 0; i < ARRAY_SIZE(fw_cfg_order); i++) {
-        if (fw_cfg_order[i].name == NULL) {
-            continue;
-        }
-
-        if (strcmp(name, fw_cfg_order[i].name) == 0) {
-            return fw_cfg_order[i].order;
-        }
-    }
-
-    /* Stick unknown stuff at the end. */
-    warn_report("Unknown firmware file in legacy mode: %s", name);
-    return FW_CFG_ORDER_OVERRIDE_LAST;
-}
-
 void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
                               FWCfgCallback select_cb,
                               FWCfgWriteCallback write_cb,
@@ -921,7 +842,6 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
 {
     int i, index, count;
     size_t dsize;
-    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
     int order = 0;
 
     if (!s->files) {
@@ -933,22 +853,10 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
     count = be32_to_cpu(s->files->count);
     assert(count < fw_cfg_file_slots(s));
 
-    /* Find the insertion point. */
-    if (mc->legacy_fw_cfg_order) {
-        /*
-         * Sort by order. For files with the same order, we keep them
-         * in the sequence in which they were added.
-         */
-        order = get_fw_cfg_order(s, filename);
-        for (index = count;
-             index > 0 && order < s->entry_order[index - 1];
-             index--);
-    } else {
-        /* Sort by file name. */
-        for (index = count;
-             index > 0 && strcmp(filename, s->files->f[index - 1].name) < 0;
-             index--);
-    }
+    /* Find the insertion point, sorting by file name. */
+    for (index = count;
+         index > 0 && strcmp(filename, s->files->f[index - 1].name) < 0;
+         index--);
 
     /*
      * Move all the entries from the index point and after down one
@@ -1058,7 +966,6 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
 
 static void fw_cfg_machine_reset(void *opaque)
 {
-    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
     FWCfgState *s = opaque;
     void *ptr;
     size_t len;
@@ -1068,11 +975,9 @@ static void fw_cfg_machine_reset(void *opaque)
     ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
     g_free(ptr);
 
-    if (!mc->legacy_fw_cfg_order) {
-        buf = get_boot_devices_lchs_list(&len);
-        ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
-        g_free(ptr);
-    }
+    buf = get_boot_devices_lchs_list(&len);
+    ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+    g_free(ptr);
 }
 
 static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/system/vl.c b/system/vl.c
index fd402b8ff8f..3b7057e6c66 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1192,10 +1192,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
             return -1;
         }
     }
-    /* For legacy, keep user files in a specific global order. */
-    fw_cfg_set_order_override(fw_cfg, FW_CFG_ORDER_OVERRIDE_USER);
     fw_cfg_add_file(fw_cfg, name, buf, size);
-    fw_cfg_reset_order_override(fw_cfg);
     return 0;
 }
 
@@ -2745,7 +2742,6 @@ static void qemu_create_cli_devices(void)
     }
 
     /* init generic devices */
-    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
     qemu_opts_foreach(qemu_find_opts("device"),
                       device_init_func, NULL, &error_fatal);
     QTAILQ_FOREACH(opt, &device_opts, next) {
@@ -2756,7 +2752,6 @@ static void qemu_create_cli_devices(void)
         assert(ret_data == NULL); /* error_fatal aborts */
         loc_pop(&opt->loc);
     }
-    rom_reset_order_override();
 }
 
 static bool qemu_machine_creation_done(Error **errp)
-- 
2.47.1

Re: [PATCH v4 11/19] hw/nvram/fw_cfg: Remove legacy FW_CFG_ORDER_OVERRIDE

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The MachineClass::legacy_fw_cfg_order boolean was only used
> by the pc-q35-2.5 and pc-i440fx-2.5 machines, which got
> removed. Remove it along with:
> 
> - FW_CFG_ORDER_OVERRIDE_* definitions
> - fw_cfg_set_order_override()
> - fw_cfg_reset_order_override()
> - fw_cfg_order[]
> - rom_set_order_override()
> - rom_reset_order_override()
> 
> Simplify CLI and pc_vga_init() / pc_nic_init().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

[PATCH v4 12/19] hw/core/machine: Remove hw_compat_2_5[] array

[Philippe Mathieu-Daudé]

The hw_compat_2_5[] array was only used by the pc-q35-2.5 and
pc-i440fx-2.5 machines, which got removed. Remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/boards.h | 3 ---
 hw/core/machine.c   | 9 ---------
 2 files changed, 12 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index ab900dacabc..f424b2b5058 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -862,7 +862,4 @@ extern const size_t hw_compat_2_7_len;
 extern GlobalProperty hw_compat_2_6[];
 extern const size_t hw_compat_2_6_len;
 
-extern GlobalProperty hw_compat_2_5[];
-extern const size_t hw_compat_2_5_len;
-
 #endif
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d66b02b564c..e7001bf92cd 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -283,15 +283,6 @@ GlobalProperty hw_compat_2_6[] = {
 };
 const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6);
 
-GlobalProperty hw_compat_2_5[] = {
-    { "isa-fdc", "fallback", "144" },
-    { "pvscsi", "x-old-pci-configuration", "on" },
-    { "pvscsi", "x-disable-pcie", "on" },
-    { "vmxnet3", "x-old-msi-offsets", "on" },
-    { "vmxnet3", "x-disable-pcie", "on" },
-};
-const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
-
 MachineState *current_machine;
 
 static char *machine_get_kernel(Object *obj, Error **errp)
-- 
2.47.1

Re: [PATCH v4 12/19] hw/core/machine: Remove hw_compat_2_5[] array

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> The hw_compat_2_5[] array was only used by the pc-q35-2.5 and
> pc-i440fx-2.5 machines, which got removed. Remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

[PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Philippe Mathieu-Daudé]

The "fallback" property was only used by the hw_compat_2_5[] array,
as 'fallback=144'. We removed all machines using that array, lets
remove ISA floppy drive 'fallback' property, manually setting the
default value in isabus_fdc_realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/block/fdc-isa.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/block/fdc-isa.c b/hw/block/fdc-isa.c
index 6d1790e0e61..5d746d73165 100644
--- a/hw/block/fdc-isa.c
+++ b/hw/block/fdc-isa.c
@@ -97,6 +97,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
                              isa->iobase, fdc_portio_list, fdctrl,
                              "fdc");
 
+    fdctrl->fallback = FLOPPY_DRIVE_TYPE_288;
     fdctrl->irq = isa_bus_get_irq(bus, isa->irq);
     fdctrl->dma_chann = isa->dma;
     if (fdctrl->dma_chann != -1) {
@@ -293,9 +294,6 @@ static const Property isa_fdc_properties[] = {
     DEFINE_PROP_SIGNED("fdtypeB", FDCtrlISABus, state.qdev_for_drives[1].type,
                         FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
-    DEFINE_PROP_SIGNED("fallback", FDCtrlISABus, state.fallback,
-                        FLOPPY_DRIVE_TYPE_288, qdev_prop_fdc_drive_type,
-                        FloppyDriveType),
 };
 
 static void isabus_fdc_class_init(ObjectClass *klass, const void *data)
-- 
2.47.1

Re: [PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Thomas Huth]

On 12/05/2025 10.39, Philippe Mathieu-Daudé wrote:
> The "fallback" property was only used by the hw_compat_2_5[] array,
> as 'fallback=144'. We removed all machines using that array, lets
> remove ISA floppy drive 'fallback' property, manually setting the
> default value in isabus_fdc_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   hw/block/fdc-isa.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

FWIW, this needs a fixup for iotest 172:

  https://gitlab.com/thuth/qemu/-/jobs/10166450223#L466

  Thomas

Re: [PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Thomas Huth]

On 27/05/2025 19.20, Thomas Huth wrote:
> On 12/05/2025 10.39, Philippe Mathieu-Daudé wrote:
>> The "fallback" property was only used by the hw_compat_2_5[] array,
>> as 'fallback=144'. We removed all machines using that array, lets
>> remove ISA floppy drive 'fallback' property, manually setting the
>> default value in isabus_fdc_realize().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>> ---
>>   hw/block/fdc-isa.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> FWIW, this needs a fixup for iotest 172:
> 
>   https://gitlab.com/thuth/qemu/-/jobs/10166450223#L466

FYI, since I was testing this series anyway, I'll fix up this patch with:

diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -14,7 +14,6 @@ Testing:
              dma = 2 (0x2)
              fdtypeA = "auto"
              fdtypeB = "auto"
-            fallback = "288"
              bus: floppy-bus.0
                type floppy-bus
                dev: floppy, id ""
@@ -43,7 +42,6 @@ Testing: -fda TEST_DIR/t.qcow2
              dma = 2 (0x2)
              fdtypeA = "auto"
              fdtypeB = "auto"
-            fallback = "288"
              bus: floppy-bus.0
                type floppy-bus
                dev: floppy, id ""
@@ -79,7 +77,6 @@ Testing: -fdb TEST_DIR/t.qcow2
              dma = 2 (0x2)
              fdtypeA = "auto"
              fdtypeB = "auto"
-            fallback = "288"
              bus: floppy-bus.0
                type floppy-bus
                dev: floppy, id ""

etc.

and I will queue this series (without the "Remove X86CPU::check_cpuid field" 
patch as mentioned by Xiaoyao Li), unless Paolo or another x86 maintainer 
wants to do this instead (please let me know!).

  Thomas

Re: [PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Thomas Huth]

On 28/05/2025 10.30, Thomas Huth wrote:
> On 27/05/2025 19.20, Thomas Huth wrote:
>> On 12/05/2025 10.39, Philippe Mathieu-Daudé wrote:
>>> The "fallback" property was only used by the hw_compat_2_5[] array,
>>> as 'fallback=144'. We removed all machines using that array, lets
>>> remove ISA floppy drive 'fallback' property, manually setting the
>>> default value in isabus_fdc_realize().
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>>> ---
>>>   hw/block/fdc-isa.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> FWIW, this needs a fixup for iotest 172:
>>
>>   https://gitlab.com/thuth/qemu/-/jobs/10166450223#L466
> 
> FYI, since I was testing this series anyway, I'll fix up this patch
...
> and I will queue this series (without the "Remove X86CPU::check_cpuid field" 
> patch as mentioned by Xiaoyao Li), unless Paolo or another x86 maintainer 
> wants to do this instead (please let me know!).

Hmm, looking at this twice, hw/block/fdc-sysbus.c has a "fallback" property, 
too, and there are people out there who set this property manually, e.g.:

  https://github.com/mariuz/linux-0.01/issues/4#issuecomment-974768898

So it seems like this property is meant as a config knob for the user, 
indeed, not just an internal one. I'll drop this patch for now.

  Thomas

Re: [PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Michael S. Tsirkin]

On Wed, May 28, 2025 at 10:30:17AM +0200, Thomas Huth wrote:
> On 27/05/2025 19.20, Thomas Huth wrote:
> > On 12/05/2025 10.39, Philippe Mathieu-Daudé wrote:
> > > The "fallback" property was only used by the hw_compat_2_5[] array,
> > > as 'fallback=144'. We removed all machines using that array, lets
> > > remove ISA floppy drive 'fallback' property, manually setting the
> > > default value in isabus_fdc_realize().
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > > ---
> > >   hw/block/fdc-isa.c | 4 +---
> > >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > FWIW, this needs a fixup for iotest 172:
> > 
> >   https://gitlab.com/thuth/qemu/-/jobs/10166450223#L466
> 
> FYI, since I was testing this series anyway, I'll fix up this patch with:
> 
> diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
> --- a/tests/qemu-iotests/172.out
> +++ b/tests/qemu-iotests/172.out
> @@ -14,7 +14,6 @@ Testing:
>              dma = 2 (0x2)
>              fdtypeA = "auto"
>              fdtypeB = "auto"
> -            fallback = "288"
>              bus: floppy-bus.0
>                type floppy-bus
>                dev: floppy, id ""
> @@ -43,7 +42,6 @@ Testing: -fda TEST_DIR/t.qcow2
>              dma = 2 (0x2)
>              fdtypeA = "auto"
>              fdtypeB = "auto"
> -            fallback = "288"
>              bus: floppy-bus.0
>                type floppy-bus
>                dev: floppy, id ""
> @@ -79,7 +77,6 @@ Testing: -fdb TEST_DIR/t.qcow2
>              dma = 2 (0x2)
>              fdtypeA = "auto"
>              fdtypeB = "auto"
> -            fallback = "288"
>              bus: floppy-bus.0
>                type floppy-bus
>                dev: floppy, id ""
> 
> etc.
> 
> and I will queue this series (without the "Remove X86CPU::check_cpuid field"
> patch as mentioned by Xiaoyao Li), unless Paolo or another x86 maintainer
> wants to do this instead (please let me know!).
> 
>  Thomas

No, pls go ahead.

Re: [PATCH v4 13/19] hw/block/fdc-isa: Remove 'fallback' property

[Philippe Mathieu-Daudé]

On 30/5/25 13:45, Michael S. Tsirkin wrote:
> On Wed, May 28, 2025 at 10:30:17AM +0200, Thomas Huth wrote:
>> On 27/05/2025 19.20, Thomas Huth wrote:
>>> On 12/05/2025 10.39, Philippe Mathieu-Daudé wrote:
>>>> The "fallback" property was only used by the hw_compat_2_5[] array,
>>>> as 'fallback=144'. We removed all machines using that array, lets
>>>> remove ISA floppy drive 'fallback' property, manually setting the
>>>> default value in isabus_fdc_realize().
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>>>> ---
>>>>    hw/block/fdc-isa.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> FWIW, this needs a fixup for iotest 172:
>>>
>>>    https://gitlab.com/thuth/qemu/-/jobs/10166450223#L466


>> and I will queue this series (without the "Remove X86CPU::check_cpuid field"
>> patch as mentioned by Xiaoyao Li), unless Paolo or another x86 maintainer
>> wants to do this instead (please let me know!).
>>
>>   Thomas
> 
> No, pls go ahead.

Thanks both!

[PATCH v4 14/19] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION definition

[Philippe Mathieu-Daudé]

PVSCSI_COMPAT_OLD_PCI_CONFIGURATION was only used by the
hw_compat_2_5[] array, via the 'x-old-pci-configuration=on'
property. We removed all machines using that array, lets remove
all the code around PVSCSI_COMPAT_OLD_PCI_CONFIGURATION.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/scsi/vmw_pvscsi.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index d5825b67868..34de59a7cf6 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -69,17 +69,11 @@ OBJECT_DECLARE_TYPE(PVSCSIState, PVSCSIClass, PVSCSI)
 
 
 /* Compatibility flags for migration */
-#define PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT 0
-#define PVSCSI_COMPAT_OLD_PCI_CONFIGURATION \
-    (1 << PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT)
 #define PVSCSI_COMPAT_DISABLE_PCIE_BIT 1
 #define PVSCSI_COMPAT_DISABLE_PCIE \
     (1 << PVSCSI_COMPAT_DISABLE_PCIE_BIT)
 
-#define PVSCSI_USE_OLD_PCI_CONFIGURATION(s) \
-    ((s)->compat_flags & PVSCSI_COMPAT_OLD_PCI_CONFIGURATION)
-#define PVSCSI_MSI_OFFSET(s) \
-    (PVSCSI_USE_OLD_PCI_CONFIGURATION(s) ? 0x50 : 0x7c)
+#define PVSCSI_MSI_OFFSET    (0x7c)
 #define PVSCSI_EXP_EP_OFFSET (0x40)
 
 typedef struct PVSCSIRingInfo {
@@ -1110,7 +1104,7 @@ pvscsi_init_msi(PVSCSIState *s)
     int res;
     PCIDevice *d = PCI_DEVICE(s);
 
-    res = msi_init(d, PVSCSI_MSI_OFFSET(s), PVSCSI_MSIX_NUM_VECTORS,
+    res = msi_init(d, PVSCSI_MSI_OFFSET, PVSCSI_MSIX_NUM_VECTORS,
                    PVSCSI_USE_64BIT, PVSCSI_PER_VECTOR_MASK, NULL);
     if (res < 0) {
         trace_pvscsi_init_msi_fail(res);
@@ -1158,15 +1152,11 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
     trace_pvscsi_state("init");
 
     /* PCI subsystem ID, subsystem vendor ID, revision */
-    if (PVSCSI_USE_OLD_PCI_CONFIGURATION(s)) {
-        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID, 0x1000);
-    } else {
-        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID,
-                     PCI_VENDOR_ID_VMWARE);
-        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID,
-                     PCI_DEVICE_ID_VMWARE_PVSCSI);
-        pci_config_set_revision(pci_dev->config, 0x2);
-    }
+    pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID,
+                 PCI_VENDOR_ID_VMWARE);
+    pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID,
+                 PCI_DEVICE_ID_VMWARE_PVSCSI);
+    pci_config_set_revision(pci_dev->config, 0x2);
 
     /* PCI latency timer = 255 */
     pci_dev->config[PCI_LATENCY_TIMER] = 0xff;
@@ -1298,8 +1288,6 @@ static const VMStateDescription vmstate_pvscsi = {
 
 static const Property pvscsi_properties[] = {
     DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
-    DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
-                    PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
     DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
                     PVSCSI_COMPAT_DISABLE_PCIE_BIT, false),
 };
-- 
2.47.1

Re: [PATCH v4 14/19] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION definition

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> PVSCSI_COMPAT_OLD_PCI_CONFIGURATION was only used by the
> hw_compat_2_5[] array, via the 'x-old-pci-configuration=on'
> property. We removed all machines using that array, lets remove
> all the code around PVSCSI_COMPAT_OLD_PCI_CONFIGURATION.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/scsi/vmw_pvscsi.c | 26 +++++++-------------------
>   1 file changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index d5825b67868..34de59a7cf6 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -69,17 +69,11 @@ OBJECT_DECLARE_TYPE(PVSCSIState, PVSCSIClass, PVSCSI)
>   
>   
>   /* Compatibility flags for migration */
> -#define PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT 0
> -#define PVSCSI_COMPAT_OLD_PCI_CONFIGURATION \
> -    (1 << PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT)
>   #define PVSCSI_COMPAT_DISABLE_PCIE_BIT 1
>   #define PVSCSI_COMPAT_DISABLE_PCIE \
>       (1 << PVSCSI_COMPAT_DISABLE_PCIE_BIT)
>   
> -#define PVSCSI_USE_OLD_PCI_CONFIGURATION(s) \
> -    ((s)->compat_flags & PVSCSI_COMPAT_OLD_PCI_CONFIGURATION)
> -#define PVSCSI_MSI_OFFSET(s) \
> -    (PVSCSI_USE_OLD_PCI_CONFIGURATION(s) ? 0x50 : 0x7c)
> +#define PVSCSI_MSI_OFFSET    (0x7c)
>   #define PVSCSI_EXP_EP_OFFSET (0x40)
>   
>   typedef struct PVSCSIRingInfo {
> @@ -1110,7 +1104,7 @@ pvscsi_init_msi(PVSCSIState *s)
>       int res;
>       PCIDevice *d = PCI_DEVICE(s);
>   
> -    res = msi_init(d, PVSCSI_MSI_OFFSET(s), PVSCSI_MSIX_NUM_VECTORS,
> +    res = msi_init(d, PVSCSI_MSI_OFFSET, PVSCSI_MSIX_NUM_VECTORS,
>                      PVSCSI_USE_64BIT, PVSCSI_PER_VECTOR_MASK, NULL);
>       if (res < 0) {
>           trace_pvscsi_init_msi_fail(res);
> @@ -1158,15 +1152,11 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
>       trace_pvscsi_state("init");
>   
>       /* PCI subsystem ID, subsystem vendor ID, revision */
> -    if (PVSCSI_USE_OLD_PCI_CONFIGURATION(s)) {
> -        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID, 0x1000);
> -    } else {
> -        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID,
> -                     PCI_VENDOR_ID_VMWARE);
> -        pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID,
> -                     PCI_DEVICE_ID_VMWARE_PVSCSI);
> -        pci_config_set_revision(pci_dev->config, 0x2);
> -    }
> +    pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID,
> +                 PCI_VENDOR_ID_VMWARE);
> +    pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID,
> +                 PCI_DEVICE_ID_VMWARE_PVSCSI);
> +    pci_config_set_revision(pci_dev->config, 0x2);
>   
>       /* PCI latency timer = 255 */
>       pci_dev->config[PCI_LATENCY_TIMER] = 0xff;
> @@ -1298,8 +1288,6 @@ static const VMStateDescription vmstate_pvscsi = {
>   
>   static const Property pvscsi_properties[] = {
>       DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> -    DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
> -                    PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
>       DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
>                       PVSCSI_COMPAT_DISABLE_PCIE_BIT, false),
>   };

[PATCH v4 15/19] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Philippe Mathieu-Daudé]

PVSCSI_COMPAT_DISABLE_PCIE_BIT was only used by the
hw_compat_2_5[] array, via the 'x-disable-pcie=on' property.
We removed all machines using that array, lets remove all the
code around PVSCSI_COMPAT_DISABLE_PCIE_BIT, including the now
unused PVSCSIState::compat_flags field.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/scsi/vmw_pvscsi.c | 30 +-----------------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 34de59a7cf6..e163023d14c 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -68,11 +68,6 @@ struct PVSCSIClass {
 OBJECT_DECLARE_TYPE(PVSCSIState, PVSCSIClass, PVSCSI)
 
 
-/* Compatibility flags for migration */
-#define PVSCSI_COMPAT_DISABLE_PCIE_BIT 1
-#define PVSCSI_COMPAT_DISABLE_PCIE \
-    (1 << PVSCSI_COMPAT_DISABLE_PCIE_BIT)
-
 #define PVSCSI_MSI_OFFSET    (0x7c)
 #define PVSCSI_EXP_EP_OFFSET (0x40)
 
@@ -123,8 +118,6 @@ struct PVSCSIState {
     uint8_t msi_used;                    /* For migration compatibility      */
     PVSCSIRingInfo rings;                /* Data transfer rings manager      */
     uint32_t resetting;                  /* Reset in progress                */
-
-    uint32_t compat_flags;
 };
 
 typedef struct PVSCSIRequest {
@@ -1224,21 +1217,8 @@ pvscsi_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static bool pvscsi_vmstate_need_pcie_device(void *opaque)
-{
-    PVSCSIState *s = PVSCSI(opaque);
-
-    return !(s->compat_flags & PVSCSI_COMPAT_DISABLE_PCIE);
-}
-
-static bool pvscsi_vmstate_test_pci_device(void *opaque, int version_id)
-{
-    return !pvscsi_vmstate_need_pcie_device(opaque);
-}
-
 static const VMStateDescription vmstate_pvscsi_pcie_device = {
     .name = "pvscsi/pcie",
-    .needed = pvscsi_vmstate_need_pcie_device,
     .fields = (const VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
         VMSTATE_END_OF_LIST()
@@ -1252,9 +1232,6 @@ static const VMStateDescription vmstate_pvscsi = {
     .pre_save = pvscsi_pre_save,
     .post_load = pvscsi_post_load,
     .fields = (const VMStateField[]) {
-        VMSTATE_STRUCT_TEST(parent_obj, PVSCSIState,
-                            pvscsi_vmstate_test_pci_device, 0,
-                            vmstate_pci_device, PCIDevice),
         VMSTATE_UINT8(msi_used, PVSCSIState),
         VMSTATE_UINT32(resetting, PVSCSIState),
         VMSTATE_UINT64(reg_interrupt_status, PVSCSIState),
@@ -1288,19 +1265,14 @@ static const VMStateDescription vmstate_pvscsi = {
 
 static const Property pvscsi_properties[] = {
     DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
-    DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
-                    PVSCSI_COMPAT_DISABLE_PCIE_BIT, false),
 };
 
 static void pvscsi_realize(DeviceState *qdev, Error **errp)
 {
     PVSCSIClass *pvs_c = PVSCSI_GET_CLASS(qdev);
     PCIDevice *pci_dev = PCI_DEVICE(qdev);
-    PVSCSIState *s = PVSCSI(qdev);
 
-    if (!(s->compat_flags & PVSCSI_COMPAT_DISABLE_PCIE)) {
-        pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
-    }
+    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
 
     pvs_c->parent_dc_realize(qdev, errp);
 }
-- 
2.47.1

Re: [PATCH v4 15/19] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> PVSCSI_COMPAT_DISABLE_PCIE_BIT was only used by the
> hw_compat_2_5[] array, via the 'x-disable-pcie=on' property.
> We removed all machines using that array, lets remove all the
> code around PVSCSI_COMPAT_DISABLE_PCIE_BIT, including the now
> unused PVSCSIState::compat_flags field.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/scsi/vmw_pvscsi.c | 30 +-----------------------------
>   1 file changed, 1 insertion(+), 29 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 34de59a7cf6..e163023d14c 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -68,11 +68,6 @@ struct PVSCSIClass {
>   OBJECT_DECLARE_TYPE(PVSCSIState, PVSCSIClass, PVSCSI)
>   
>   
> -/* Compatibility flags for migration */
> -#define PVSCSI_COMPAT_DISABLE_PCIE_BIT 1
> -#define PVSCSI_COMPAT_DISABLE_PCIE \
> -    (1 << PVSCSI_COMPAT_DISABLE_PCIE_BIT)
> -
>   #define PVSCSI_MSI_OFFSET    (0x7c)
>   #define PVSCSI_EXP_EP_OFFSET (0x40)
>   
> @@ -123,8 +118,6 @@ struct PVSCSIState {
>       uint8_t msi_used;                    /* For migration compatibility      */
>       PVSCSIRingInfo rings;                /* Data transfer rings manager      */
>       uint32_t resetting;                  /* Reset in progress                */
> -
> -    uint32_t compat_flags;
>   };
>   
>   typedef struct PVSCSIRequest {
> @@ -1224,21 +1217,8 @@ pvscsi_post_load(void *opaque, int version_id)
>       return 0;
>   }
>   
> -static bool pvscsi_vmstate_need_pcie_device(void *opaque)
> -{
> -    PVSCSIState *s = PVSCSI(opaque);
> -
> -    return !(s->compat_flags & PVSCSI_COMPAT_DISABLE_PCIE);
> -}
> -
> -static bool pvscsi_vmstate_test_pci_device(void *opaque, int version_id)
> -{
> -    return !pvscsi_vmstate_need_pcie_device(opaque);
> -}
> -
>   static const VMStateDescription vmstate_pvscsi_pcie_device = {
>       .name = "pvscsi/pcie",
> -    .needed = pvscsi_vmstate_need_pcie_device,
>       .fields = (const VMStateField[]) {
>           VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
>           VMSTATE_END_OF_LIST()
> @@ -1252,9 +1232,6 @@ static const VMStateDescription vmstate_pvscsi = {
>       .pre_save = pvscsi_pre_save,
>       .post_load = pvscsi_post_load,
>       .fields = (const VMStateField[]) {
> -        VMSTATE_STRUCT_TEST(parent_obj, PVSCSIState,
> -                            pvscsi_vmstate_test_pci_device, 0,
> -                            vmstate_pci_device, PCIDevice),
>           VMSTATE_UINT8(msi_used, PVSCSIState),
>           VMSTATE_UINT32(resetting, PVSCSIState),
>           VMSTATE_UINT64(reg_interrupt_status, PVSCSIState),
> @@ -1288,19 +1265,14 @@ static const VMStateDescription vmstate_pvscsi = {
>   
>   static const Property pvscsi_properties[] = {
>       DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> -    DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
> -                    PVSCSI_COMPAT_DISABLE_PCIE_BIT, false),
>   };
>   
>   static void pvscsi_realize(DeviceState *qdev, Error **errp)
>   {
>       PVSCSIClass *pvs_c = PVSCSI_GET_CLASS(qdev);
>       PCIDevice *pci_dev = PCI_DEVICE(qdev);
> -    PVSCSIState *s = PVSCSI(qdev);
>   
> -    if (!(s->compat_flags & PVSCSI_COMPAT_DISABLE_PCIE)) {
> -        pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> -    }
> +    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>   
>       pvs_c->parent_dc_realize(qdev, errp);
>   }

[PATCH v4 16/19] hw/scsi/vmw_pvscsi: Convert DeviceRealize -> InstanceInit

[Philippe Mathieu-Daudé]

Simplify replacing pvscsi_realize() by pvscsi_instance_init(),
removing the need for device_class_set_parent_realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/scsi/vmw_pvscsi.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index e163023d14c..7c98b1b8ea6 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1267,21 +1267,15 @@ static const Property pvscsi_properties[] = {
     DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
 };
 
-static void pvscsi_realize(DeviceState *qdev, Error **errp)
+static void pvscsi_instance_init(Object *obj)
 {
-    PVSCSIClass *pvs_c = PVSCSI_GET_CLASS(qdev);
-    PCIDevice *pci_dev = PCI_DEVICE(qdev);
-
-    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
-
-    pvs_c->parent_dc_realize(qdev, errp);
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
 }
 
 static void pvscsi_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-    PVSCSIClass *pvs_k = PVSCSI_CLASS(klass);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->realize = pvscsi_realizefn;
@@ -1290,8 +1284,6 @@ static void pvscsi_class_init(ObjectClass *klass, const void *data)
     k->device_id = PCI_DEVICE_ID_VMWARE_PVSCSI;
     k->class_id = PCI_CLASS_STORAGE_SCSI;
     k->subsystem_id = 0x1000;
-    device_class_set_parent_realize(dc, pvscsi_realize,
-                                    &pvs_k->parent_dc_realize);
     device_class_set_legacy_reset(dc, pvscsi_reset);
     dc->vmsd = &vmstate_pvscsi;
     device_class_set_props(dc, pvscsi_properties);
@@ -1306,6 +1298,7 @@ static const TypeInfo pvscsi_info = {
     .class_size    = sizeof(PVSCSIClass),
     .instance_size = sizeof(PVSCSIState),
     .class_init    = pvscsi_class_init,
+    .instance_init = pvscsi_instance_init,
     .interfaces = (const InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
         { INTERFACE_PCIE_DEVICE },
-- 
2.47.1

Re: [PATCH v4 16/19] hw/scsi/vmw_pvscsi: Convert DeviceRealize -> InstanceInit

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> Simplify replacing pvscsi_realize() by pvscsi_instance_init(),
> removing the need for device_class_set_parent_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/scsi/vmw_pvscsi.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index e163023d14c..7c98b1b8ea6 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -1267,21 +1267,15 @@ static const Property pvscsi_properties[] = {
>       DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
>   };
>   
> -static void pvscsi_realize(DeviceState *qdev, Error **errp)
> +static void pvscsi_instance_init(Object *obj)
>   {
> -    PVSCSIClass *pvs_c = PVSCSI_GET_CLASS(qdev);
> -    PCIDevice *pci_dev = PCI_DEVICE(qdev);
> -
> -    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> -
> -    pvs_c->parent_dc_realize(qdev, errp);
> +    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
>   }
>   
>   static void pvscsi_class_init(ObjectClass *klass, const void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -    PVSCSIClass *pvs_k = PVSCSI_CLASS(klass);
>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>   
>       k->realize = pvscsi_realizefn;
> @@ -1290,8 +1284,6 @@ static void pvscsi_class_init(ObjectClass *klass, const void *data)
>       k->device_id = PCI_DEVICE_ID_VMWARE_PVSCSI;
>       k->class_id = PCI_CLASS_STORAGE_SCSI;
>       k->subsystem_id = 0x1000;
> -    device_class_set_parent_realize(dc, pvscsi_realize,
> -                                    &pvs_k->parent_dc_realize);
>       device_class_set_legacy_reset(dc, pvscsi_reset);
>       dc->vmsd = &vmstate_pvscsi;
>       device_class_set_props(dc, pvscsi_properties);
> @@ -1306,6 +1298,7 @@ static const TypeInfo pvscsi_info = {
>       .class_size    = sizeof(PVSCSIClass),
>       .instance_size = sizeof(PVSCSIState),
>       .class_init    = pvscsi_class_init,
> +    .instance_init = pvscsi_instance_init,
>       .interfaces = (const InterfaceInfo[]) {
>           { TYPE_HOTPLUG_HANDLER },
>           { INTERFACE_PCIE_DEVICE },

[PATCH v4 17/19] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS definition

[Philippe Mathieu-Daudé]

VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS was only used by the
hw_compat_2_5[] array, via the 'x-old-msi-offsets=on' property.
We removed all machines using that array, lets remove all the
code around VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/net/vmxnet3.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 83d942af179..3cf5d71f478 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -42,18 +42,13 @@
 #define VMXNET3_MSIX_BAR_SIZE 0x2000
 
 /* Compatibility flags for migration */
-#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
-#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS \
-    (1 << VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT)
 #define VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT 1
 #define VMXNET3_COMPAT_FLAG_DISABLE_PCIE \
     (1 << VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT)
 
 #define VMXNET3_EXP_EP_OFFSET (0x48)
-#define VMXNET3_MSI_OFFSET(s) \
-    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x50 : 0x84)
-#define VMXNET3_MSIX_OFFSET(s) \
-    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0 : 0x9c)
+#define VMXNET3_MSI_OFFSET    (0x84)
+#define VMXNET3_MSIX_OFFSET   (0x9c)
 #define VMXNET3_DSN_OFFSET     (0x100)
 
 #define VMXNET3_BAR0_IDX      (0)
@@ -61,8 +56,7 @@
 #define VMXNET3_MSIX_BAR_IDX  (2)
 
 #define VMXNET3_OFF_MSIX_TABLE (0x000)
-#define VMXNET3_OFF_MSIX_PBA(s) \
-    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x800 : 0x1000)
+#define VMXNET3_OFF_MSIX_PBA   (0x1000)
 
 /* Link speed in Mbps should be shifted by 16 */
 #define VMXNET3_LINK_SPEED      (1000 << 16)
@@ -2122,8 +2116,8 @@ vmxnet3_init_msix(VMXNET3State *s)
                         &s->msix_bar,
                         VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
                         &s->msix_bar,
-                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
-                        VMXNET3_MSIX_OFFSET(s), NULL);
+                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA,
+                        VMXNET3_MSIX_OFFSET, NULL);
 
     if (0 > res) {
         VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
@@ -2221,7 +2215,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
     /* Interrupt pin A */
     pci_dev->config[PCI_INTERRUPT_PIN] = 0x01;
 
-    ret = msi_init(pci_dev, VMXNET3_MSI_OFFSET(s), VMXNET3_MAX_NMSIX_INTRS,
+    ret = msi_init(pci_dev, VMXNET3_MSI_OFFSET, VMXNET3_MAX_NMSIX_INTRS,
                    VMXNET3_USE_64BIT, VMXNET3_PER_VECTOR_MASK, NULL);
     /* Any error other than -ENOTSUP(board's MSI support is broken)
      * is a programming error. Fall back to INTx silently on -ENOTSUP */
@@ -2472,8 +2466,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
 
 static const Property vmxnet3_properties[] = {
     DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
-    DEFINE_PROP_BIT("x-old-msi-offsets", VMXNET3State, compat_flags,
-                    VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT, false),
     DEFINE_PROP_BIT("x-disable-pcie", VMXNET3State, compat_flags,
                     VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT, false),
 };
-- 
2.47.1

Re: [PATCH v4 17/19] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS definition

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS was only used by the
> hw_compat_2_5[] array, via the 'x-old-msi-offsets=on' property.
> We removed all machines using that array, lets remove all the
> code around VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/net/vmxnet3.c | 20 ++++++--------------
>   1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 83d942af179..3cf5d71f478 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -42,18 +42,13 @@
>   #define VMXNET3_MSIX_BAR_SIZE 0x2000
>   
>   /* Compatibility flags for migration */
> -#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
> -#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS \
> -    (1 << VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT)
>   #define VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT 1
>   #define VMXNET3_COMPAT_FLAG_DISABLE_PCIE \
>       (1 << VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT)
>   
>   #define VMXNET3_EXP_EP_OFFSET (0x48)
> -#define VMXNET3_MSI_OFFSET(s) \
> -    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x50 : 0x84)
> -#define VMXNET3_MSIX_OFFSET(s) \
> -    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0 : 0x9c)
> +#define VMXNET3_MSI_OFFSET    (0x84)
> +#define VMXNET3_MSIX_OFFSET   (0x9c)
>   #define VMXNET3_DSN_OFFSET     (0x100)
>   
>   #define VMXNET3_BAR0_IDX      (0)
> @@ -61,8 +56,7 @@
>   #define VMXNET3_MSIX_BAR_IDX  (2)
>   
>   #define VMXNET3_OFF_MSIX_TABLE (0x000)
> -#define VMXNET3_OFF_MSIX_PBA(s) \
> -    ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x800 : 0x1000)
> +#define VMXNET3_OFF_MSIX_PBA   (0x1000)
>   
>   /* Link speed in Mbps should be shifted by 16 */
>   #define VMXNET3_LINK_SPEED      (1000 << 16)
> @@ -2122,8 +2116,8 @@ vmxnet3_init_msix(VMXNET3State *s)
>                           &s->msix_bar,
>                           VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
>                           &s->msix_bar,
> -                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
> -                        VMXNET3_MSIX_OFFSET(s), NULL);
> +                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA,
> +                        VMXNET3_MSIX_OFFSET, NULL);
>   
>       if (0 > res) {
>           VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
> @@ -2221,7 +2215,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>       /* Interrupt pin A */
>       pci_dev->config[PCI_INTERRUPT_PIN] = 0x01;
>   
> -    ret = msi_init(pci_dev, VMXNET3_MSI_OFFSET(s), VMXNET3_MAX_NMSIX_INTRS,
> +    ret = msi_init(pci_dev, VMXNET3_MSI_OFFSET, VMXNET3_MAX_NMSIX_INTRS,
>                      VMXNET3_USE_64BIT, VMXNET3_PER_VECTOR_MASK, NULL);
>       /* Any error other than -ENOTSUP(board's MSI support is broken)
>        * is a programming error. Fall back to INTx silently on -ENOTSUP */
> @@ -2472,8 +2466,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>   
>   static const Property vmxnet3_properties[] = {
>       DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
> -    DEFINE_PROP_BIT("x-old-msi-offsets", VMXNET3State, compat_flags,
> -                    VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT, false),
>       DEFINE_PROP_BIT("x-disable-pcie", VMXNET3State, compat_flags,
>                       VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT, false),
>   };

[PATCH v4 18/19] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_DISABLE_PCIE definition

[Philippe Mathieu-Daudé]

VMXNET3_COMPAT_FLAG_DISABLE_PCIE was only used by the
hw_compat_2_5[] array, via the 'x-disable-pcie=on' property.
We removed all machines using that array, lets remove all the
code around VMXNET3_COMPAT_FLAG_DISABLE_PCIE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/net/vmxnet3.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 3cf5d71f478..d080fe9b38a 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -41,11 +41,6 @@
 #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
 #define VMXNET3_MSIX_BAR_SIZE 0x2000
 
-/* Compatibility flags for migration */
-#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT 1
-#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE \
-    (1 << VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT)
-
 #define VMXNET3_EXP_EP_OFFSET (0x48)
 #define VMXNET3_MSI_OFFSET    (0x84)
 #define VMXNET3_MSIX_OFFSET   (0x9c)
@@ -2466,8 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
 
 static const Property vmxnet3_properties[] = {
     DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
-    DEFINE_PROP_BIT("x-disable-pcie", VMXNET3State, compat_flags,
-                    VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT, false),
 };
 
 static void vmxnet3_realize(DeviceState *qdev, Error **errp)
@@ -2476,9 +2469,7 @@ static void vmxnet3_realize(DeviceState *qdev, Error **errp)
     PCIDevice *pci_dev = PCI_DEVICE(qdev);
     VMXNET3State *s = VMXNET3(qdev);
 
-    if (!(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE)) {
-        pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
-    }
+    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
 
     vc->parent_dc_realize(qdev, errp);
 }
-- 
2.47.1

Re: [PATCH v4 18/19] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_DISABLE_PCIE definition

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> VMXNET3_COMPAT_FLAG_DISABLE_PCIE was only used by the
> hw_compat_2_5[] array, via the 'x-disable-pcie=on' property.
> We removed all machines using that array, lets remove all the
> code around VMXNET3_COMPAT_FLAG_DISABLE_PCIE.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/net/vmxnet3.c | 11 +----------
>   1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 3cf5d71f478..d080fe9b38a 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -41,11 +41,6 @@
>   #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
>   #define VMXNET3_MSIX_BAR_SIZE 0x2000
>   
> -/* Compatibility flags for migration */
> -#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT 1
> -#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE \
> -    (1 << VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT)
> -
>   #define VMXNET3_EXP_EP_OFFSET (0x48)
>   #define VMXNET3_MSI_OFFSET    (0x84)
>   #define VMXNET3_MSIX_OFFSET   (0x9c)
> @@ -2466,8 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>   
>   static const Property vmxnet3_properties[] = {
>       DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
> -    DEFINE_PROP_BIT("x-disable-pcie", VMXNET3State, compat_flags,
> -                    VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT, false),
>   };
>   
>   static void vmxnet3_realize(DeviceState *qdev, Error **errp)
> @@ -2476,9 +2469,7 @@ static void vmxnet3_realize(DeviceState *qdev, Error **errp)
>       PCIDevice *pci_dev = PCI_DEVICE(qdev);
>       VMXNET3State *s = VMXNET3(qdev);
>   
> -    if (!(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE)) {
> -        pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> -    }
> +    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>   
>       vc->parent_dc_realize(qdev, errp);
>   }

[PATCH v4 19/19] hw/net/vmxnet3: Merge DeviceRealize in InstanceInit

[Philippe Mathieu-Daudé]

Simplify merging vmxnet3_realize() within vmxnet3_instance_init(),
removing the need for device_class_set_parent_realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/net/vmxnet3.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index d080fe9b38a..7c0ca56b7c0 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2238,6 +2238,7 @@ static void vmxnet3_instance_init(Object *obj)
     device_add_bootindex_property(obj, &s->conf.bootindex,
                                   "bootindex", "/ethernet-phy@0",
                                   DEVICE(obj));
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
 }
 
 static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
@@ -2463,22 +2464,10 @@ static const Property vmxnet3_properties[] = {
     DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
 };
 
-static void vmxnet3_realize(DeviceState *qdev, Error **errp)
-{
-    VMXNET3Class *vc = VMXNET3_DEVICE_GET_CLASS(qdev);
-    PCIDevice *pci_dev = PCI_DEVICE(qdev);
-    VMXNET3State *s = VMXNET3(qdev);
-
-    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
-
-    vc->parent_dc_realize(qdev, errp);
-}
-
 static void vmxnet3_class_init(ObjectClass *class, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(class);
     PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
-    VMXNET3Class *vc = VMXNET3_DEVICE_CLASS(class);
 
     c->realize = vmxnet3_pci_realize;
     c->exit = vmxnet3_pci_uninit;
@@ -2489,8 +2478,6 @@ static void vmxnet3_class_init(ObjectClass *class, const void *data)
     c->class_id = PCI_CLASS_NETWORK_ETHERNET;
     c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE;
     c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3;
-    device_class_set_parent_realize(dc, vmxnet3_realize,
-                                    &vc->parent_dc_realize);
     dc->desc = "VMWare Paravirtualized Ethernet v3";
     device_class_set_legacy_reset(dc, vmxnet3_qdev_reset);
     dc->vmsd = &vmstate_vmxnet3;
-- 
2.47.1

Re: [PATCH v4 19/19] hw/net/vmxnet3: Merge DeviceRealize in InstanceInit

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> Simplify merging vmxnet3_realize() within vmxnet3_instance_init(),
> removing the need for device_class_set_parent_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   hw/net/vmxnet3.c | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index d080fe9b38a..7c0ca56b7c0 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2238,6 +2238,7 @@ static void vmxnet3_instance_init(Object *obj)
>       device_add_bootindex_property(obj, &s->conf.bootindex,
>                                     "bootindex", "/ethernet-phy@0",
>                                     DEVICE(obj));
> +    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
>   }
>   
>   static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
> @@ -2463,22 +2464,10 @@ static const Property vmxnet3_properties[] = {
>       DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
>   };
>   
> -static void vmxnet3_realize(DeviceState *qdev, Error **errp)
> -{
> -    VMXNET3Class *vc = VMXNET3_DEVICE_GET_CLASS(qdev);
> -    PCIDevice *pci_dev = PCI_DEVICE(qdev);
> -    VMXNET3State *s = VMXNET3(qdev);
> -
> -    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> -
> -    vc->parent_dc_realize(qdev, errp);
> -}
> -
>   static void vmxnet3_class_init(ObjectClass *class, const void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(class);
>       PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
> -    VMXNET3Class *vc = VMXNET3_DEVICE_CLASS(class);
>   
>       c->realize = vmxnet3_pci_realize;
>       c->exit = vmxnet3_pci_uninit;
> @@ -2489,8 +2478,6 @@ static void vmxnet3_class_init(ObjectClass *class, const void *data)
>       c->class_id = PCI_CLASS_NETWORK_ETHERNET;
>       c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE;
>       c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3;
> -    device_class_set_parent_realize(dc, vmxnet3_realize,
> -                                    &vc->parent_dc_realize);
>       dc->desc = "VMWare Paravirtualized Ethernet v3";
>       device_class_set_legacy_reset(dc, vmxnet3_qdev_reset);
>       dc->vmsd = &vmstate_vmxnet3;

Re: [PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Xiaoyao Li]

On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
> (series reviewed)
> 
> Since v3:
> - Fixed 2 issues noticed by Thomas (floppy fallback, e1000)
> 
> Since v2:
> - Removed qtest in test-x86-cpuid-compat.c
> 
> Since v1:
> - Fixed issues noticed by Thomas
> 
> The versioned 'pc' and 'q35' machines up to 2.12 been marked
> as deprecated two releases ago, and are older than 6 years,
> so according to our support policy we can remove them.
> 
> This series only includes the 2.4 and 2.5 machines removal,
> as it is a big enough number of LoC removed. Rest will
> follow. Highlight is the legacy fw_cfg API removal :)

Overall it looks good to me, except the below 4 patches
>    target/i386/cpu: Remove X86CPU::check_cpuid field

This one gets hard NAK because it changes the default behavior of QEMU.

>    hw/net/e1000: Remove unused E1000_FLAG_MAC flag
>    hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition
>    hw/block/fdc-isa: Remove 'fallback' property

I'm not sure about the three. Because Unlike other properties removed by 
this series, the property name removed by above three patches don't have 
a 'x-' prefix.

It should be OK to remove properties with 'x-' prefix, but I'm not sure 
about the ones without it. There might be user using them explicitly. If 
so, remove them needs to go through standard deprecation process.

Re: [PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Thomas Huth]

On 28/05/2025 05.01, Xiaoyao Li wrote:
> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
>> (series reviewed)
>>
>> Since v3:
>> - Fixed 2 issues noticed by Thomas (floppy fallback, e1000)
>>
>> Since v2:
>> - Removed qtest in test-x86-cpuid-compat.c
>>
>> Since v1:
>> - Fixed issues noticed by Thomas
>>
>> The versioned 'pc' and 'q35' machines up to 2.12 been marked
>> as deprecated two releases ago, and are older than 6 years,
>> so according to our support policy we can remove them.
>>
>> This series only includes the 2.4 and 2.5 machines removal,
>> as it is a big enough number of LoC removed. Rest will
>> follow. Highlight is the legacy fw_cfg API removal :)
> 
> Overall it looks good to me, except the below 4 patches
>>    target/i386/cpu: Remove X86CPU::check_cpuid field
> 
> This one gets hard NAK because it changes the default behavior of QEMU.
> 
>>    hw/net/e1000: Remove unused E1000_FLAG_MAC flag
>>    hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition
>>    hw/block/fdc-isa: Remove 'fallback' property
> 
> I'm not sure about the three. Because Unlike other properties removed by 
> this series, the property name removed by above three patches don't have a 
> 'x-' prefix.
> 
> It should be OK to remove properties with 'x-' prefix, but I'm not sure 
> about the ones without it. There might be user using them explicitly. If so, 
> remove them needs to go through standard deprecation process.

As discussed in another thread, we have a lot of properties that are meant 
for internal use, though they are exposed to the user (and "x-" was 
originally meant for experimental properties, not for internal ones). I 
assume these properties here are such internal ones, so I think it should be 
ok to remove them now without explicit deprecation. We did this in the past 
already for the older pc machine types, too, and so far nobody ever 
complained AFAIK. So I suggest to remove them now, and if someone comlains, 
we can still revert the corresponding patch.

  Thomas

Re: [PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Xiaoyao Li]

On 5/28/2025 4:11 PM, Thomas Huth wrote:
> On 28/05/2025 05.01, Xiaoyao Li wrote:
>> On 5/12/2025 4:39 PM, Philippe Mathieu-Daudé wrote:
>>> (series reviewed)
>>>
>>> Since v3:
>>> - Fixed 2 issues noticed by Thomas (floppy fallback, e1000)
>>>
>>> Since v2:
>>> - Removed qtest in test-x86-cpuid-compat.c
>>>
>>> Since v1:
>>> - Fixed issues noticed by Thomas
>>>
>>> The versioned 'pc' and 'q35' machines up to 2.12 been marked
>>> as deprecated two releases ago, and are older than 6 years,
>>> so according to our support policy we can remove them.
>>>
>>> This series only includes the 2.4 and 2.5 machines removal,
>>> as it is a big enough number of LoC removed. Rest will
>>> follow. Highlight is the legacy fw_cfg API removal :)
>>
>> Overall it looks good to me, except the below 4 patches
>>>    target/i386/cpu: Remove X86CPU::check_cpuid field
>>
>> This one gets hard NAK because it changes the default behavior of QEMU.
>>
>>>    hw/net/e1000: Remove unused E1000_FLAG_MAC flag
>>>    hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition
>>>    hw/block/fdc-isa: Remove 'fallback' property
>>
>> I'm not sure about the three. Because Unlike other properties removed 
>> by this series, the property name removed by above three patches don't 
>> have a 'x-' prefix.
>>
>> It should be OK to remove properties with 'x-' prefix, but I'm not 
>> sure about the ones without it. There might be user using them 
>> explicitly. If so, remove them needs to go through standard 
>> deprecation process.
> 
> As discussed in another thread, we have a lot of properties that are 
> meant for internal use, though they are exposed to the user (and "x-" 
> was originally meant for experimental properties, not for internal 
> ones). I assume these properties here are such internal ones, so I think 
> it should be ok to remove them now without explicit deprecation. We did 
> this in the past already for the older pc machine types, too, and so far 
> nobody ever complained AFAIK. So I suggest to remove them now, and if 
> someone comlains, we can still revert the corresponding patch.

I'm fine with it since nobody explicitly rejects the removal as I did 
for "target/i386/cpu: Remove X86CPU::check_cpuid field"

>   Thomas
> 

Re: [PATCH v4 00/19] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Michael S. Tsirkin]

On Mon, May 12, 2025 at 10:39:29AM +0200, Philippe Mathieu-Daudé wrote:
> (series reviewed)

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


Thomas, since you wanted to do it, pls feel free to merge.


> Since v3:
> - Fixed 2 issues noticed by Thomas (floppy fallback, e1000)
> 
> Since v2:
> - Removed qtest in test-x86-cpuid-compat.c
> 
> Since v1:
> - Fixed issues noticed by Thomas
> 
> The versioned 'pc' and 'q35' machines up to 2.12 been marked
> as deprecated two releases ago, and are older than 6 years,
> so according to our support policy we can remove them.
> 
> This series only includes the 2.4 and 2.5 machines removal,
> as it is a big enough number of LoC removed. Rest will
> follow. Highlight is the legacy fw_cfg API removal :)
> 
> Philippe Mathieu-Daudé (19):
>   hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines
>   hw/i386/pc: Remove PCMachineClass::broken_reserved_end field
>   hw/i386/pc: Remove pc_compat_2_4[] array
>   target/i386/cpu: Remove X86CPU::check_cpuid field
>   hw/core/machine: Remove hw_compat_2_4[] array
>   hw/net/e1000: Remove unused E1000_FLAG_MAC flag
>   hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition
>   hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition
>   hw/i386/pc: Remove deprecated pc-q35-2.5 and pc-i440fx-2.5 machines
>   hw/i386/x86: Remove X86MachineClass::save_tsc_khz field
>   hw/nvram/fw_cfg: Remove legacy FW_CFG_ORDER_OVERRIDE
>   hw/core/machine: Remove hw_compat_2_5[] array
>   hw/block/fdc-isa: Remove 'fallback' property
>   hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION
>     definition
>   hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition
>   hw/scsi/vmw_pvscsi: Convert DeviceRealize -> InstanceInit
>   hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS definition
>   hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_DISABLE_PCIE definition
>   hw/net/vmxnet3: Merge DeviceRealize in InstanceInit
> 
>  include/hw/boards.h                 |   9 +--
>  include/hw/i386/pc.h                |   7 --
>  include/hw/i386/x86.h               |   5 --
>  include/hw/loader.h                 |   2 -
>  include/hw/nvram/fw_cfg.h           |  10 ---
>  include/hw/virtio/virtio-pci.h      |   8 --
>  target/i386/cpu.h                   |   1 -
>  hw/block/fdc-isa.c                  |   4 +-
>  hw/core/loader.c                    |  14 ----
>  hw/core/machine.c                   |  18 -----
>  hw/i386/pc.c                        |  42 ++---------
>  hw/i386/pc_piix.c                   |  26 -------
>  hw/i386/pc_q35.c                    |  26 -------
>  hw/i386/x86.c                       |   1 -
>  hw/net/e1000.c                      |  93 ++++++++++--------------
>  hw/net/vmxnet3.c                    |  44 ++---------
>  hw/nvram/fw_cfg.c                   | 109 ++--------------------------
>  hw/scsi/vmw_pvscsi.c                |  67 +++--------------
>  hw/virtio/virtio-pci.c              |  11 +--
>  system/vl.c                         |   5 --
>  target/i386/cpu.c                   |   3 +-
>  target/i386/machine.c               |   5 +-
>  tests/qtest/test-x86-cpuid-compat.c |  14 ----
>  23 files changed, 77 insertions(+), 447 deletions(-)
> 
> -- 
> 2.47.1