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

["Cédric Le Goater" <clg@redhat.com>]

On 4/24/25 23:52, John Levon wrote:
> On Thu, Apr 24, 2025 at 05:17:28PM +0200, Cédric Le Goater wrote:
> 
>> !-------------------------------------------------------------------|
>>   CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> On 4/9/25 15:48, John Levon wrote:
>>> Split out parts of TYPE_VFIO_PCI into a base TYPE_VFIO_PCI_BASE. The
>>> base type contains properties generic to all vfio-pci implementations
>>> (although we have not yet introduced another subclass).
>>>
>>> Note that currently there is no need for additional data for
>>> TYPE_VFIO_PCI, so it shares the same C struct type as
>>> TYPE_VFIO_PCI_BASE, VFIOPCIDevice.
>>
>> I don't understand how the properties are distributed between the
>> abstract vfio-pci base class and the vfio-pci class. What's the
>> rationale ?
> 
> It's for properties that apply to all vfio pci classes, and those that are
> specific to the kernel vfio pci implementation.

It seems quite vague to me.

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

Thanks,

C.