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

2018-05-04 Thread Erik Skultety
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.

2018-04-27 Thread Alex Williamson
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.

2018-04-26 Thread Dr. David Alan Gilbert
* 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.

2018-04-26 Thread Kirti Wankhede


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.

2018-04-26 Thread Alex Williamson
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.

2018-04-25 Thread Gerd Hoffmann
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.

2018-04-25 Thread Tian, Kevin
> 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.

2018-04-25 Thread Dr. David Alan Gilbert
* 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.

2018-04-25 Thread Alex Williamson
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.

2018-04-25 Thread Kirti Wankhede


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.

2018-04-25 Thread Zhang, Tina


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

2018-04-24 Thread Alex Williamson
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.

2018-04-24 Thread Kirti Wankhede


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.

2018-04-24 Thread Alex Williamson
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.

2018-04-24 Thread Gerd Hoffmann
  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.

2018-04-23 Thread Alex Williamson
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.

2018-04-19 Thread Paolo Bonzini
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.

2018-04-19 Thread Alex Williamson
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.

2018-04-19 Thread Zhenyu Wang
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.

2018-04-19 Thread Gerd Hoffmann
  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.

2018-04-18 Thread Alex Williamson
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