Re: [Qemu-devel] [RFC PATCH v3 1/3] vGPU Core driver

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 4 May 2016 02:45:59 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Alex Williamson
> > Sent: Wednesday, May 04, 2016 6:44 AM
> >   
> > > diff --git a/drivers/vgpu/Kconfig b/drivers/vgpu/Kconfig
> > > new file mode 100644
> > > index 0000000..792eb48
> > > --- /dev/null
> > > +++ b/drivers/vgpu/Kconfig
> > > @@ -0,0 +1,21 @@
> > > +
> > > +menuconfig VGPU
> > > +    tristate "VGPU driver framework"
> > > +    depends on VFIO
> > > +    select VGPU_VFIO
> > > +    help
> > > +        VGPU provides a framework to virtualize GPU without SR-IOV cap
> > > +        See Documentation/vgpu.txt for more details.
> > > +
> > > +        If you don't know what do here, say N.
> > > +
> > > +config VGPU
> > > +    tristate
> > > +    depends on VFIO
> > > +    default n
> > > +
> > > +config VGPU_VFIO
> > > +    tristate
> > > +    depends on VGPU
> > > +    default n
> > > +  
> > 
> > This is a little bit convoluted, it seems like everything added in this
> > patch is vfio agnostic, it doesn't necessarily care what the consumer
> > is.  That makes me think we should only be adding CONFIG_VGPU here and
> > it should not depend on CONFIG_VFIO or be enabling CONFIG_VGPU_VFIO.
> > The middle config entry is also redundant to the first, just move the
> > default line up to the first and remove the rest.  
> 
> Agree. Removing such dependency also benefits other hypervisor if
> VFIO is not used.
> 
> Alex, there is one idea which I'd like to hear your comment. When looking at
> the whole series, we can see the majority logic (maybe I cannot say 100%)
> is GPU agnostic. Same frameworks in VFIO and vGPU core are actually neutral
> to underlying device type, which e.g. can be easily applied to a NIC card too
> if a similar technology is developed there.
> 
> Do you think whether we'd better make framework not GPU specific now
> (mostly naming change), or continue current style and change later only 
> when there is a real implementation on a different device? 

Yeah, I see that too and I made a bunch of comments in patch 3 that
we're not doing anything vGPU specific and we should be careful about
assuming the user for the various interfaces.  In patch 1, we are
fairly v/GPU specific because we're dealing with how vGPUs are created
from the physical GPU.  Maybe the interface is general, maybe it's not,
it's hard to say.  Starting with patch 2 though, we really shouldn't
know or care what the device is beyond a PCI compatible device.  We're
just trying to create a vfio bus driver compatible with vfio-pci and
offload enough generic operations so that we don't need to pass
everything back to the vendor driver.  Patch 3 of course should be
completely device agnostic, we should only care that the vfio backend
provides mediation of the device, so an iommu is not required.  It may
be too much of a rathole to try to completely generalize the interface
at this point, but let's certainly try not to let vgpu specific ideas
spread beyond where we need.  Thanks,

Alex