Re: [RFC PATCH v4 01/10] driver core: export driver_probe_device()

[Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>]

On Sat, Feb 15, 2014 at 04:33:44PM +0000, Stuart Yoder wrote:
> > > Why?  driver_probe_device() allows a driver to explicitly bind
> > > to a specific device.   What is conceptually wrong with allowing
> > > that?
> > 
> > Because that's not how a bus should work, and the fact that no other
> > subsystem in the kernel does that might be a hint you are trying to do
> > something a bit "wrong" here.
> 
> Let me try to succinctly as I can describe the problem we are trying to
> solve here...
> 
> The vfio mechanism in the kernel (e.g. vfio-pci) allows devices to be
> exposed user space (via file descriptors), enabling user space
> drivers.  So, for example to export an e1000 card to user space, I do
> this:
> 
>    echo 0001:03:00.0 > /sys/bus/pci/devices/0001:03:00.0/driver/unbind
>    echo 8086 10d3 > /sys/bus/pci/drivers/vfio-pci/new_id

What's wrong with using the "bind" file instead?  That picks a specific
device and binds it to a specific driver.  Or have we been down this
path before?  :)

And that is for a PCI "driver" not a totally separate bus, which it
looks like you are wanting to do here.

> The first step unbinds the target device (0001:03:00.0) from the normal
> e1000 driver.
> 
> The second step causes the vfio-pci driver to bind to device 0001:03:00.0.
> This second step tells vfio-pci that it now handles e1000 device IDs,
> and the vfio-pci drivers registers with the PCI bus to handle '8086 10d3'. 
> 
> That works, but it is ugly.  We now have 2 active drivers handling
> the same device type...which introduces various possible race conditions.
> 
> We never want vfio-pci to auto-bind to any new device that shows up
> on the PCI bus.  Binding a device to vfio-pci must be an explicit
> action by an administrator.

Then use the "bind" file.

> You mentioned previously that user space can sort out the problem
> of multiple drivers registered for handling the same device type.
> That is true, but doesn't help here.   We don't want vfio-pci
> to handle _all_ e1000 cards, just explicitly selected e1000 cards.
> 
> We want the normal e1000 driver to be loaded and to bind to new
> devices that may be hot-plugged.

I want a pony too...

> There are 2 proposed mechanisms that have been put forth, both of
> which you have now rejected:
> 
>    1.  sysfs_bind_only flag was proposed which would allow a vfio
>        driver (like vfio-pci) to only bind by explicit request through
>        the sysfs 'bind' file.

Why did I reject this?  What did the patch look like?

>    2.  Have the vfio driver call driver_probe_device() to explicitly bind
>        a particular device instance to the driver.  Only change we need
>        here is the EXPORT_SYMBOL.

How are you going to prevent the driver from being bound to the device
in the core with this change?  How are you going to call this function?
When?  On what action of the user?

> Are you in principle opposed to any mechanism that would allow 2 drivers
> to be resident/active and allow a sysadmin to explicitly bind a 
> particular device instance to the driver of their choice?

No, that works today with the bind/unbind/new_id files, it's just that
you don't like it :)

thanks,

greg k-h