Re: [PATCH 08/14] vfio: add vfio-pci-base class

[Cédric Le Goater]

On 4/25/25 15:01, John Levon wrote:

On Fri, Apr 25, 2025 at 02:46:48PM +0200, Cédric Le Goater wrote:

After this patch, here is what we have for the base class :
     static const Property vfio_pci_base_dev_properties[] = {
         DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", 
VFIOPCIDevice,
                                 vbasedev.pre_copy_dirty_page_tracking,
                                 ON_OFF_AUTO_ON),
         DEFINE_PROP_ON_OFF_AUTO("x-device-dirty-page-tracking", VFIOPCIDevice,
                                 vbasedev.device_dirty_page_tracking,
                                 ON_OFF_AUTO_ON),
         DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice,
                            intx.mmap_timeout, 1100),
         DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
                                 vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
         DEFINE_PROP("x-migration-multifd-transfer", VFIOPCIDevice,
                     vbasedev.migration_multifd_transfer,
                     vfio_pci_migration_multifd_transfer_prop, OnOffAuto,
                     .set_default = true, .defval.i = ON_OFF_AUTO_AUTO),
         DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
                          vbasedev.migration_events, false),
         DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
         DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
                          vbasedev.ram_block_discard_allowed, false),
         DEFINE_PROP_BOOL("x-no-kvm-intx", VFIOPCIDevice, no_kvm_intx, false),
         DEFINE_PROP_BOOL("x-no-kvm-msi", VFIOPCIDevice, no_kvm_msi, false),
         DEFINE_PROP_BOOL("x-no-kvm-msix", VFIOPCIDevice, no_kvm_msix, false),
         DEFINE_PROP_BOOL("x-no-kvm-ioeventfd", VFIOPCIDevice, no_kvm_ioeventfd,
                          false),
         DEFINE_PROP_BOOL("x-no-vfio-ioeventfd", VFIOPCIDevice, 
no_vfio_ioeventfd,
                          false),
         DEFINE_PROP_UINT32("x-pci-vendor-id", VFIOPCIDevice, vendor_id, 
PCI_ANY_ID),
         DEFINE_PROP_UINT32("x-pci-device-id", VFIOPCIDevice, device_id, 
PCI_ANY_ID),
         DEFINE_PROP_UINT32("x-pci-sub-vendor-id", VFIOPCIDevice,
                            sub_vendor_id, PCI_ANY_ID),
         DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
                            sub_device_id, PCI_ANY_ID),
         DEFINE_PROP_OFF_AUTO_PCIBAR("x-msix-relocation", VFIOPCIDevice, 
msix_relo,
                                     OFF_AUTO_PCIBAR_OFF),
     };
and for vfio-pci :
     static const Property vfio_pci_dev_properties[] = {
         DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
         DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
         DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
         DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
                                 display, ON_OFF_AUTO_OFF),
         DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
         DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
         DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
                         VFIO_FEATURE_ENABLE_VGA_BIT, false),
         DEFINE_PROP_BIT("x-req", VFIOPCIDevice, features,
                         VFIO_FEATURE_ENABLE_REQ_BIT, true),
         DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
                         VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
         DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
         DEFINE_PROP_BIT("x-igd-lpc", VFIOPCIDevice, features,
                         VFIO_FEATURE_ENABLE_IGD_LPC_BIT, false),
         DEFINE_PROP_ON_OFF_AUTO("x-igd-legacy-mode", VFIOPCIDevice,
                                 igd_legacy_mode, ON_OFF_AUTO_AUTO),
         DEFINE_PROP_BOOL("x-no-geforce-quirks", VFIOPCIDevice,
                          no_geforce_quirks, false),
         DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
                                        nv_gpudirect_clique,
                                        qdev_prop_nv_gpudirect_clique, uint8_t),
     #ifdef CONFIG_IOMMUFD
         DEFINE_PROP_LINK("iommufd", VFIOPCIDevice, vbasedev.iommufd,
                          TYPE_IOMMUFD_BACKEND, IOMMUFDBackend *),
     #endif
         DEFINE_PROP_BOOL("skip-vsc-check", VFIOPCIDevice, skip_vsc_check, 
true),
     };
Graphic property and host device definitions are excluded from the
base class it seems. This might fit vfio-user needs but it looks
like a quick hack from the vfio-pci side. It needs more work.

Can you suggest a preferred approach? I'm definitely not wedded to the current
way (after all, I didn't write it !), but I'm not sure how else we could do it.

Perhaps if there's some way to deregister properties when vfio-user
instantiates?

Can you remind me why the vfio-pci class for vfio-user can not
inherit directly from vfio-pci ?

For the above reason: we'd inherit many properties that don't work for
vfio-user.

What do you mean by "don't work" ? functionally irrelevant ?

I don't know the answer to that in general. Certainly some are just irrelevant
(like sysfsdev), but it's entirely possible the other stuff actively breaks.
Presumably you agree it's not good to introduce potential footguns for users
here?

Do we know which properties are required for the vfio-user variant of the
vfio-pci device ?

I'd be tempted to start with an empty abstract vfio-pci-base device class.
This wouldn't change the current vfio-pci device much, and the vfio-user
variant would duplicate the necessary properties.


Thanks,

C.