[libvirt] [PATCH 0/3] sample: vfio mdev display devices.

[Kirti Wankhede]

On 4/24/2018 3:10 AM, Alex Williamson wrote:
> On Wed, 18 Apr 2018 12:31:53 -0600
> Alex Williamson <alex.williamson at redhat.com> wrote:
> 
>> On Mon,  9 Apr 2018 12:35:10 +0200
>> Gerd Hoffmann <kraxel at redhat.com> wrote:
>>
>>> This little series adds three drivers, for demo-ing and testing vfio
>>> display interface code.  There is one mdev device for each interface
>>> type (mdpy.ko for region and mbochs.ko for dmabuf).  
>>
>> Erik Skultety brought up a good question today regarding how libvirt is
>> meant to handle these different flavors of display interfaces and
>> knowing whether a given mdev device has display support at all.  It
>> seems that we cannot simply use the default display=auto because
>> libvirt needs to specifically configure gl support for a dmabuf type
>> interface versus not having such a requirement for a region interface,
>> perhaps even removing the emulated graphics in some cases (though I
>> don't think we have boot graphics through either solution yet).
>> Additionally, GVT-g seems to need the x-igd-opregion support
>> enabled(?), which is a non-starter for libvirt as it's an experimental
>> option!
>>
>> Currently the only way to determine display support is through the
>> VFIO_DEVICE_QUERY_GFX_PLANE ioctl, but for libvirt to probe that on
>> their own they'd need to get to the point where they could open the
>> vfio device and perform the ioctl.  That means opening a vfio
>> container, adding the group, setting the iommu type, and getting the
>> device.  I was initially a bit appalled at asking libvirt to do that,
>> but the alternative is to put this information in sysfs, but doing that
>> we risk that we need to describe every nuance of the mdev device
>> through sysfs and it becomes a dumping ground for every possible
>> feature an mdev device might have.
>>

One or two sysfs file for each feature shouldn't be that much of over
head? In kernel, other subsystem modules expose capability through
sysfs, like PCI subsystem adds 'boot_vga' file for VGA device which
returns 0/1 depending on if its boot VGA device. Similarly
'd3cold_allowed', 'msi_bus'...

>> So I was ready to return and suggest that maybe libvirt should probe
>> the device to know about these ancillary configuration details, but
>> then I remembered that both mdev vGPU vendors had external dependencies
>> to even allow probing the device.  KVMGT will fail to open the device
>> if it's not associated with an instance of KVM and NVIDIA vGPU, I
>> believe, will fail if the vGPU manager process cannot find the QEMU
>> instance to extract the VM UUID.  (Both of these were bad ideas)
> 
> Here's another proposal that's really growing on me:
> 
>  * Fix the vendor drivers!  Allow devices to be opened and probed
>    without these external dependencies.
>  * Libvirt uses the existing vfio API to open the device and probe the
>    necessary ioctls, if it can't probe the device, the feature is
>    unavailable, ie. display=off, no migration.
> 

I'm trying to think simpler mechanism using sysfs that could work for
any feature and knowing source-destination migration compatibility check
by libvirt before initiating migration.

I have another proposal:
* Add a ioctl VFIO_DEVICE_PROBE_FEATURES
struct vfio_device_features {
    __u32 argsz;
    __u32 features;
}

Define bit for each feature:
#define VFIO_DEVICE_FEATURE_DISPLAY_REGION	(1 << 0)
#define VFIO_DEVICE_FEATURE_DISPLAY_DMABUF	(1 << 1)
#define VFIO_DEVICE_FEATURE_MIGRATION		(1 << 2)

* Vendor driver returns bitmask of supported features during
initialization phase.

* In vfio core module, trap this ioctl for each device  in
vfio_device_fops_unl_ioctl(), check features bitmask returned by vendor
driver and add a sysfs file if feature is supported that device. This
sysfs file would return 0/1.

For migration this bit will only indicate if host driver supports
migration feature.

For source and destination compatibility check libvirt would need more
data/variables to check like,
* if same type of 'mdev_type' device create-able at destination,
   i.e. if ('mdev_type'->available_instances > 0)

* if host_driver_version at source and destination are compatible.
Host driver from same release branch should be mostly compatible, but if
there are major changes in structures or APIs, host drivers from
different branches might not be compatible, for example, if source and
destination are from different branches and one of the structure had
changed, then data collected at source might not be compatible with
structures at destination and typecasting it to changed structures would
mess up migrated data during restoration.

* if guest_driver_version is compatible with host driver at destination.
For mdev devices, guest driver communicates with host driver in some
form. If there are changes in structures/APIs of such communication,
guest driver at source might not be compatible with host driver at
destination.

'available_instances' sysfs already exist, later two should be added by
vendor driver which libvirt can use for migration compatibility check.

Thanks,
Kirti