Message-ID: <MWHPR11MB18861FBE62D10E1FC77AB6208C389@MWHPR11MB1886.namprd11.prod.outlook.com>
Date:   Mon, 7 Jun 2021 06:22:08 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     Jason Gunthorpe <jgg@...dia.com>,
        Cornelia Huck <cohuck@...hat.com>,
        "Kirti Wankhede" <kwankhede@...dia.com>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Dey, Megha" <megha.dey@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>, "Lu, Baolu" <baolu.lu@...el.com>,
        "Kumar, Sanjay K" <sanjay.k.kumar@...el.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "eric.auger@...hat.com" <eric.auger@...hat.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: RE: [PATCH v6 00/20] Add VFIO mediated device support and DEV-MSI
 support for the idxd driver

Hi, Alex,

Thanks for sharing your thoughts.

> From: Alex Williamson <alex.williamson@...hat.com>
> Sent: Friday, June 4, 2021 11:40 AM
> 
> On Thu, 3 Jun 2021 05:52:58 +0000
> "Tian, Kevin" <kevin.tian@...el.com> wrote:
> 
> > > From: Jason Gunthorpe <jgg@...dia.com>
> > > Sent: Thursday, June 3, 2021 9:50 AM
> > >
> > > On Thu, Jun 03, 2021 at 01:11:37AM +0000, Tian, Kevin wrote:
> > >
> > > > Jason, can you clarify your attitude on mdev guid stuff? Are you
> > > > completely against it or case-by-case? If the former, this is a big
> > > > decision thus it's better to have consensus with Alex/Kirti. If the
> > > > latter, would like to hear your criteria for when it can be used
> > > > and when not...
> > >
> > > I dislike it generally, but it exists so <shrug>. I know others feel
> > > more strongly about it being un-kernely and the wrong way to use sysfs.
> > >
> > > Here I was remarking how the example in the cover letter made the mdev
> > > part seem totally pointless. If it is pointless then don't do it.
> >
> > Is your point about that as long as a mdev requires pre-config
> > through driver specific sysfs then it doesn't make sense to use
> > mdev guid interface anymore?
> 
> Can you describe exactly what step 1. is doing in this case from the
> original cover letter ("Enable wq with "mdev" wq type")?  That does
> sound a bit like configuring something to use mdev then separately
> going to the trouble of creating the mdev.  As Jason suggests, if a wq
> is tagged for mdev/vfio, it could just register itself as a vfio bus
> driver.

I'll leave to Dave to explain the exact detail in step 1.

> 
> But if we want to use mdev, why doesn't available_instances for your
> mdev type simply report all unassigned wq and the `echo $UUID > create`
> grabs a wq for mdev?  That would remove this pre-config contention,
> right?

This way could also work. It sort of changes pre-config to post-config,
i.e. after an unassigned wq is grabbed for mdev, the admin then 
configures additional vendor specific parameters (not initialized by 
parent driver) before this mdev is assigned to a VM. Looks this is also
what NVIDIA is doing for their vGPU, with a cmdline tool (nvidia-smi)
and nvidia sysfs node for setting plugin parameters:

	https://docs.nvidia.com/grid/latest/pdf/grid-vgpu-user-guide.pdf

But I'll leave to Dave again as there must be a reason why they choose
pre-config in the first place.

> 
> > The value of mdev guid interface is providing a vendor-agnostic
> > interface for mdev life-cycle management which allows one-
> > enable-fit-all in upper management stack. Requiring vendor
> > specific pre-config does blur the boundary here.
> 
> We need to be careful about using work-avoidance in the upper
> management stack as a primary use case for an interface though.

ok

> 
> > Alex/Kirt/Cornelia, what about your opinion here? It's better
> > we can have an consensus on when and where the existing
> > mdev sysfs could be used, as this will affect every new mdev
> > implementation from now on.
> 
> I have a hard time defining some fixed criteria for using mdev.  It's
> essentially always been true that vendors could write their own vfio
> "bus driver", like vfio-pci or vfio-platform, specific to their device.
> Mdevs were meant to be a way for the (non-vfio) driver of a device to
> expose portions of the device through mediation for use with vfio.  It
> seems like that's largely being done here.
> 
> What I think has changed recently is this desire to make it easier to
> create those vendor drivers and some promise of making module binding
> work to avoid the messiness around picking a driver for the device.  In
> the auxiliary bus case that I think Jason is working on, it sounds like
> the main device driver exposes portions of itself on an auxiliary bus
> where drivers on that bus can integrate into the vfio subsystem.  It
> starts to get pretty fuzzy with what mdev already does, but it's also a
> more versatile interface.  Is it right for everyone?  Probably not.

idxd is also moving toward this model per Jason's suggestion. Although
auxiliar bus is not directly used, idxd driver has its own bus for exposing
portion of its resources. From this angle, all the motivation around mdev
bus does get fuzzy...

> 
> Is the pre-configuration issue here really a push vs pull problem?  I
> can see the requirement in step 1. is dedicating some resources to an
> mdev use case, so at that point it seems like the argument is whether we
> should just create aux bus devices that get automatically bound to a
> vendor vfio-pci variant and we avoid the mdev lifecycle, which is both
> convenient and ugly.  On the other hand, mdev has more of a pull
> interface, ie. here are a bunch of device types and how many of each we
> can support, use create to pull what you need.

I see your point. Looks what idxd is toward now is a mixed model. The
parent driver uses a push interface to initialize a pool of instances
which are then managed through mdev in a pull mode. 

> 
> > > Remember we have stripped away the actual special need to use
> > > mdev. You don't *have* to use mdev anymore to use vfio. That is a
> > > significant ideology change even from a few months ago.
> > >
> >
> > Yes, "don't have to" but if there is value of doing so  it's
> > not necessary to blocking it? One point in my mind is that if
> > we should minimize vendor-specific contracts for user to
> > manage mdev or subdevice...
> 
> Again, this in itself is not a great justification for using mdev,
> we're creating vendor specific device types with vendor specific
> additional features, that could all be done via some sort of netlink
> interface too.  The thing that pushes this more towards mdev for me is
> that I don't think each of these wqs appear as devices to the host,
> they're internal resources of the parent device and we want to compose
> them in ways that are slightly more amenable to traditional mdevs... I
> think.  Thanks,
> 

Yes, this is one reason going toward mdev.

btw I'm not clear what the netlink interface will finally be, especially 
about whether any generic cmd should be defined cross devices given 
that subdevice management still has large generality. Jason, do you have
an example somewhere which we can take a look regarding to mlx 
netlink design? 

Thanks
Kevin

Powered by blists - more mailing lists