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

[Alex Williamson]

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 the patch, vgpu_dev.c + vgpu_sysfs.c form such vgpu module and 
> vgpu_vfio.c is for VFIO interface. Each vgpu device should be added to 
> vfio group, so vgpu_group_init() from vgpu_vfio.c should be called per 
> device. In the vgpu module, vgpu devices are created on request, so 
> vgpu_group_init() should be called explicitly for per vgpu device. 
>   That’s why had merged the 2 modules, vgpu + vgpu_vfio to form one vgpu 
> module.  Vgpu_vfio would remain separate entity but merged with vgpu 
> module.

I disagree with this design, creation of a vgpu necessarily involves the
GPU driver and should not be tied to use of the vgpu with vfio.  vfio
should be a driver for the device, maybe eventually not the only driver
for the device.  Thanks,

Alex