Re: [Qemu-devel] VFIO based vGPU(was Re: [Announcement] 2015-Q3 release of XenGT - a Mediated ...)

[Alex Williamson]

On Thu, 2016-01-28 at 02:25 +0530, Kirti Wankhede wrote:
> 
> On 1/27/2016 9:30 PM, Alex Williamson wrote:
> > On Wed, 2016-01-27 at 13:36 +0530, Kirti Wankhede wrote:
> > > 
> > > On 1/27/2016 1:36 AM, Alex Williamson wrote:
> > > > On Tue, 2016-01-26 at 02:20 -0800, Neo Jia wrote:
> > > > > On Mon, Jan 25, 2016 at 09:45:14PM +0000, Tian, Kevin wrote:
> > > > > > > From: Alex Williamson [mailto:address@hidden
> > > > > 
> > > > > Hi Alex, Kevin and Jike,
> > > > > 
> > > > > (Seems I shouldn't use attachment, resend it again to the list, 
> > > > > patches are
> > > > > inline at the end)
> > > > > 
> > > > > Thanks for adding me to this technical discussion, a great opportunity
> > > > > for us to design together which can bring both Intel and NVIDIA vGPU 
> > > > > solution to
> > > > > KVM platform.
> > > > > 
> > > > > Instead of directly jumping to the proposal that we have been working 
> > > > > on
> > > > > recently for NVIDIA vGPU on KVM, I think it is better for me to put 
> > > > > out couple
> > > > > quick comments / thoughts regarding the existing discussions on this 
> > > > > thread as
> > > > > fundamentally I think we are solving the same problem, DMA, interrupt 
> > > > > and MMIO.
> > > > > 
> > > > > Then we can look at what we have, hopefully we can reach some 
> > > > > consensus soon.
> > > > > 
> > > > > > Yes, and since you're creating and destroying the vgpu here, this is
> > > > > > where I'd expect a struct device to be created and added to an IOMMU
> > > > > > group.  The lifecycle management should really include links between
> > > > > > the vGPU and physical GPU, which would be much, much easier to do 
> > > > > > with
> > > > > > struct devices create here rather than at the point where we start
> > > > > > doing vfio "stuff".
> > > > > 
> > > > > Infact to keep vfio-vgpu to be more generic, vgpu device creation and 
> > > > > management
> > > > > can be centralized and done in vfio-vgpu. That also include adding to 
> > > > > IOMMU
> > > > > group and VFIO group.
> > > > Is this really a good idea?  The concept of a vgpu is not unique to
> > > > vfio, we want vfio to be a driver for a vgpu, not an integral part of
> > > > the lifecycle of a vgpu.  That certainly doesn't exclude adding
> > > > infrastructure to make lifecycle management of a vgpu more consistent
> > > > between drivers, but it should be done independently of vfio.  I'll go
> > > > back to the SR-IOV model, vfio is often used with SR-IOV VFs, but vfio
> > > > does not create the VF, that's done in coordination with the PF making
> > > > use of some PCI infrastructure for consistency between drivers.
> > > > 
> > > > It seems like we need to take more advantage of the class and driver
> > > > core support to perhaps setup a vgpu bus and class with vfio-vgpu just
> > > > being a driver for those devices.
> > > 
> > > For device passthrough or SR-IOV model, PCI devices are created by PCI
> > > bus driver and from the probe routine each device is added in vfio group.
> > 
> > An SR-IOV VF is created by the PF driver using standard interfaces
> > provided by the PCI core.  The IOMMU group for a VF is added by the
> > IOMMU driver when the device is created on the pci_bus_type.  The probe
> > routine of the vfio bus driver (vfio-pci) is what adds the device into
> > the vfio group.
> > 
> > > For vgpu, there should be a common module that create vgpu device, say
> > > vgpu module, add vgpu device to an IOMMU group and then add it to vfio
> > > group.  This module can handle management of vgpus. Advantage of keeping
> > > this module a separate module than doing device creation in vendor
> > > modules is to have generic interface for vgpu management, for example,
> > > files /sys/class/vgpu/vgpu_start and  /sys/class/vgpu/vgpu_shudown and
> > > vgpu driver registration interface.
> > 
> > But you're suggesting something very different from the SR-IOV model.
> > If we wanted to mimic that model, the GPU specific driver should create
> > the vgpu using services provided by a common interface.  For instance
> > i915 could call a new vgpu_device_create() which creates the device,
> > adds it to the vgpu class, etc.  That vgpu device should not be assumed
> > to be used with vfio though, that should happen via a separate probe
> > using a vfio-vgpu driver.  It's that vfio bus driver that will add the
> > device to a vfio group.
> > 
> 
> In that case vgpu driver should provide a driver registration interface 
> to register vfio-vgpu driver.
> 
> struct vgpu_driver {
>       const char *name;
>       int (*probe) (struct vgpu_device *vdev);
>       void (*remove) (struct vgpu_device *vdev);
> }
> 
> int vgpu_register_driver(struct vgpu_driver *driver)
> {
> ...
> }
> EXPORT_SYMBOL(vgpu_register_driver);
> 
> int vgpu_unregister_driver(struct vgpu_driver *driver)
> {
> ...
> }
> EXPORT_SYMBOL(vgpu_unregister_driver);
> 
> vfio-vgpu driver registers to vgpu driver. Then from 
> vgpu_device_create(), after creating the device it calls 
> vgpu_driver->probe(vgpu_device) and vfio-vgpu driver adds the device to 
> vfio group.
> 
> +--------------+    vgpu_register_driver()+---------------+
> >     __init() +------------------------->+               |
> >              |                          |               |
> >              +<-------------------------+    vgpu.ko    |
> > vfio_vgpu.ko |   probe()/remove()       |               |
> >              |                +---------+               +---------+
> +--------------+                |         +-------+-------+         |
>                                  |                 ^                 |
>                                  | callback        |                 |
>                                  |         +-------+--------+        |
>                                  |         |vgpu_register_device()   |
>                                  |         |                |        |
>                                  +---^-----+-----+    +-----+------+-+
>                                      | nvidia.ko |    |  i915.ko   |
>                                      |           |    |            |
>                                      +-----------+    +------------+
> 
> Is my understanding correct?

We have an entire driver core subsystem in Linux for the purpose of
matching devices to drivers, I don't think we should be re-inventing
that.  That's why I'm suggesting that we should have infrastructure
which facilitates GPU drivers to create vGPU devices in a common way,
perhaps even placing the devices on a virtual vgpu bus, and then allow a
vfio-vgpu driver to register as a driver for devices of that bus/class
and use the existing driver callbacks.  Thanks,

Alex