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