RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag toallow binding via sysfs only

[Stuart Yoder]

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> Sent: Thursday, December 19, 2013 2:34 PM
> To: Wood Scott-B07421
> Cc: Kim Phillips; linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.dall@xxxxxxxxxx;
> alex.williamson@xxxxxxxxxx; a.motakis@xxxxxxxxxxxxxxxxxxxxxx;
> agraf@xxxxxxx; Sethi Varun-B16395
> Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag
> to allow binding via sysfs only
> 
> On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote:
> > On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 03, 2013 at 12:34:46PM +0000, Kim Phillips wrote:
> > > > VFIO supports pass-through of devices to user space - for sake
> > > > of illustration, say a PCI e1000 device:
> > > >
> > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs
> > > > - the vfio-pci driver is told via new_id that it now handles e1000
> devices
> > > > - the e1000 is explicitly bound to vfio-pci through sysfs
> > > >
> > > > However, now we have two drivers in the system that both handle
> e1000
> > > > devices.  A hotplug event could then occur and it is ambiguous as
> to which
> > > > driver will claim the device.  The desired semantics is that vfio-
> pci is
> > > > only bound to devices by explicit request in sysfs.  This patch
> makes this
> > > > possible by introducing a sysfs_bind_only flag in struct
> device_driver.
> > >
> > > Why deal with this at all and not just deal with the "bind" sysfs
> file
> > > instead?  That way no driver core logic needs to be changed at all,
> and
> > > your userspace tools know _exactly_ which device is being bound to
> the
> > > new device.
> > >
> > > Don't mess with the "new_id" file for stuff like this, as you point
> out,
> > > it's "tricky"...
> >
> > As discussed before, "bind" does not bypass the ID checks, and thus it
> > does not work without either "new_id" or a wildcard match.
> 
> Ah, forgot about that.
> 
> > Or are you proposing changing "bind" so that it does bypass the ID
> > checks?  Or perhaps a new "force_bind" file that does?
> 
> No.  But you can use bind/unbind along with the existing new_id file to
> get what you want today.

Yes, but that only works for PCI.  There is no such concept for platform
drivers.

> If you just happen to bind a device to a wrong
> driver for a while, that's not really a problem, right?

It's annoying but not the end of the world.

> I don't like this patch as we are adding lots of special and odd logic
> to the core, for use by almost no one, which ensures that it will never
> get tested, and will probably get broken in some subtle way in the
> future.

It certainly will be used by users of vfio-platform.

Here is the problem-- the new platform device "match_any_dev" mechanism
in patch 2 of this series is not going to work without "sysfs_bind_only".
A platform driver that just sets "match_any_dev" will grab any or all 
platform devices during normal bus probing.

> Heck, I can't even ensure that you got it right now, with this tiny
> patch, how do you know it works given that there are no users of this
> flag anywhere (hint, you never showed me any...)

It has been tested on both vfio-pci and vfio-platform in some limited
experiments as far as I know, but that code is not ready for upstream
yet.

Thanks,
Stuart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/