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

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

[Philippe Mathieu-Daudé]

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é (21):
  hw/i386/pc: Remove unused pc_compat_2_3 declarations
  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
  target/i386/cpu: Pass Error** to x86_cpu_filter_features()
  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

 docs/about/removed-features.rst |   4 ++
 include/hw/boards.h             |   9 +--
 include/hw/i386/pc.h            |  10 ---
 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                  |  63 ++++--------------
 hw/net/vmxnet3.c                |  44 ++-----------
 hw/nvram/fw_cfg.c               | 109 ++------------------------------
 hw/scsi/vmw_pvscsi.c            |  67 ++++----------------
 hw/virtio/virtio-pci.c          |  15 +----
 system/vl.c                     |   5 --
 target/i386/cpu.c               |  27 +++-----
 target/i386/machine.c           |   5 +-
 23 files changed, 61 insertions(+), 454 deletions(-)

-- 
2.47.1

[PATCH 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Philippe Mathieu-Daudé]

We removed the implementations in commit 46a2bd52571
("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
but forgot to remove the declarations. Do it now.

Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a558705cb9a..103b54301f8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -302,9 +302,6 @@ extern const size_t pc_compat_2_5_len;
 extern GlobalProperty pc_compat_2_4[];
 extern const size_t pc_compat_2_4_len;
 
-extern GlobalProperty pc_compat_2_3[];
-extern const size_t pc_compat_2_3_len;
-
 #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
     static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
     { \
-- 
2.47.1

Re: [PATCH 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:27AM +0100, Philippe Mathieu-Daudé wrote:
> We removed the implementations in commit 46a2bd52571
> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> but forgot to remove the declarations. Do it now.
> 
> Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/i386/pc.h | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Richard Henderson]

On 1/15/25 15:22, Philippe Mathieu-Daudé wrote:
> We removed the implementations in commit 46a2bd52571
> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> but forgot to remove the declarations. Do it now.
> 
> Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/i386/pc.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index a558705cb9a..103b54301f8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -302,9 +302,6 @@ extern const size_t pc_compat_2_5_len;
>   extern GlobalProperty pc_compat_2_4[];
>   extern const size_t pc_compat_2_4_len;
>   
> -extern GlobalProperty pc_compat_2_3[];
> -extern const size_t pc_compat_2_3_len;
> -
>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
>       { \

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Thomas Huth]

On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
> We removed the implementations in commit 46a2bd52571
> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> but forgot to remove the declarations. Do it now.
> 
> Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/i386/pc.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index a558705cb9a..103b54301f8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -302,9 +302,6 @@ extern const size_t pc_compat_2_5_len;
>   extern GlobalProperty pc_compat_2_4[];
>   extern const size_t pc_compat_2_4_len;
>   
> -extern GlobalProperty pc_compat_2_3[];
> -extern const size_t pc_compat_2_3_len;
> -
>   #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
>       static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
>       { \

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Michael Tokarev]

17.01.2025 11:52, Thomas Huth wrote:
> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>> We removed the implementations in commit 46a2bd52571
>> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
>> but forgot to remove the declarations. Do it now.
>>
>> Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thomas, such pick-ups from larger series may not be a good idea
for qemu-trivial in general.  But this one looks okay'ish since
the series is about 2.4 and 2.5 machines :)

Philippe, are you okay with merging this one through qemu-trivial?

Thanks,

/mjt

Re: [PATCH 01/21] hw/i386/pc: Remove unused pc_compat_2_3 declarations

[Philippe Mathieu-Daudé]

On 30/1/25 10:59, Michael Tokarev wrote:
> 17.01.2025 11:52, Thomas Huth wrote:
>> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>>> We removed the implementations in commit 46a2bd52571
>>> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine")
>>> but forgot to remove the declarations. Do it now.
>>>
>>> Fixes: 46a2bd52571 ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 
>>> machine")
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Thomas, such pick-ups from larger series may not be a good idea
> for qemu-trivial in general.  But this one looks okay'ish since
> the series is about 2.4 and 2.5 machines :)
> 
> Philippe, are you okay with merging this one through qemu-trivial?

Sure! Even appreciated :)

[PATCH 02/21] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/about/removed-features.rst |  4 ++++
 hw/i386/pc_piix.c               | 13 -------------
 hw/i386/pc_q35.c                | 13 -------------
 3 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index c6616ce05e5..e4bdb385a25 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1012,6 +1012,10 @@ This machine has been renamed ``fuloong2e``.
 These machine types were very old and likely could not be used for live
 migration from old QEMU versions anymore. Use a newer machine type instead.
 
+``pc-q35-2.4`` and `pc-i440fx-2.4` (removed in 10.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''''
+This versioned machine has been supported for a period of more than 6 years.
+
 Raspberry Pi ``raspi2`` and ``raspi3`` machines (removed in 6.2)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 04d2957adcd..64c1ab5ff3f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -780,19 +780,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 77536dd697f..847f56263f3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -674,16 +674,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);
-- 
2.47.1

Re: [PATCH 02/21] hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:28AM +0100, 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.

Yes, but ...see commit c9fd2d9a48ee3c195cf83cc611b87b09f02f0013

[quote]
    The new deprecation and deletion policy for versioned machine types is
    being introduced in QEMU 9.1.0.
    
    Under the new policy a number of old machine types (any prior to 2.12)
    would be liable for immediate deletion which would be a violation of our
    historical deprecation and removal policy
    
    Thus automatic deletions (by skipping QOM registration) are temporarily
    gated on existance of the env variable "QEMU_DELETE_MACHINES" / QEMU
    version number >= 10.1.0. This allows opt-in testing of the automatic
    deletion logic, while activating it fully in QEMU >= 10.1.0.
[/quote]

IOW, we only intended to apply the "delete after 6 years" special
rule for machine types from 10.1.0 onwards.

Until then we should consider machine types to be following the
general process, which was deprecate for 2 cycles, then delete
in the 3rd cycle.

You added these deprecations in:

  commit 792b4fdd4eb8197bd6eb9e80a1dfaf0cb3b54aeb
  Author: Philippe Mathieu-Daudé <philmd@linaro.org>
  Date:   Wed Feb 28 10:34:35 2024 +0100

    hw/i386/pc: Deprecate 2.4 to 2.12 pc-i440fx machines
    
    Similarly to the commit c7437f0ddb "docs/about: Mark the
    old pc-i440fx-2.0 - 2.3 machine types as deprecated",
    deprecate the 2.4 to 2.12 machines.

which IIUC was the 9.1.0 cycle too.

So the new 3 year / 6 year policy applies, but the 6 year
deletion policy was declared to NOT start until the 10.1.0
release cycle.

So we are 1 release too early to start applying that
deletion rule.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  docs/about/removed-features.rst |  4 ++++
>  hw/i386/pc_piix.c               | 13 -------------
>  hw/i386/pc_q35.c                | 13 -------------
>  3 files changed, 4 insertions(+), 26 deletions(-)
> 
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index c6616ce05e5..e4bdb385a25 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -1012,6 +1012,10 @@ This machine has been renamed ``fuloong2e``.
>  These machine types were very old and likely could not be used for live
>  migration from old QEMU versions anymore. Use a newer machine type instead.
>  
> +``pc-q35-2.4`` and `pc-i440fx-2.4` (removed in 10.0)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''
> +This versioned machine has been supported for a period of more than 6 years.
> +
>  Raspberry Pi ``raspi2`` and ``raspi3`` machines (removed in 6.2)
>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>  
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 04d2957adcd..64c1ab5ff3f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -780,19 +780,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 77536dd697f..847f56263f3 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -674,16 +674,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);
> -- 
> 2.47.1
> 
> 

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 02/21] hw/i386/pc: Remove deprecated pc-q35-2.4 and pc-i440fx-2.4 machines

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:28AM +0100, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  docs/about/removed-features.rst |  4 ++++
>  hw/i386/pc_piix.c               | 13 -------------
>  hw/i386/pc_q35.c                | 13 -------------
>  3 files changed, 4 insertions(+), 26 deletions(-)

I'll give this:

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

on the basis that this patch series isn't added to a PULL until
the 10.1.0 dev cycle opens.

> 
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index c6616ce05e5..e4bdb385a25 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -1012,6 +1012,10 @@ This machine has been renamed ``fuloong2e``.
>  These machine types were very old and likely could not be used for live
>  migration from old QEMU versions anymore. Use a newer machine type instead.
>  
> +``pc-q35-2.4`` and `pc-i440fx-2.4` (removed in 10.0)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''
> +This versioned machine has been supported for a period of more than 6 years.
> +
>  Raspberry Pi ``raspi2`` and ``raspi3`` machines (removed in 6.2)
>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>

We might be able to drop chunk this depending on discussion around
Thomas' general docs patch


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 :|

[PATCH 03/21] 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>
---
 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 103b54301f8..7fc34bb4a34 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 9334b033f65..bf5308160e4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -994,14 +994,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));
@@ -1039,9 +1038,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 03/21] hw/i386/pc: Remove PCMachineClass::broken_reserved_end field

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:29AM +0100, 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>
> ---
>  include/hw/i386/pc.h |  1 -
>  hw/i386/pc.c         | 13 +++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 03/21] hw/i386/pc: Remove PCMachineClass::broken_reserved_end field

[Thomas Huth]

On 16/01/2025 00.22, 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>
> ---
>   include/hw/i386/pc.h |  1 -
>   hw/i386/pc.c         | 13 +++++--------
>   2 files changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 04/21] 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>
---
 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 7fc34bb4a34..31e799061ef 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -298,9 +298,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, void *data) \
     { \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bf5308160e4..6d0c9014d63 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -257,25 +257,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 04/21] hw/i386/pc: Remove pc_compat_2_4[] array

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:30AM +0100, 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>
> ---
>  include/hw/i386/pc.h |  3 ---
>  hw/i386/pc.c         | 19 -------------------
>  2 files changed, 22 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 05/21] 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>
---
 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 dbd8f1ffc79..87917b7f895 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2091,7 +2091,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 0b639848cd6..42227643126 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7822,7 +7822,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() ?
@@ -8491,7 +8491,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 05/21] target/i386/cpu: Remove X86CPU::check_cpuid field

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:31AM +0100, 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().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/cpu.h | 1 -
>  target/i386/cpu.c | 3 +--
>  2 files changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 06/21] target/i386/cpu: Pass Error** to x86_cpu_filter_features()

[Philippe Mathieu-Daudé]

Simplify x86_cpu_realizefn() by passing an Error**
argument to x86_cpu_filter_features().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/cpu.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 42227643126..c48241fb902 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5896,7 +5896,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
     }
 }
 
-static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose);
+static bool x86_cpu_filter_features(X86CPU *cpu, Error **errp);
 
 /* Build a list with the name of all features on a feature word array */
 static void x86_cpu_list_feature_names(FeatureWordArray features,
@@ -6084,7 +6084,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
         error_free(err);
     }
 
-    x86_cpu_filter_features(xc, false);
+    x86_cpu_filter_features(xc, NULL);
 
     x86_cpu_list_feature_names(xc->filtered_features, tail);
 
@@ -7650,7 +7650,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
  *
  * Returns: true if any flag is not supported by the host, false otherwise.
  */
-static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
+static bool x86_cpu_filter_features(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
     FeatureWord w;
@@ -7660,7 +7660,7 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
     uint32_t eax_0, ebx_0, ecx_0, edx_0;
     uint32_t eax_1, ebx_1, ecx_1, edx_1;
 
-    if (verbose) {
+    if (errp) {
         prefix = accel_uses_host_cpuid()
                  ? "host doesn't support requested feature"
                  : "TCG doesn't support requested feature";
@@ -7712,15 +7712,13 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
         uint8_t version = ebx_0 & 0xff;
 
         if (version < env->avx10_version) {
-            if (prefix) {
-                warn_report("%s: avx10.%d. Adjust to avx10.%d",
-                            prefix, env->avx10_version, version);
-            }
+            error_setg(errp, "%s: avx10.%d. Adjust to avx10.%d",
+                       prefix, env->avx10_version, version);
             env->avx10_version = version;
             have_filtered_features = true;
         }
     } else if (env->avx10_version && prefix) {
-        warn_report("%s: avx10.%d.", prefix, env->avx10_version);
+        error_setg(errp, "%s: avx10.%d.", prefix, env->avx10_version);
         have_filtered_features = true;
     }
 
@@ -7822,14 +7820,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
-    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid)) {
-        if (cpu->enforce_cpuid) {
-            error_setg(&local_err,
-                       accel_uses_host_cpuid() ?
-                       "Host doesn't support requested features" :
-                       "TCG doesn't support requested features");
-            goto out;
-        }
+    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid ? &local_err : NULL)) {
+        goto out;
     }
 
     /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
-- 
2.47.1

Re: [PATCH 06/21] target/i386/cpu: Pass Error** to x86_cpu_filter_features()

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:32AM +0100, Philippe Mathieu-Daudé wrote:
> Simplify x86_cpu_realizefn() by passing an Error**
> argument to x86_cpu_filter_features().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/cpu.c | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 42227643126..c48241fb902 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5896,7 +5896,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
>      }
>  }
>  
> -static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose);
> +static bool x86_cpu_filter_features(X86CPU *cpu, Error **errp);
>  
>  /* Build a list with the name of all features on a feature word array */
>  static void x86_cpu_list_feature_names(FeatureWordArray features,
> @@ -6084,7 +6084,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>          error_free(err);
>      }
>  
> -    x86_cpu_filter_features(xc, false);
> +    x86_cpu_filter_features(xc, NULL);
>  
>      x86_cpu_list_feature_names(xc->filtered_features, tail);
>  
> @@ -7650,7 +7650,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>   *
>   * Returns: true if any flag is not supported by the host, false otherwise.
>   */
> -static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
> +static bool x86_cpu_filter_features(X86CPU *cpu, Error **errp)
>  {
>      CPUX86State *env = &cpu->env;
>      FeatureWord w;
> @@ -7660,7 +7660,7 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>      uint32_t eax_0, ebx_0, ecx_0, edx_0;
>      uint32_t eax_1, ebx_1, ecx_1, edx_1;
>  
> -    if (verbose) {
> +    if (errp) {
>          prefix = accel_uses_host_cpuid()
>                   ? "host doesn't support requested feature"
>                   : "TCG doesn't support requested feature";
> @@ -7712,15 +7712,13 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>          uint8_t version = ebx_0 & 0xff;
>  
>          if (version < env->avx10_version) {
> -            if (prefix) {
> -                warn_report("%s: avx10.%d. Adjust to avx10.%d",
> -                            prefix, env->avx10_version, version);
> -            }
> +            error_setg(errp, "%s: avx10.%d. Adjust to avx10.%d",
> +                       prefix, env->avx10_version, version);
>              env->avx10_version = version;
>              have_filtered_features = true;

This doesn't look right.  Previously it was correct to carry on and
set  'env->avx10_version = version' or 'have_filtered_features',
because it was upto the caller whether this was an error scenario
or just a warning.

With your change though, we're unambiguously treating this as an
error condition. So we should return from this method immediately
after calling 'error_setg' now.

>          }
>      } else if (env->avx10_version && prefix) {
> -        warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> +        error_setg(errp, "%s: avx10.%d.", prefix, env->avx10_version);
>          have_filtered_features = true;

Same here, needs a 'return'

>      }
>  
> @@ -7822,14 +7820,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>      }
>  
> -    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid)) {
> -        if (cpu->enforce_cpuid) {
> -            error_setg(&local_err,
> -                       accel_uses_host_cpuid() ?
> -                       "Host doesn't support requested features" :
> -                       "TCG doesn't support requested features");
> -            goto out;
> -        }
> +    if (x86_cpu_filter_features(cpu, cpu->enforce_cpuid ? &local_err : NULL)) {
> +        goto out;
>      }
>  
>      /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> -- 
> 2.47.1
> 
> 

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 :|

[PATCH 07/21] 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>
---
 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 2ad711e56db..e9e33c57f27 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -841,7 +841,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 c23b3994964..bdd4dee3d67 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -281,15 +281,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 07/21] hw/core/machine: Remove hw_compat_2_4[] array

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:33AM +0100, 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>
> ---
>  include/hw/boards.h | 3 ---
>  hw/core/machine.c   | 9 ---------
>  2 files changed, 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 08/21] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/e1000.c | 63 +++++++++-----------------------------------------
 1 file changed, 11 insertions(+), 52 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 3d0b2277039..14d2133cd80 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)
 
@@ -1218,46 +1216,17 @@ enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 };
  * n - flag needed
  * p - partially implenented */
 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),
-
-    [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_PARTIAL,
+    [TDFT]  = MAC_ACCESS_PARTIAL,
+    [TDFHS] = MAC_ACCESS_PARTIAL,
+    [TDFTS] = MAC_ACCESS_PARTIAL,
+    [TDFPC] = MAC_ACCESS_PARTIAL,
+    [RDFH]  = MAC_ACCESS_PARTIAL,
+    [RDFT]  = MAC_ACCESS_PARTIAL,
+    [RDFHS] = MAC_ACCESS_PARTIAL,
+    [RDFTS] = MAC_ACCESS_PARTIAL,
+    [RDFPC] = MAC_ACCESS_PARTIAL,
+    [PBM]   = MAC_ACCESS_PARTIAL,
 };
 
 static void
@@ -1419,13 +1388,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 +1413,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 +1640,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

Re: [PATCH 08/21] hw/net/e1000: Remove unused E1000_FLAG_MAC flag

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:34AM +0100, Philippe Mathieu-Daudé wrote:
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/e1000.c | 63 +++++++++-----------------------------------------
>  1 file changed, 11 insertions(+), 52 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 3d0b2277039..14d2133cd80 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)
>  
> @@ -1218,46 +1216,17 @@ enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 };
>   * n - flag needed
>   * p - partially implenented */
>  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),
> -
> -    [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_PARTIAL,
> +    [TDFT]  = MAC_ACCESS_PARTIAL,
> +    [TDFHS] = MAC_ACCESS_PARTIAL,
> +    [TDFTS] = MAC_ACCESS_PARTIAL,
> +    [TDFPC] = MAC_ACCESS_PARTIAL,
> +    [RDFH]  = MAC_ACCESS_PARTIAL,
> +    [RDFT]  = MAC_ACCESS_PARTIAL,
> +    [RDFHS] = MAC_ACCESS_PARTIAL,
> +    [RDFTS] = MAC_ACCESS_PARTIAL,
> +    [RDFPC] = MAC_ACCESS_PARTIAL,
> +    [PBM]   = MAC_ACCESS_PARTIAL,

It seems like this is removing all use of the 'markflag' macro ?

If I'm correct, then that macro can be removed. That in turnm means
the 'MAC_ACCESS_FLAG_NEEDED' enum entry can also be removed, as well
as places in code that check this flag.

>  };
>  
>  static void
> @@ -1419,13 +1388,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 +1413,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 +1640,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
> 
> 

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 08/21] hw/net/e1000: Remove unused E1000_FLAG_MAC flag

[Thomas Huth]

On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/e1000.c | 63 +++++++++-----------------------------------------
>   1 file changed, 11 insertions(+), 52 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 3d0b2277039..14d2133cd80 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)
>   
> @@ -1218,46 +1216,17 @@ enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 };
>    * n - flag needed
>    * p - partially implenented */
>   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),

I think this is wrong. All those registers should still be marked with 
MAC_ACCESS_FLAG_NEEDED, shouldn't they?

  Thomas


> -    [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_PARTIAL,
> +    [TDFT]  = MAC_ACCESS_PARTIAL,
> +    [TDFHS] = MAC_ACCESS_PARTIAL,
> +    [TDFTS] = MAC_ACCESS_PARTIAL,
> +    [TDFPC] = MAC_ACCESS_PARTIAL,
> +    [RDFH]  = MAC_ACCESS_PARTIAL,
> +    [RDFT]  = MAC_ACCESS_PARTIAL,
> +    [RDFHS] = MAC_ACCESS_PARTIAL,
> +    [RDFTS] = MAC_ACCESS_PARTIAL,
> +    [RDFPC] = MAC_ACCESS_PARTIAL,
> +    [PBM]   = MAC_ACCESS_PARTIAL,
>   };
>   
>   static void
> @@ -1419,13 +1388,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 +1413,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 +1640,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,

Re: [PATCH 08/21] hw/net/e1000: Remove unused E1000_FLAG_MAC flag

[Philippe Mathieu-Daudé]

On 17/1/25 09:58, Thomas Huth wrote:
> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>> 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/net/e1000.c | 63 +++++++++-----------------------------------------
>>   1 file changed, 11 insertions(+), 52 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 3d0b2277039..14d2133cd80 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)
>> @@ -1218,46 +1216,17 @@ enum { MAC_ACCESS_PARTIAL = 1, 
>> MAC_ACCESS_FLAG_NEEDED = 2 };
>>    * n - flag needed
>>    * p - partially implenented */
>>   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),
> 
> I think this is wrong. All those registers should still be marked with 
> MAC_ACCESS_FLAG_NEEDED, shouldn't they?

I followed Paolo's removal in commit fa4ec9ffda7
("e1000: remove old compatibility code"):

-    [RDTR]    = markflag(MIT),    [TADV]    = markflag(MIT),
-    [RADV]    = markflag(MIT),    [ITR]     = markflag(MIT),

Is it the same problem?

> 
>   Thomas

Re: [PATCH 08/21] hw/net/e1000: Remove unused E1000_FLAG_MAC flag

[Thomas Huth]

On 17/01/2025 17.41, Philippe Mathieu-Daudé wrote:
> On 17/1/25 09:58, Thomas Huth wrote:
>> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>>> 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.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/net/e1000.c | 63 +++++++++-----------------------------------------
>>>   1 file changed, 11 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>>> index 3d0b2277039..14d2133cd80 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)
>>> @@ -1218,46 +1216,17 @@ enum { MAC_ACCESS_PARTIAL = 1, 
>>> MAC_ACCESS_FLAG_NEEDED = 2 };
>>>    * n - flag needed
>>>    * p - partially implenented */
>>>   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),
>>
>> I think this is wrong. All those registers should still be marked with 
>> MAC_ACCESS_FLAG_NEEDED, shouldn't they?
> 
> I followed Paolo's removal in commit fa4ec9ffda7
> ("e1000: remove old compatibility code"):
> 
> -    [RDTR]    = markflag(MIT),    [TADV]    = markflag(MIT),
> -    [RADV]    = markflag(MIT),    [ITR]     = markflag(MIT),
> 
> Is it the same problem?

Oops, sorry, I think I got confused by that MAC_ACCESS_FLAG_NEEDED logic in 
e1000_mmio_read() and e1000_mmio_write() ... after reading it more 
carefully, I think it is ok what you did here.

But maybe you could even remove the markflag macro and 
MAC_ACCESS_FLAG_NEEDED now completely?

  Thomas

[PATCH 09/21] 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>
---
 include/hw/virtio/virtio-pci.h |  4 ----
 hw/virtio/virtio-pci.c         | 10 ----------
 2 files changed, 14 deletions(-)

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 971c5fabd44..dd6eb9a4fc7 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 c773a9130c7..8dca3ae73e6 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -144,13 +144,6 @@ 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;
-}
-
 static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
@@ -2352,8 +2345,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,
@@ -2601,7 +2592,6 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->load_queue = virtio_pci_load_queue;
     k->save_extra_state = virtio_pci_save_extra_state;
     k->load_extra_state = virtio_pci_load_extra_state;
-    k->has_extra_state = virtio_pci_has_extra_state;
     k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
     k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
-- 
2.47.1

Re: [PATCH 09/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:35AM +0100, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>  include/hw/virtio/virtio-pci.h |  4 ----
>  hw/virtio/virtio-pci.c         | 10 ----------
>  2 files changed, 14 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 09/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition

[Thomas Huth]

On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>   include/hw/virtio/virtio-pci.h |  4 ----
>   hw/virtio/virtio-pci.c         | 10 ----------
>   2 files changed, 14 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 971c5fabd44..dd6eb9a4fc7 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 c773a9130c7..8dca3ae73e6 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -144,13 +144,6 @@ 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;
> -}
> -
>   static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
>   {
>       VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> @@ -2352,8 +2345,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,
> @@ -2601,7 +2592,6 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>       k->load_queue = virtio_pci_load_queue;
>       k->save_extra_state = virtio_pci_save_extra_state;
>       k->load_extra_state = virtio_pci_load_extra_state;
> -    k->has_extra_state = virtio_pci_has_extra_state;

Did you test your patches? Looking at the calling site:

static bool virtio_extra_state_needed(void *opaque)
{
     VirtIODevice *vdev = opaque;
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);

     return k->has_extra_state &&
         k->has_extra_state(qbus->parent);
}

... this will return now false instead of true, so I think your patch here 
is wrong?

  Thomas

Re: [PATCH 09/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition

[Philippe Mathieu-Daudé]

On 17/1/25 10:06, Thomas Huth wrote:
> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>> 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>
>> ---
>>   include/hw/virtio/virtio-pci.h |  4 ----
>>   hw/virtio/virtio-pci.c         | 10 ----------
>>   2 files changed, 14 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/ 
>> virtio-pci.h
>> index 971c5fabd44..dd6eb9a4fc7 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 c773a9130c7..8dca3ae73e6 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -144,13 +144,6 @@ 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;
>> -}
>> -
>>   static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
>>   {
>>       VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>> @@ -2352,8 +2345,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,
>> @@ -2601,7 +2592,6 @@ static void 
>> virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>>       k->load_queue = virtio_pci_load_queue;
>>       k->save_extra_state = virtio_pci_save_extra_state;
>>       k->load_extra_state = virtio_pci_load_extra_state;
>> -    k->has_extra_state = virtio_pci_has_extra_state;
> 
> Did you test your patches? Looking at the calling site:
> 
> static bool virtio_extra_state_needed(void *opaque)
> {
>      VirtIODevice *vdev = opaque;
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> 
>      return k->has_extra_state &&
>          k->has_extra_state(qbus->parent);
> }
> 
> ... this will return now false instead of true, so I think your patch 
> here is wrong?

Oops, good catch...

[PATCH 10/21] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 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 dd6eb9a4fc7..1ca7419cd43 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 8dca3ae73e6..2f2c6b13863 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2347,8 +2347,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,
@@ -2377,8 +2375,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 10/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:36AM +0100, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/virtio/virtio-pci.h | 4 ----
>  hw/virtio/virtio-pci.c         | 5 +----
>  2 files changed, 1 insertion(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 10/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Thomas Huth]

On 16/01/2025 00.22, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   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 dd6eb9a4fc7..1ca7419cd43 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,

I assume it's ok that the other following bits change their value here?

If so:
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 10/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Philippe Mathieu-Daudé]

On 17/1/25 10:08, Thomas Huth wrote:
> On 16/01/2025 00.22, 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   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 dd6eb9a4fc7..1ca7419cd43 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,
> 
> I assume it's ok that the other following bits change their value here?

I followed previous commit 9a4c0e220d8 ("hw/virtio-pci:
fix virtio behaviour"):

diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index e4548c2f970..25fbf8a375d 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
  enum {
      VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
      VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
-    VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT,
-    VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT,
      VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT,
      VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,

> 
> If so:
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!

Re: [PATCH 10/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_DISABLE_PCIE definition

[Thomas Huth]

On 17/01/2025 17.43, Philippe Mathieu-Daudé wrote:
> On 17/1/25 10:08, Thomas Huth wrote:
>> On 16/01/2025 00.22, 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.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   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 dd6eb9a4fc7..1ca7419cd43 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,
>>
>> I assume it's ok that the other following bits change their value here?
> 
> I followed previous commit 9a4c0e220d8 ("hw/virtio-pci:
> fix virtio behaviour"):
> 
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index e4548c2f970..25fbf8a375d 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
>   enum {
>       VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
>       VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
> -    VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT,
> -    VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT,
>       VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT,
>       VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
>       VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,

Ok, if it has been done before and nobody complained, the exact numbering 
likely doesn't matter here :-)

  Thomas

[PATCH 11/21] 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>
---
 docs/about/removed-features.rst |  4 ++--
 include/hw/i386/pc.h            |  3 ---
 hw/i386/pc.c                    |  3 ---
 hw/i386/pc_piix.c               | 13 -------------
 hw/i386/pc_q35.c                | 13 -------------
 5 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index e4bdb385a25..7b0eaa6ba90 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1012,8 +1012,8 @@ This machine has been renamed ``fuloong2e``.
 These machine types were very old and likely could not be used for live
 migration from old QEMU versions anymore. Use a newer machine type instead.
 
-``pc-q35-2.4`` and `pc-i440fx-2.4` (removed in 10.0)
-''''''''''''''''''''''''''''''''''''''''''''''''''''
+``pc-q35-2.4``, ``pc-q35-2.5``, ``pc-i440fx-2.4`` and ``pc-i440fx-2.5`` (removed in 10.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 This versioned machine has been supported for a period of more than 6 years.
 
 Raspberry Pi ``raspi2`` and ``raspi3`` machines (removed in 6.2)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 31e799061ef..02c0cb8bd41 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -295,9 +295,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, void *data) \
     { \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6d0c9014d63..137f827b69e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -254,9 +254,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 64c1ab5ff3f..ae573a39836 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -767,19 +767,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 847f56263f3..ff473b67985 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -661,16 +661,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 11/21] hw/i386/pc: Remove deprecated pc-q35-2.5 and pc-i440fx-2.5 machines

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:37AM +0100, 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>
> ---
>  docs/about/removed-features.rst |  4 ++--
>  include/hw/i386/pc.h            |  3 ---
>  hw/i386/pc.c                    |  3 ---
>  hw/i386/pc_piix.c               | 13 -------------
>  hw/i386/pc_q35.c                | 13 -------------
>  5 files changed, 2 insertions(+), 34 deletions(-)

On the assumption this series isn't sent in a PULL until 10.1.0
dev cycle

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index e4bdb385a25..7b0eaa6ba90 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -1012,8 +1012,8 @@ This machine has been renamed ``fuloong2e``.
>  These machine types were very old and likely could not be used for live
>  migration from old QEMU versions anymore. Use a newer machine type instead.
>  
> -``pc-q35-2.4`` and `pc-i440fx-2.4` (removed in 10.0)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''
> +``pc-q35-2.4``, ``pc-q35-2.5``, ``pc-i440fx-2.4`` and ``pc-i440fx-2.5`` (removed in 10.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>  This versioned machine has been supported for a period of more than 6 years.
>  
>  Raspberry Pi ``raspi2`` and ``raspi3`` machines (removed in 6.2)

Again possibly able to be dropped vs Thomas' docs patch

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 :|

[PATCH 12/21] 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>
---
 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 d43cb3908e6..29ffa1f450e 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 69bfc00b9a5..57014863936 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -382,7 +382,6 @@ static void x86_machine_class_init(ObjectClass *oc, 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 d9d4f25d1a2..417cb4cd7f9 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 12/21] hw/i386/x86: Remove X86MachineClass::save_tsc_khz field

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:38AM +0100, 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>
> ---
>  include/hw/i386/x86.h | 5 -----
>  hw/i386/x86.c         | 1 -
>  target/i386/machine.c | 5 ++---
>  3 files changed, 2 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 12/21] hw/i386/x86: Remove X86MachineClass::save_tsc_khz field

[Thomas Huth]

On 16/01/2025 00.22, 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>
> ---
>   include/hw/i386/x86.h | 5 -----
>   hw/i386/x86.c         | 1 -
>   target/i386/machine.c | 5 ++---
>   3 files changed, 2 insertions(+), 9 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 13/21] 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>
---
 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 e9e33c57f27..a231c8cecee 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -284,8 +284,7 @@ struct MachineClass {
         no_floppy:1,
         no_cdrom:1,
         no_sdcard:1,
-        pci_allow_0_address:1,
-        legacy_fw_cfg_order:1;
+        pci_allow_0_address:1;
     bool is_default;
     const char *default_machine_opts;
     const char *default_boot_order;
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 8985046be40..036982db202 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -282,8 +282,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 4dfdb027eee..33523ee9291 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1352,20 +1352,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 137f827b69e..b244cc71ab4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1028,7 +1028,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;
@@ -1036,7 +1035,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;
 }
 
@@ -1223,8 +1222,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);
     }
@@ -1233,8 +1230,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 a757939cfb7..48d9e915209 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 be029c52ef8..5bff12ca241 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1197,10 +1197,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;
 }
 
@@ -2672,7 +2669,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) {
@@ -2683,7 +2679,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 13/21] hw/nvram/fw_cfg: Remove legacy FW_CFG_ORDER_OVERRIDE

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:39AM +0100, 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>
> ---
>  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(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index e9e33c57f27..a231c8cecee 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -284,8 +284,7 @@ struct MachineClass {
>          no_floppy:1,
>          no_cdrom:1,
>          no_sdcard:1,
> -        pci_allow_0_address:1,
> -        legacy_fw_cfg_order:1;
> +        pci_allow_0_address:1;

Pre-existing separate problem - why do we go to the trouble of using
bitfields here ???? Further down in the struct we lots of simple "bool"
fields.

The number of instances of MachineClass is not large enough that we s
ave a useful amount of memory through bitfields.

SO I'd suggest we just convert these to bools. Not needed for this patch
though.


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 :|

[PATCH 14/21] 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>
---
 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 a231c8cecee..505a7ee85cf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -837,7 +837,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 bdd4dee3d67..212944c9445 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -272,15 +272,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 14/21] hw/core/machine: Remove hw_compat_2_5[] array

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:40AM +0100, 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>
> ---
>  include/hw/boards.h | 3 ---
>  hw/core/machine.c   | 9 ---------
>  2 files changed, 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 15/21] 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>
---
 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 a10c24aab10..9e99a183c1d 100644
--- a/hw/block/fdc-isa.c
+++ b/hw/block/fdc-isa.c
@@ -112,6 +112,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     }
 
     qdev_set_legacy_instance_id(dev, isa->iobase, 2);
+    qdev_prop_set_enum(dev, "fallback", FLOPPY_DRIVE_TYPE_288);
 
     fdctrl_realize_common(dev, fdctrl, &err);
     if (err != NULL) {
@@ -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, void *data)
-- 
2.47.1

Re: [PATCH 15/21] hw/block/fdc-isa: Remove 'fallback' property

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:41AM +0100, 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>
> ---
>  hw/block/fdc-isa.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 16/21] 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>
---
 hw/scsi/vmw_pvscsi.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index f07e377cb88..8c793b4188d 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);
@@ -1298,8 +1292,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 16/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:42AM +0100, 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>
> ---
>  hw/scsi/vmw_pvscsi.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 16/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION definition

[Thomas Huth]

On 16/01/2025 00.22, 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>
> ---
>   hw/scsi/vmw_pvscsi.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index f07e377cb88..8c793b4188d 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);
> @@ -1298,8 +1292,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),
>   };

Does this compile? I think you missed a spot that uses 
PVSCSI_USE_OLD_PCI_CONFIGURATION in pvscsi_realizefn() ?

  Thomas

Re: [PATCH 16/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_OLD_PCI_CONFIGURATION definition

[Philippe Mathieu-Daudé]

On 17/1/25 10:16, Thomas Huth wrote:
> On 16/01/2025 00.22, 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>
>> ---
>>   hw/scsi/vmw_pvscsi.c | 12 ++----------
>>   1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
>> index f07e377cb88..8c793b4188d 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);
>> @@ -1298,8 +1292,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),
>>   };
> 
> Does this compile? I think you missed a spot that uses 
> PVSCSI_USE_OLD_PCI_CONFIGURATION in pvscsi_realizefn() ?

Indeed...

../../hw/scsi/vmw_pvscsi.c:1155:9: error: call to undeclared function 
'PVSCSI_USE_OLD_PCI_CONFIGURATION' [-Wimplicit-function-declaration]
  1155 |     if (PVSCSI_USE_OLD_PCI_CONFIGURATION(s)) {
       |         ^

[PATCH 17/21] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/vmw_pvscsi.c | 44 ++++++++------------------------------------
 1 file changed, 8 insertions(+), 36 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 8c793b4188d..4c95794ba1e 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)
 
@@ -1152,15 +1147,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;
@@ -1228,21 +1219,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()
@@ -1256,9 +1234,8 @@ 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_STRUCT(parent_obj, PVSCSIState, 0,
+                       vmstate_pci_device, PCIDevice),
         VMSTATE_UINT8(msi_used, PVSCSIState),
         VMSTATE_UINT32(resetting, PVSCSIState),
         VMSTATE_UINT64(reg_interrupt_status, PVSCSIState),
@@ -1292,19 +1269,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 17/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:43AM +0100, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/scsi/vmw_pvscsi.c | 44 ++++++++------------------------------------
>  1 file changed, 8 insertions(+), 36 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 17/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Thomas Huth]

On 16/01/2025 00.22, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/scsi/vmw_pvscsi.c | 44 ++++++++------------------------------------
>   1 file changed, 8 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 8c793b4188d..4c95794ba1e 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)
>   
> @@ -1152,15 +1147,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);

Ah, here it is. This should go into the previous patch instead.

>       /* PCI latency timer = 255 */
>       pci_dev->config[PCI_LATENCY_TIMER] = 0xff;
> @@ -1228,21 +1219,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()
> @@ -1256,9 +1234,8 @@ 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_STRUCT(parent_obj, PVSCSIState, 0,
> +                       vmstate_pci_device, PCIDevice),

I think this is wrong, too. pvscsi_vmstate_test_pci_device() should return 
false for modern machines, so this should be removed instead?

  Thomas

Re: [PATCH 17/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Philippe Mathieu-Daudé]

On 17/1/25 10:21, Thomas Huth wrote:
> On 16/01/2025 00.22, 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/scsi/vmw_pvscsi.c | 44 ++++++++------------------------------------
>>   1 file changed, 8 insertions(+), 36 deletions(-)


>> @@ -1228,21 +1219,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()
>> @@ -1256,9 +1234,8 @@ 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_STRUCT(parent_obj, PVSCSIState, 0,
>> +                       vmstate_pci_device, PCIDevice),
> 
> I think this is wrong, too. pvscsi_vmstate_test_pci_device() should 
> return false for modern machines, so this should be removed instead?

You are correct, good catch!

Re: [PATCH 17/21] hw/scsi/vmw_pvscsi: Remove PVSCSI_COMPAT_DISABLE_PCIE_BIT definition

[Thomas Huth]

On 16/01/2025 00.22, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
...
> @@ -1292,19 +1269,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),
>   };

While you're at it, I think this was the last user of compat_flags, so you 
could remove that compat_flags field from the struct, too, now?

  Thomas

[PATCH 18/21] 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>
---
 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 4c95794ba1e..02b4b41f4a5 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1271,21 +1271,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, 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;
@@ -1294,8 +1288,6 @@ static void pvscsi_class_init(ObjectClass *klass, 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);
@@ -1310,6 +1302,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 = (InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
         { INTERFACE_PCIE_DEVICE },
-- 
2.47.1

Re: [PATCH 18/21] hw/scsi/vmw_pvscsi: Convert DeviceRealize -> InstanceInit

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:44AM +0100, 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>
> ---
>  hw/scsi/vmw_pvscsi.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 19/21] 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>
---
 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 7abed664690..a527042ab32 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)
@@ -2123,8 +2117,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);
@@ -2222,7 +2216,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 */
@@ -2473,8 +2467,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 19/21] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:45AM +0100, 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>
> ---
>  hw/net/vmxnet3.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 20/21] 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>
---
 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 a527042ab32..d8132773fad 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)
@@ -2467,8 +2462,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)
@@ -2477,9 +2470,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 20/21] hw/net/vmxnet3: Remove VMXNET3_COMPAT_FLAG_DISABLE_PCIE definition

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:46AM +0100, 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>
> ---
>  hw/net/vmxnet3.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 21/21] 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>
---
 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 d8132773fad..2757fd83505 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2239,6 +2239,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)
@@ -2464,22 +2465,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, 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;
@@ -2490,8 +2479,6 @@ static void vmxnet3_class_init(ObjectClass *class, 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 21/21] hw/net/vmxnet3: Merge DeviceRealize in InstanceInit

[Daniel P. Berrangé]

On Thu, Jan 16, 2025 at 12:22:47AM +0100, 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>
> ---
>  hw/net/vmxnet3.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 00/21] hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines

[Michael S. Tsirkin]

On Thu, Jan 16, 2025 at 12:22:26AM +0100, Philippe Mathieu-Daudé wrote:
> 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 :)

tagged, thanks!

> Philippe Mathieu-Daudé (21):
>   hw/i386/pc: Remove unused pc_compat_2_3 declarations
>   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
>   target/i386/cpu: Pass Error** to x86_cpu_filter_features()
>   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
> 
>  docs/about/removed-features.rst |   4 ++
>  include/hw/boards.h             |   9 +--
>  include/hw/i386/pc.h            |  10 ---
>  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                  |  63 ++++--------------
>  hw/net/vmxnet3.c                |  44 ++-----------
>  hw/nvram/fw_cfg.c               | 109 ++------------------------------
>  hw/scsi/vmw_pvscsi.c            |  67 ++++----------------
>  hw/virtio/virtio-pci.c          |  15 +----
>  system/vl.c                     |   5 --
>  target/i386/cpu.c               |  27 +++-----
>  target/i386/machine.c           |   5 +-
>  23 files changed, 61 insertions(+), 454 deletions(-)
> 
> -- 
> 2.47.1

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

[Philippe Mathieu-Daudé]

On 17/1/25 11:37, Michael S. Tsirkin wrote:
> On Thu, Jan 16, 2025 at 12:22:26AM +0100, Philippe Mathieu-Daudé wrote:
>> 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 :)
> 
> tagged, thanks!

Thanks but hold on... First Thomas made review comments I need
to address, but then Daniel clarified we can not remove these
until 10.1...