Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Fri, Apr 27, 2018 at 12:15:01AM +0530, Kirti Wankhede wrote: > > > On 4/26/2018 1:22 AM, Dr. David Alan Gilbert wrote: > > * Alex Williamson (alex.william...@redhat.com) wrote: > >> On Wed, 25 Apr 2018 21:00:39 +0530 > >> Kirti Wankhede wrote: > >> > >>> On 4/25/2018 4:29 AM, Alex Williamson wrote: > On Wed, 25 Apr 2018 01:20:08 +0530 > Kirti Wankhede wrote: > > > On 4/24/2018 3:10 AM, Alex Williamson wrote: > >> On Wed, 18 Apr 2018 12:31:53 -0600 > >> Alex Williamson wrote: > >> > >>> On Mon, 9 Apr 2018 12:35:10 +0200 > >>> Gerd Hoffmann 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. > >> ... > >>> 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(), > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > blocking point with mdev drivers, we can't get a device fd, so we can't > call an ioctl on the device fd. > > >>> > >>> I'm sorry, I thought we could expose features when QEMU initialize, but > >>> libvirt needs to know supported features before QEMU initialize. > >>> > >>> > > 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. > > I don't understand why we have an ioctl interface, if the user can get > to the device fd then we have existing interfaces to probe these > things, it seems like you're just
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Thu, 26 Apr 2018 19:55:23 +0100 "Dr. David Alan Gilbert" wrote: > * Kirti Wankhede (kwankh...@nvidia.com) wrote: > > > > > > On 4/26/2018 1:22 AM, Dr. David Alan Gilbert wrote: > > > * Alex Williamson (alex.william...@redhat.com) wrote: > > >> On Wed, 25 Apr 2018 21:00:39 +0530 > > >> Kirti Wankhede wrote: > > >> > > >>> On 4/25/2018 4:29 AM, Alex Williamson wrote: > > On Wed, 25 Apr 2018 01:20:08 +0530 > > Kirti Wankhede wrote: > > > > > On 4/24/2018 3:10 AM, Alex Williamson wrote: > > >> On Wed, 18 Apr 2018 12:31:53 -0600 > > >> Alex Williamson wrote: > > >> > > >>> On Mon, 9 Apr 2018 12:35:10 +0200 > > >>> Gerd Hoffmann 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. > > >> ... > > >>> 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(), > > > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > > blocking point with mdev drivers, we can't get a device fd, so we can't > > call an ioctl on the device fd. > > > > >>> > > >>> I'm sorry, I thought we could expose features when QEMU initialize, but > > >>> libvirt needs to
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
* Kirti Wankhede (kwankh...@nvidia.com) wrote: > > > On 4/26/2018 1:22 AM, Dr. David Alan Gilbert wrote: > > * Alex Williamson (alex.william...@redhat.com) wrote: > >> On Wed, 25 Apr 2018 21:00:39 +0530 > >> Kirti Wankhede wrote: > >> > >>> On 4/25/2018 4:29 AM, Alex Williamson wrote: > On Wed, 25 Apr 2018 01:20:08 +0530 > Kirti Wankhede wrote: > > > On 4/24/2018 3:10 AM, Alex Williamson wrote: > >> On Wed, 18 Apr 2018 12:31:53 -0600 > >> Alex Williamson wrote: > >> > >>> On Mon, 9 Apr 2018 12:35:10 +0200 > >>> Gerd Hoffmann 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. > >> ... > >>> 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(), > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > blocking point with mdev drivers, we can't get a device fd, so we can't > call an ioctl on the device fd. > > >>> > >>> I'm sorry, I thought we could expose features when QEMU initialize, but > >>> libvirt needs to know supported features before QEMU initialize. > >>> > >>> > > 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. > > I don't understand why we have an ioctl interface, if the user can get > to the device fd then we have existing interfaces to probe these > things,
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On 4/26/2018 1:22 AM, Dr. David Alan Gilbert wrote: > * Alex Williamson (alex.william...@redhat.com) wrote: >> On Wed, 25 Apr 2018 21:00:39 +0530 >> Kirti Wankhede wrote: >> >>> On 4/25/2018 4:29 AM, Alex Williamson wrote: On Wed, 25 Apr 2018 01:20:08 +0530 Kirti Wankhede wrote: > On 4/24/2018 3:10 AM, Alex Williamson wrote: >> On Wed, 18 Apr 2018 12:31:53 -0600 >> Alex Williamson wrote: >> >>> On Mon, 9 Apr 2018 12:35:10 +0200 >>> Gerd Hoffmann 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. >> ... >>> 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(), Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our blocking point with mdev drivers, we can't get a device fd, so we can't call an ioctl on the device fd. >>> >>> I'm sorry, I thought we could expose features when QEMU initialize, but >>> libvirt needs to know supported features before QEMU initialize. >>> >>> > 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. I don't understand why we have an ioctl interface, if the user can get to the device fd then we have existing interfaces to probe these things, it seems like you're just wanting to pass a features bitmap through to vfio_add_group_dev() that vfio-core would expose through sysfs, but a list of feature bits doesn't convey enough info except for the most basic uses. >>> >>> Yes, vfio_add_group_d
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Thu, 26 Apr 2018 08:14:27 +0200 Gerd Hoffmann wrote: > On Thu, Apr 26, 2018 at 03:44:15AM +, Tian, Kevin wrote: > > > From: Alex Williamson > > > Sent: Thursday, April 19, 2018 2:32 AM > > > > > > That almost begins to look reasonable, but then we can only expose this > > > for mdev devices, what if we were to hack a back door into a directly > > > assigned GPU that tracks the location of active display in the > > > framebuffer and implement the GFX_PLANE interface for that? We have no > > > sysfs representation for either the template or the actual device for > > > anything other than mdev. This inconsistency with physically assigned > > > devices has been one of my arguments against enhancing mdev sysfs. > > > > One possible option is to wrap directly assigned GPU into a mdev. The > > parent driver could be a dummy PCI driver which does basic PCI > > initialization, and then provide hooks for vendor-specific hack. > > Thowing amdgpu into the mix. Looks they have vgpu support too, but > using sriov instead of mdev. Having VFIO_GFX support surely looks > useful there. Adding a mdev dependency to the VFIO_GFX api would makes > things more complicated there for (IMHO) no good reason ... Yes, it may be that a device wanting to implement display or migration might take the mdev approach, but that should be a choice of the implementation, not a requirement imposed by the API. Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Thu, Apr 26, 2018 at 03:44:15AM +, Tian, Kevin wrote: > > From: Alex Williamson > > Sent: Thursday, April 19, 2018 2:32 AM > > > > That almost begins to look reasonable, but then we can only expose this > > for mdev devices, what if we were to hack a back door into a directly > > assigned GPU that tracks the location of active display in the > > framebuffer and implement the GFX_PLANE interface for that? We have no > > sysfs representation for either the template or the actual device for > > anything other than mdev. This inconsistency with physically assigned > > devices has been one of my arguments against enhancing mdev sysfs. > > One possible option is to wrap directly assigned GPU into a mdev. The > parent driver could be a dummy PCI driver which does basic PCI > initialization, and then provide hooks for vendor-specific hack. Thowing amdgpu into the mix. Looks they have vgpu support too, but using sriov instead of mdev. Having VFIO_GFX support surely looks useful there. Adding a mdev dependency to the VFIO_GFX api would makes things more complicated there for (IMHO) no good reason ... cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
> From: Alex Williamson > Sent: Thursday, April 19, 2018 2:32 AM > > That almost begins to look reasonable, but then we can only expose this > for mdev devices, what if we were to hack a back door into a directly > assigned GPU that tracks the location of active display in the > framebuffer and implement the GFX_PLANE interface for that? We have no > sysfs representation for either the template or the actual device for > anything other than mdev. This inconsistency with physically assigned > devices has been one of my arguments against enhancing mdev sysfs. > One possible option is to wrap directly assigned GPU into a mdev. The parent driver could be a dummy PCI driver which does basic PCI initialization, and then provide hooks for vendor-specific hack. Thanks Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
* Alex Williamson (alex.william...@redhat.com) wrote: > On Wed, 25 Apr 2018 21:00:39 +0530 > Kirti Wankhede wrote: > > > On 4/25/2018 4:29 AM, Alex Williamson wrote: > > > On Wed, 25 Apr 2018 01:20:08 +0530 > > > Kirti Wankhede wrote: > > > > > >> On 4/24/2018 3:10 AM, Alex Williamson wrote: > > >>> On Wed, 18 Apr 2018 12:31:53 -0600 > > >>> Alex Williamson wrote: > > >>> > > On Mon, 9 Apr 2018 12:35:10 +0200 > > Gerd Hoffmann 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. > ... > > 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(), > > > > > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > > > blocking point with mdev drivers, we can't get a device fd, so we can't > > > call an ioctl on the device fd. > > > > > > > I'm sorry, I thought we could expose features when QEMU initialize, but > > libvirt needs to know supported features before QEMU initialize. > > > > > > >> 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. > > > > > > I don't understand why we have an ioctl interface, if the user can get > > > to the device fd then we have existing interfaces to probe these > > > things, it seems like you're just wanting to pass a features bitmap > > > through to vfio_add_group_dev() that vfio-core would expose through > > > sysfs, but a list of feature bits doesn't convey enough info except for > > > the most basic uses. > > >
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Wed, 25 Apr 2018 21:00:39 +0530 Kirti Wankhede wrote: > On 4/25/2018 4:29 AM, Alex Williamson wrote: > > On Wed, 25 Apr 2018 01:20:08 +0530 > > Kirti Wankhede wrote: > > > >> On 4/24/2018 3:10 AM, Alex Williamson wrote: > >>> On Wed, 18 Apr 2018 12:31:53 -0600 > >>> Alex Williamson wrote: > >>> > On Mon, 9 Apr 2018 12:35:10 +0200 > Gerd Hoffmann 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. ... > 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(), > > > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > > blocking point with mdev drivers, we can't get a device fd, so we can't > > call an ioctl on the device fd. > > > > I'm sorry, I thought we could expose features when QEMU initialize, but > libvirt needs to know supported features before QEMU initialize. > > > >> 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. > > > > I don't understand why we have an ioctl interface, if the user can get > > to the device fd then we have existing interfaces to probe these > > things, it seems like you're just wanting to pass a features bitmap > > through to vfio_add_group_dev() that vfio-core would expose through > > sysfs, but a list of feature bits doesn't convey enough info except for > > the most basic uses. > > > > Yes, vfio_add_group_dev() seems to be better way to convey features to > vfio core. > > >> For migration this bit will only indicate if host driver supports > >> migration feature. > >> > >> For source and destination compatibility check libvirt would ne
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On 4/25/2018 4:29 AM, Alex Williamson wrote: > On Wed, 25 Apr 2018 01:20:08 +0530 > Kirti Wankhede wrote: > >> On 4/24/2018 3:10 AM, Alex Williamson wrote: >>> On Wed, 18 Apr 2018 12:31:53 -0600 >>> Alex Williamson wrote: >>> On Mon, 9 Apr 2018 12:35:10 +0200 Gerd Hoffmann 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'... > > Obviously we could add sysfs files, but unlike properties that the PCI > core exposes about struct pci_dev fields, the idea of a vfio_device is > much more abstract. Each bus driver creates its own device > representation, so we have a top level vfio_device referencing through > an opaque pointer a vfio_pci_device, vfio_platform_device, or > mdev_device, and each mdev vendor driver creates its own private data > structure below the mdev_device. So it's not quite a simple as one new > attribute "show" function to handle all devices of that bus_type. We > need a consistent implementation in each bus driver and vendor driver > or we need to figure out how to percolate the information up to the > vfio core. Your idea below seems to take the percolate approach. > 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(), > > Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our > blocking point with mdev drivers, we can't get a device fd, so we can't > call an ioctl on the device fd. > I'm sorry, I thought we could expose features when QEMU initialize, but libvirt needs to know supported fe
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
> -Original Message- > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On > Behalf Of Alex Williamson > Sent: Wednesday, April 25, 2018 1:36 AM > To: Gerd Hoffmann > Cc: k...@vger.kernel.org; Erik Skultety ; libvirt > l...@redhat.com>; Zhang, Tina ; > kwankh...@nvidia.com; intel-gvt-...@lists.freedesktop.org > Subject: Re: [PATCH 0/3] sample: vfio mdev display devices. > > On Tue, 24 Apr 2018 09:17:37 +0200 > Gerd Hoffmann wrote: > > > Hi, > > > > > 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. > > > > Hmm. If you try use gvt with tcg then, wouldn't qemu think "device > > probed ok, all green" then even though that isn't the case? > > Well, is there a way to make it work with tcg? That would be the best > solution. > Perhaps KVM could be handled as an accelerator rather than a required > component. I don't really understand how the page tracking interface is used > and why it's not required by NVIDIA if it's so fundamental to GVT-g. > Otherwise, GVT-g needs hypervisors' (like Xen or KVM) help to trap the guest GPU page table update, so that GVT-g can update the shadow page table correctly, in host, with host physical address, not guest physical address. As this page table is in memory, GVT-g needs hypervisors' help to make it write protected, so that it can trap the updates on time. > are there other points at which the device could refuse to be enabled, for > instance what if the write to enable bus-master in the PCI command register > returned an error if the device isn't fully configured. Paolo had suggested > offline If we add some logic to let GVT-g support basic VFIO APIs even in tcg use case, could the following things be reasonable? 1. A dummy vGPU is created with an UUID. 2. When VFIO_DEVICE_GET_INFO is invoked by libvirt, GVT-g tells that this vGPU is actually a dummy one and cannot work. 3. Then libvirt choose not to boot a VM with this dummy vGPU. 4. Maybe we also need some logic to let a VM with this dummy vGPU boot and work just as there is no vGPU support. Thanks. BR, Tina > that maybe there could be a read-only mode of the device that allows probing. > I > think that would be a fair bit of work and complexity to support, but I'm > open to > those sorts of ideas. I can't be sure the NVIDIA requirement isn't purely for > accounting purposes within their own proprietary userspace manager, without > any real technical requirement. > Hoping Intel and NVIDIA can comment on these so we really understand why > these are in place before we bend over backwards for a secondary API > interface. > Thanks, > > Alex > ___ > intel-gvt-dev mailing list > intel-gvt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Wed, 25 Apr 2018 01:20:08 +0530 Kirti Wankhede wrote: > On 4/24/2018 3:10 AM, Alex Williamson wrote: > > On Wed, 18 Apr 2018 12:31:53 -0600 > > Alex Williamson wrote: > > > >> On Mon, 9 Apr 2018 12:35:10 +0200 > >> Gerd Hoffmann 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'... Obviously we could add sysfs files, but unlike properties that the PCI core exposes about struct pci_dev fields, the idea of a vfio_device is much more abstract. Each bus driver creates its own device representation, so we have a top level vfio_device referencing through an opaque pointer a vfio_pci_device, vfio_platform_device, or mdev_device, and each mdev vendor driver creates its own private data structure below the mdev_device. So it's not quite a simple as one new attribute "show" function to handle all devices of that bus_type. We need a consistent implementation in each bus driver and vendor driver or we need to figure out how to percolate the information up to the vfio core. Your idea below seems to take the percolate approach. > >> 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(), Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our blocking point with mdev drivers, we can't get a device fd, so we can't call an ioctl on the device fd. > 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. I don't understand why we have an ioctl interface, if the user can get to
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On 4/24/2018 3:10 AM, Alex Williamson wrote: > On Wed, 18 Apr 2018 12:31:53 -0600 > Alex Williamson wrote: > >> On Mon, 9 Apr 2018 12:35:10 +0200 >> Gerd Hoffmann 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
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Tue, 24 Apr 2018 09:17:37 +0200 Gerd Hoffmann wrote: > Hi, > > > 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. > > Hmm. If you try use gvt with tcg then, wouldn't qemu think "device > probed ok, all green" then even though that isn't the case? Well, is there a way to make it work with tcg? That would be the best solution. Perhaps KVM could be handled as an accelerator rather than a required component. I don't really understand how the page tracking interface is used and why it's not required by NVIDIA if it's so fundamental to GVT-g. Otherwise, are there other points at which the device could refuse to be enabled, for instance what if the write to enable bus-master in the PCI command register returned an error if the device isn't fully configured. Paolo had suggested offline that maybe there could be a read-only mode of the device that allows probing. I think that would be a fair bit of work and complexity to support, but I'm open to those sorts of ideas. I can't be sure the NVIDIA requirement isn't purely for accounting purposes within their own proprietary userspace manager, without any real technical requirement. Hoping Intel and NVIDIA can comment on these so we really understand why these are in place before we bend over backwards for a secondary API interface. Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
Hi, > 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. Hmm. If you try use gvt with tcg then, wouldn't qemu think "device probed ok, all green" then even though that isn't the case? cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Wed, 18 Apr 2018 12:31:53 -0600 Alex Williamson wrote: > On Mon, 9 Apr 2018 12:35:10 +0200 > Gerd Hoffmann 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. > > 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 really having a hard time getting behind inventing a secondary API just to work around arbitrary requirements from mdev vendor drivers. vfio was never intended to be locked to QEMU or KVM, these two vendor drivers are the only examples of such requirements, and we're only encouraging this behavior if we add a redundant API for device probing. Any solution on the table currently would require changes to the mdev vendor drivers, so why not this change? Please defend why each driver needs these external dependencies and why the device open callback is the best, or only, place in the stack to enforce that dependency. Let's see what we're really dealing with here. Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On 19/04/2018 10:40, Gerd Hoffmann wrote: >> 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) > Oops. I've trapped into the kvm issue too. Wondering what the reason > is, shouldn't this work with tcg too? As far as I understand, KVMGT requires KVM support in order to track writes to guest memory. It's a kernel API provided by the kvm.ko module, so no TCG support. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Thu, 19 Apr 2018 10:40:18 +0200 Gerd Hoffmann wrote: > > 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) > > Oops. I've trapped into the kvm issue too. Wondering what the reason > is, shouldn't this work with tcg too? It's used for some sort of page tracking backdoor. Yes, I think vfio devices, including mdev, should work with tcg. Separating device assignment to not be integrally tied to kvm is something I've strived for with vfio. > But, yes, that indeed pretty much kills the "just let libvirt use the > probe ioctl" idea. > > > The existing device_api file reports "vfio-pci", so we base the device > > API info in a directory named vfio-pci. We're specifically exposing > > device information, so we have a device directory. We have a GFX_PLANE > > query ioctl, so we have a gfx_plane sub-directory. I imagine the > > dmabuf and region files here expose either Y/N or 1/0. > > Do we want tie this to vfio-pci? All existing devices are actually pci, > and the qemu code only works for vfio-pci devices too. But at vfio api > level there is no vfio-pci dependency I'm aware of, and I think we > shouldn't add one without a good reason. The intention was to tie it to 'device_api' which reports 'vfio-pci', so the user would read the device_api, learn that it uses vfio-pci, then look for attributes in a vfio-pci sub-directory. If device_api reported vfio-ccw, they'd look for a vfio-ccw directory. > Should we just add a gfx_plane_api file maybe? Which would be a > comma-separated list of interfaces, listed in order of preference in > case multiple are supported. I'm afraid that as soon as we get away from a strict representation of the vfio API, we're going to see feature creep with such a solution. Ex. which hw encoders are supported, frame rate limiters, number of heads, etc. > > anything other than mdev. This inconsistency with physically assigned > > devices has been one of my arguments against enhancing mdev sysfs. > > > > Thanks to anyone still reading this. Ideas how we might help libvirt > > fill this information void so that they can actually configure a VM > > with a display device? Thanks, > > Well, no good idea for the physical assigned device case. Minimally, I think anything we decide needs to be placed into the instantiated device sysfs hierarchy rather than the template directory for a given mdev type, otherwise we have no hope of supporting it with physical devices. > PS: Any comment on the sample driver patches? Or should I take the lack > of comments as "no news is good news, they are queued up already"? I do not have them queued yet, I'll take a closer look at them shortly and let you know if I find any issues. Thanks for doing these! I think they'll be very helpful, especially for the task above to provide reference implementations for whatever API exposure we design. Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On 2018.04.19 10:40:18 +0200, Gerd Hoffmann wrote: > Hi, > > > 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). > > Correct, no boot graphics yet. The option to disable emulated graphics > should be added nevertheless. It's an option after all, you don't have > to use it. > > But after install things usually work just fine, it just takes a little > longer for the guest display to show up.. There is also the option to > add a serial console to the guest for boot loader access. > > > 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! > > Windows guests need it, yes. And it seems we have still have to add igd > opregion support to ovmf as only bios guests are working. Or hack up a > efi rom doing that. But patching ovmf is probably alot easier because > it already has support code for fw_cfg access. > > Linux i915.ko is happy without opregion. > yeah, that's true. > > 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) > > Oops. I've trapped into the kvm issue too. Wondering what the reason > is, shouldn't this work with tcg too? > > But, yes, that indeed pretty much kills the "just let libvirt use the > probe ioctl" idea. I also don't like that strict link and although now KVM is the only upstream hypervisor GVT supports, we shouldn't require a must available instance for some device info access. > > > The existing device_api file reports "vfio-pci", so we base the device > > API info in a directory named vfio-pci. We're specifically exposing > > device information, so we have a device directory. We have a GFX_PLANE > > query ioctl, so we have a gfx_plane sub-directory. I imagine the > > dmabuf and region files here expose either Y/N or 1/0. > > Do we want tie this to vfio-pci? All existing devices are actually pci, > and the qemu code only works for vfio-pci devices too. But at vfio api > level there is no vfio-pci dependency I'm aware of, and I think we > shouldn't add one without a good reason. > > Should we just add a gfx_plane_api file maybe? Which would be a > comma-separated list of interfaces, listed in order of preference in > case multiple are supported. Or a 'feature' file with defined string list for those capabilities? Might be easier to extend in future. > > > anything other than mdev. This inconsistency with physically assigned > > devices has been one of my arguments against enhancing mdev sysfs. > > > > Thanks to anyone still reading this. Ideas how we might help libvirt > > fill this information void so that they can actually configure a VM > > with a display device? Thanks, > > Well, no good idea for the physical assigned device case. > > cheers, > Gerd > > PS: Any comment on the sample driver patches? Or should I take the lack > of comments as "no news is good news, they are queued up already"? > ___ > intel-gvt-dev mailing list > intel-gvt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
Hi, > 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). Correct, no boot graphics yet. The option to disable emulated graphics should be added nevertheless. It's an option after all, you don't have to use it. But after install things usually work just fine, it just takes a little longer for the guest display to show up.. There is also the option to add a serial console to the guest for boot loader access. > 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! Windows guests need it, yes. And it seems we have still have to add igd opregion support to ovmf as only bios guests are working. Or hack up a efi rom doing that. But patching ovmf is probably alot easier because it already has support code for fw_cfg access. Linux i915.ko is happy without opregion. > 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) Oops. I've trapped into the kvm issue too. Wondering what the reason is, shouldn't this work with tcg too? But, yes, that indeed pretty much kills the "just let libvirt use the probe ioctl" idea. > The existing device_api file reports "vfio-pci", so we base the device > API info in a directory named vfio-pci. We're specifically exposing > device information, so we have a device directory. We have a GFX_PLANE > query ioctl, so we have a gfx_plane sub-directory. I imagine the > dmabuf and region files here expose either Y/N or 1/0. Do we want tie this to vfio-pci? All existing devices are actually pci, and the qemu code only works for vfio-pci devices too. But at vfio api level there is no vfio-pci dependency I'm aware of, and I think we shouldn't add one without a good reason. Should we just add a gfx_plane_api file maybe? Which would be a comma-separated list of interfaces, listed in order of preference in case multiple are supported. > anything other than mdev. This inconsistency with physically assigned > devices has been one of my arguments against enhancing mdev sysfs. > > Thanks to anyone still reading this. Ideas how we might help libvirt > fill this information void so that they can actually configure a VM > with a display device? Thanks, Well, no good idea for the physical assigned device case. cheers, Gerd PS: Any comment on the sample driver patches? Or should I take the lack of comments as "no news is good news, they are queued up already"? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.
On Mon, 9 Apr 2018 12:35:10 +0200 Gerd Hoffmann 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. 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) Therefore, how can libvirt know if a given mdev device supports a display and which type of display it supports, and potentially which vendor specific options might be required to further enable that display (if they weren't experimental)? A terrible solution would be that libvirt hard codes that NVIDIA works with regions and Intel works with dmabufs, but even then there's a backwards and forwards compatibility problem, that libvirt needs to support older kernels and drivers where display support is not present and newer drivers where perhaps Intel is now doing regions and NVIDIA is supporting dmabuf, so it cannot simply be assumed based on the vendor. The only solution I see down that path would be identifying specific {vendor,type} pairs that support a predefined display type, but that's just absurd to think that vendors would rev their mdev types to expose this and that libvirt would keep a database mapping types to features. We also have the name and description attributes, but these are currently free form, so libvirt rightfully ignores them entirely. I don't know if we could create a defined feature string within those free form strings. Otherwise, it seems we have no choice but to dive into the pool of exposing such features via sysfs and we'll need to be vigilant of feature creep or vendor specific features (ex. we're not adding a feature to indicate an opregion requirement). How should we do this? Perhaps a bar we can set is that if a feature cannot be discovered through a standard vfio API, then it is not suitable for this sysfs API. Such things can be described via our existing mdev vendor specific attribute interface. We currently have this sysfs interface: mdev_supported_types/ |-- $VENDOR_TYPE | |-- available_instances | |-- create | |-- description | |-- device_api | |-- devices | `-- name ioctls for vfio devices which only provide information include: VFIO_DEVICE_GET_INFO VFIO_DEVICE_GET_REGION_INFO VFIO_DEVICE_GET_IRQ_INFO VFIO_DEVICE_GET_PCI_HOT_RESET_INFO VFIO_DEVICE_QUERY_GFX_PLANE We don't need to support all of these initially, but here's a starting idea for what this may look like in sysfs: $VENDOR_TYPE/ |-- available_instances |-- create |-- description |-- device_api |-- devices |-- name `-- vfio-pci `-- device |-- gfx_plane | |-- dmabuf | `-- region |-- irqs | |-- 0 | | |-- count | | `-- flags | `-- 1 | |-- count | `-- flags `-- regions |-- 0 | |-- flags | |-- offset | `-- size `-- 3 |-- flags |-- offset `-- size The existing device_api file reports "vfio-pci", so we base the device API info in a directory named vfio-pci. We're specifically exposing device information, so we have a device directory. We ha