Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-10 at 03:01 -0500, Bhushan Bharat-R65777 wrote:
>
> > -Original Message-
> > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On
> > Behalf Of
> > Kim Phillips
> > Sent: Thursday, October 10, 2013 8:36 AM
> > To: Wood Scott-B07421
> > Cc: Yoder Stuart-B08248; Wood Scott-B07421; christoffer.d...@linaro.org;
> > alex.william...@redhat.com; linux-kernel@vger.kernel.org;
> > a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan
> > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org;
> > k...@vger.kernel.org; gre...@linuxfoundation.org
> > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device
> >
> > On Wed, 9 Oct 2013 15:03:19 -0500
> > Scott Wood wrote:
> >
> > > On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
> > > > > From: Wood Scott-B07421
> > > > > Sent: Wednesday, October 09, 2013 2:22 PM
> > > > >
> > > > > On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> > > > > > Have been thinking about this issue some more. As Scott
> > > > > > mentioned,
> >
> > thanks for bringing this up again.
> >
> > > > > There's already a "bool suppress_bind_attrs" to prevent sysfs
> > > > > bind/unbind. I suggested a similar flag to mean the oppsosite --
> > > > > bind
> > > > > *only* through sysfs. Greg KH was skeptical and wanted to see a
> > > > > patch before any further discussion.
> > > >
> > > > Ah, think I understand now...yes that works as well, and would be
> > > > less intrustive. So are you writing a patch? :)
> > >
> > > I've been meaning to since the previous round of discussion, but I've
> > > been busy. Would someone else be able to test it in the context of
> > > using it for VFIO?
> >
> > yes - see below.
> >
> > > Otherwise, that looks about right, for the driver side (though
> > > driver_attach could error out earlier rather than testing it inside
> > > the loop).
> >
> > I've made the changes you suggested and tested the resulting diff below on
> > an
> > arndale board. I successfully performed the following sequence of commands
> > after first changing the i2c@12C8 node in the device tree to be
> > exclusively
> > compatible with "vfio":
This is not the intended usage. Leave the device tree alone, add a
wildcard option to platform_match() and use it in VFIO, and set
drv->sysfs_bind_only in VFIO.
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 73f6c29..da81442
> > 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv,
> > const
> > char *buf,
> > int err = -ENODEV;
> >
> > dev = bus_find_device_by_name(bus, NULL, buf);
> > - if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
> > + if (dev && dev->driver == NULL && (drv->sysfs_bind_only ||
> > + driver_match_device(drv, dev))) {
>
> Should not we check
> if (dev && dev->driver == NULL &&
> (device->explicit_bind_only && drv->explicit_bind_only) ||
> driver_match_device(drv, dev)))
device->sysfs_bind_only would be a separate patch.
As for drv->sysfs_bind_only, that does not override
driver_match_device(). Wildcard matches are separate and are handled by
individual bus match functions. This function does not need to be
changed at all for drv->sysfs_bind_only.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-10 at 02:45 -0500, Bhushan Bharat-R65777 wrote: > > > -Original Message- > > From: Wood Scott-B07421 > > Sent: Thursday, October 10, 2013 1:33 AM > > To: Yoder Stuart-B08248 > > Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson; > > linux- > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > > Sethi > > Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; > > santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device > > > > On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: > > > Ah, think I understand now...yes that works as well, and would be > > > less intrustive. So are you writing a patch? :) > > > > I've been meaning to since the previous round of discussion, but I've been > > busy. > > Would someone else be able to test it in the context of using it for VFIO? > > I wish I could have but I do not have vfio-platform stuff. VFIO PCI without new_id would also be a useful test. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> -Original Message- > From: Wood Scott-B07421 > Sent: Thursday, October 10, 2013 8:53 PM > To: Bhushan Bharat-R65777 > Cc: Wood Scott-B07421; Yoder Stuart-B08248; Kim Phillips; Christoffer Dall; > Alex > Williamson; linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com; > ag...@suse.de; Sethi Varun-B16395; peter.mayd...@linaro.org; > santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device > > On Thu, 2013-10-10 at 02:45 -0500, Bhushan Bharat-R65777 wrote: > > > > > -Original Message- > > > From: Wood Scott-B07421 > > > Sent: Thursday, October 10, 2013 1:33 AM > > > To: Yoder Stuart-B08248 > > > Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex > > > Williamson; linux- ker...@vger.kernel.org; > > > a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; > > > Bhushan Bharat-R65777; peter.mayd...@linaro.org; > > > santosh.shu...@linaro.org; k...@vger.kernel.org; > > > gre...@linuxfoundation.org > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a > > > platform device > > > > > > On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: > > > > Ah, think I understand now...yes that works as well, and would be > > > > less intrustive. So are you writing a patch? :) > > > > > > I've been meaning to since the previous round of discussion, but I've been > busy. > > > Would someone else be able to test it in the context of using it for VFIO? > > > > I wish I could have but I do not have vfio-platform stuff. > > VFIO PCI without new_id would also be a useful test. I will do that :) -Bharat > > -Scott >
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> I am trying to understand what you are proposing here (example "DEVICE" > can be handled by "DRIVER1" and "VFIO-PLATFORM-DRIVER"): > - By default drv->explicit_bind_only will be clear in all drivers. > - By default device->explicit_bind_only will also be clear for all > devices. > - On boot, matching devices will bound to the respective driver (DEVICE > >==> DRIVER1). >This will never bound with VFIO-PLATFORM-DRIVER. So far same as > before. > - Via Sysfs interface set drv->explicit_bind_only for VFIO-PLATFORM- > DRIVER. No. VFIO-PLATFORM-DRIVER is _always_ explicit_bind_only and thus will be statically set in the driver. See Kim's patch. > - Then for the devices user want, set device->explicit_bind_only. > - unbind DEVICE from DRIVER1 > - bind DEVICE with VFIO-PLATFORM-DRIVER. This time it will be successful > because (device->explicit_bind_only && drv->explicit_bind_only) is set. > - Now when done, unbind the DEVICE from VFIO-PLATFORM-DRIVER. > - Now user can re-bind the device with either DRIVER1 or VFIO-PLATFORM- > DRIVER. > - Now once drv->explicit_bind_only is set in VFIO-PLATFORM-DRIVER, and a > new device comes (device - hotplug) then can gets bound to matching drive > and not with VFIO-PLATFORM-DRIVER. Otherwise, it looks correct to me. Stuart
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> -Original Message- > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf > Of > Kim Phillips > Sent: Thursday, October 10, 2013 8:36 AM > To: Wood Scott-B07421 > Cc: Yoder Stuart-B08248; Wood Scott-B07421; christoffer.d...@linaro.org; > alex.william...@redhat.com; linux-kernel@vger.kernel.org; > a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > k...@vger.kernel.org; gre...@linuxfoundation.org > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device > > On Wed, 9 Oct 2013 15:03:19 -0500 > Scott Wood wrote: > > > On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: > > > > From: Wood Scott-B07421 > > > > Sent: Wednesday, October 09, 2013 2:22 PM > > > > > > > > On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote: > > > > > Have been thinking about this issue some more. As Scott > > > > > mentioned, > > thanks for bringing this up again. > > > > > There's already a "bool suppress_bind_attrs" to prevent sysfs > > > > bind/unbind. I suggested a similar flag to mean the oppsosite -- > > > > bind > > > > *only* through sysfs. Greg KH was skeptical and wanted to see a > > > > patch before any further discussion. > > > > > > Ah, think I understand now...yes that works as well, and would be > > > less intrustive. So are you writing a patch? :) > > > > I've been meaning to since the previous round of discussion, but I've > > been busy. Would someone else be able to test it in the context of > > using it for VFIO? > > yes - see below. > > > Otherwise, that looks about right, for the driver side (though > > driver_attach could error out earlier rather than testing it inside > > the loop). > > I've made the changes you suggested and tested the resulting diff below on an > arndale board. I successfully performed the following sequence of commands > after first changing the i2c@12C8 node in the device tree to be > exclusively > compatible with "vfio": > > === > # ls -l /sys/bus/platform/drivers/vfio-platform/ > total 0 > --w--- 1 root root 4096 Sep 24 19:17 bind > --w--- 1 root root 4096 Sep 24 19:13 uevent > --w--- 1 root root 4096 Sep 24 19:18 unbind # ls -l > /sys/bus/platform/drivers/s3c-i2c total 0 > lrwxrwxrwx 1 root root0 Sep 24 19:11 12c6.i2c -> > ../../../../devices/12c6.i2c > lrwxrwxrwx 1 root root0 Sep 24 19:11 12c9.i2c -> > ../../../../devices/12c9.i2c > lrwxrwxrwx 1 root root0 Sep 24 19:20 12ce.i2c -> > ../../../../devices/12ce.i2c > --w--- 1 root root 4096 Sep 24 19:18 bind > --w--- 1 root root 4096 Sep 24 19:11 uevent > --w--- 1 root root 4096 Sep 24 19:17 unbind # ls -l > /sys/devices/12c8.i2c/driver # this is the one with the 'vfio' compatible > ls: cannot access /sys/devices/12c8.i2c/driver: No such file or directory > # > ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 root root 0 Sep 24 19:18 > /sys/devices/12ce.i2c/driver -> ../../bus/platform/drivers/s3c-i2c > # echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/unbind > # ls -l /sys/devices/12ce.i2c/driver > ls: cannot access /sys/devices/12ce.i2c/driver: No such file or directory > # > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > # ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 root root 0 Sep 24 19:21 > /sys/devices/12ce.i2c/driver -> ../../bus/platform/drivers/vfio-platform > # echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/unbind > # ls -l /sys/devices/12ce.i2c/driver # echo 12ce.i2c > > /sys/bus/platform/drivers/s3c-i2c/bind > [ 722.137524] s3c-i2c 12ce.i2c: slave address 0x38 [ 722.141037] s3c-i2c > 12ce.i2c: bus frequency set to 65 KHz [ 722.150605] s3c-i2c 12ce.i2c: > i2c-8: S3C I2C adapter # ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 > root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver -> > ../../bus/platform/drivers/s3c-i2c > # > > > so it's correctly not allowing 'vfio' driver to bind to a device tree > compatible > it's declared, and it then can bind the i2c @ 12ce device to the vfio- > platform driver, and unbind and bind it back to the i2c driver. > > For clarity's sake, before this diff, the command: > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > would error with: > > echo: write error: No such device > > > The other half
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> -Original Message-
> From: Wood Scott-B07421
> Sent: Thursday, October 10, 2013 1:33 AM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson; linux-
> ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi
> Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org;
> santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org
> Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device
>
> On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
> >
> > > -Original Message-
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, October 09, 2013 2:22 PM
> > > To: Yoder Stuart-B08248
> > > Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex
> > > Williamson; linux-kernel@vger.kernel.org;
> > > a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395;
> > > Bhushan Bharat-R65777; peter.mayd...@linaro.org;
> > > santosh.shu...@linaro.org; k...@vger.kernel.org;
> > > gre...@linuxfoundation.org
> > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a
> > > platform device
> > >
> > > On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> > > > Have been thinking about this issue some more. As Scott
> > > > mentioned, 'wildcard' matching for a driver can be fairly done in
> > > > the platform bus driver. We could add a new flag to the platform driver
> struct:
> > > >
> > > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > > index 4f8bef3..4d6cf14 100644
> > > > --- a/drivers/base/platform.c
> > > > +++ b/drivers/base/platform.c
> > > > @@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
> > > struct device_driver *drv)
> > > > struct platform_device *pdev = to_platform_device(dev);
> > > > struct platform_driver *pdrv = to_platform_driver(drv);
> > > >
> > > > + /* the driver matches any device */
> > > > + if (pdrv->match_any)
> > > > + return 1;
> > > > +
> > > > /* Attempt an OF style match first */
> > > > if (of_driver_match_device(dev, drv))
> > > > return 1;
> > > >
> > > > However, the more problematic issue is that a bus driver has no
> > > > way to differentiate from an explicit bind request via sysfs and a
> > > > bind that happened through bus probing.
> > >
> > > Again, I think the wildcard match should be orthogonal to "don't
> > > bind by default" as far as the mechanism goes.
> > >
> > > There's already a "bool suppress_bind_attrs" to prevent sysfs
> > > bind/unbind. I suggested a similar flag to mean the oppsosite --
> > > bind
> > > *only* through sysfs. Greg KH was skeptical and wanted to see a
> > > patch before any further discussion.
> >
> > Ah, think I understand now...yes that works as well, and would be
> > less intrustive. So are you writing a patch? :)
>
> I've been meaning to since the previous round of discussion, but I've been
> busy.
> Would someone else be able to test it in the context of using it for VFIO?
I wish I could have but I do not have vfio-platform stuff.
>
> > It would be something like this, right?
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c index
> > 35fa368..c9a61ea 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver
> > *drv, void *data) {
> > struct device *dev = data;
> >
> > - if (!driver_match_device(drv, dev))
> > + if (!drv->explicit_bind_only && !driver_match_device(drv,
> > + dev))
> > return 0;
>
> if (drv->explicit_bind_only || !driver_match_device(drv, dev))
> return 0;
Scott,
I am trying to understand what you are proposing here (example "DEVICE" can be
handled by "DRIVER1" and "VFIO-PLATFORM-DRIVER"):
- By default drv->explicit_bind_only will be clear in all drivers.
- By default device->explicit_bind_only will also be clear for all devices.
- On boot, matching devices will bound to the respective driver (DEVICE >==>
DRIVER1).
This will never bound with VFIO-PLATFORM-DRIVER. So far same as before.
- Via Sysfs interface set drv->explicit_bind_only for VFIO-PLATFORM-DRIVER.
- T
RE: RFC: (re-)binding the VFIO platform driver to a platform device
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, October 10, 2013 1:33 AM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson; linux-
ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi
Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org;
santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org
Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
-Original Message-
From: Wood Scott-B07421
Sent: Wednesday, October 09, 2013 2:22 PM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex
Williamson; linux-kernel@vger.kernel.org;
a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395;
Bhushan Bharat-R65777; peter.mayd...@linaro.org;
santosh.shu...@linaro.org; k...@vger.kernel.org;
gre...@linuxfoundation.org
Subject: Re: RFC: (re-)binding the VFIO platform driver to a
platform device
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott
mentioned, 'wildcard' matching for a driver can be fairly done in
the platform bus driver. We could add a new flag to the platform driver
struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
struct device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no
way to differentiate from an explicit bind request via sysfs and a
bind that happened through bus probing.
Again, I think the wildcard match should be orthogonal to don't
bind by default as far as the mechanism goes.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite --
bind
*only* through sysfs. Greg KH was skeptical and wanted to see a
patch before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
I've been meaning to since the previous round of discussion, but I've been
busy.
Would someone else be able to test it in the context of using it for VFIO?
I wish I could have but I do not have vfio-platform stuff.
It would be something like this, right?
diff --git a/drivers/base/dd.c b/drivers/base/dd.c index
35fa368..c9a61ea 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver
*drv, void *data) {
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv,
+ dev))
return 0;
if (drv-explicit_bind_only || !driver_match_device(drv, dev))
return 0;
Scott,
I am trying to understand what you are proposing here (example DEVICE can be
handled by DRIVER1 and VFIO-PLATFORM-DRIVER):
- By default drv-explicit_bind_only will be clear in all drivers.
- By default device-explicit_bind_only will also be clear for all devices.
- On boot, matching devices will bound to the respective driver (DEVICE ==
DRIVER1).
This will never bound with VFIO-PLATFORM-DRIVER. So far same as before.
- Via Sysfs interface set drv-explicit_bind_only for VFIO-PLATFORM-DRIVER.
- Then for the devices user want, set device-explicit_bind_only.
- unbind DEVICE from DRIVER1
- bind DEVICE with VFIO-PLATFORM-DRIVER. This time it will be successful
because (device-explicit_bind_only drv-explicit_bind_only) is set.
- Now when done, unbind the DEVICE from VFIO-PLATFORM-DRIVER.
- Now user can re-bind the device with either DRIVER1 or VFIO-PLATFORM-DRIVER.
- Now once drv-explicit_bind_only is set in VFIO-PLATFORM-DRIVER, and a new
device comes (device - hotplug) then can gets bound to matching drive and not
with VFIO-PLATFORM-DRIVER.
This looks ok to me :)
Thanks
-Bharat
return driver_probe_device(drv, dev); @@ -450,7 +450,7 @@
static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv,
+ dev))
return 0;
Likewise -- or error out earlier in driver_attach().
Otherwise
RE: RFC: (re-)binding the VFIO platform driver to a platform device
-Original Message- From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf Of Kim Phillips Sent: Thursday, October 10, 2013 8:36 AM To: Wood Scott-B07421 Cc: Yoder Stuart-B08248; Wood Scott-B07421; christoffer.d...@linaro.org; alex.william...@redhat.com; linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device On Wed, 9 Oct 2013 15:03:19 -0500 Scott Wood scottw...@freescale.com wrote: On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: From: Wood Scott-B07421 Sent: Wednesday, October 09, 2013 2:22 PM On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote: Have been thinking about this issue some more. As Scott mentioned, thanks for bringing this up again. There's already a bool suppress_bind_attrs to prevent sysfs bind/unbind. I suggested a similar flag to mean the oppsosite -- bind *only* through sysfs. Greg KH was skeptical and wanted to see a patch before any further discussion. Ah, think I understand now...yes that works as well, and would be less intrustive. So are you writing a patch? :) I've been meaning to since the previous round of discussion, but I've been busy. Would someone else be able to test it in the context of using it for VFIO? yes - see below. Otherwise, that looks about right, for the driver side (though driver_attach could error out earlier rather than testing it inside the loop). I've made the changes you suggested and tested the resulting diff below on an arndale board. I successfully performed the following sequence of commands after first changing the i2c@12C8 node in the device tree to be exclusively compatible with vfio: === # ls -l /sys/bus/platform/drivers/vfio-platform/ total 0 --w--- 1 root root 4096 Sep 24 19:17 bind --w--- 1 root root 4096 Sep 24 19:13 uevent --w--- 1 root root 4096 Sep 24 19:18 unbind # ls -l /sys/bus/platform/drivers/s3c-i2c total 0 lrwxrwxrwx 1 root root0 Sep 24 19:11 12c6.i2c - ../../../../devices/12c6.i2c lrwxrwxrwx 1 root root0 Sep 24 19:11 12c9.i2c - ../../../../devices/12c9.i2c lrwxrwxrwx 1 root root0 Sep 24 19:20 12ce.i2c - ../../../../devices/12ce.i2c --w--- 1 root root 4096 Sep 24 19:18 bind --w--- 1 root root 4096 Sep 24 19:11 uevent --w--- 1 root root 4096 Sep 24 19:17 unbind # ls -l /sys/devices/12c8.i2c/driver # this is the one with the 'vfio' compatible ls: cannot access /sys/devices/12c8.i2c/driver: No such file or directory # ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 root root 0 Sep 24 19:18 /sys/devices/12ce.i2c/driver - ../../bus/platform/drivers/s3c-i2c # echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/unbind # ls -l /sys/devices/12ce.i2c/driver ls: cannot access /sys/devices/12ce.i2c/driver: No such file or directory # echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind # ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver - ../../bus/platform/drivers/vfio-platform # echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/unbind # ls -l /sys/devices/12ce.i2c/driver # echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind [ 722.137524] s3c-i2c 12ce.i2c: slave address 0x38 [ 722.141037] s3c-i2c 12ce.i2c: bus frequency set to 65 KHz [ 722.150605] s3c-i2c 12ce.i2c: i2c-8: S3C I2C adapter # ls -l /sys/devices/12ce.i2c/driver lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver - ../../bus/platform/drivers/s3c-i2c # so it's correctly not allowing 'vfio' driver to bind to a device tree compatible it's declared, and it then can bind the i2c @ 12ce device to the vfio- platform driver, and unbind and bind it back to the i2c driver. For clarity's sake, before this diff, the command: echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind would error with: echo: write error: No such device The other half of fixing the raciness is to ensure that the device doesn't get bound back to a non-VFIO driver (e.g. due to a module load or new_id). The solution I proposed for that was a similar explicit-bind-only flag for a device, that the user sets through sysfs prior to unbinding. This would also be useful in non-VFIO contexts to simply say I don't want to use this device at all. I can take a look at doing this if you're still busy. Thanks, Kim diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 73f6c29..da81442 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -201,7 +201,8 @@ static ssize_t bind_store(struct
RE: RFC: (re-)binding the VFIO platform driver to a platform device
I am trying to understand what you are proposing here (example DEVICE can be handled by DRIVER1 and VFIO-PLATFORM-DRIVER): - By default drv-explicit_bind_only will be clear in all drivers. - By default device-explicit_bind_only will also be clear for all devices. - On boot, matching devices will bound to the respective driver (DEVICE == DRIVER1). This will never bound with VFIO-PLATFORM-DRIVER. So far same as before. - Via Sysfs interface set drv-explicit_bind_only for VFIO-PLATFORM- DRIVER. No. VFIO-PLATFORM-DRIVER is _always_ explicit_bind_only and thus will be statically set in the driver. See Kim's patch. - Then for the devices user want, set device-explicit_bind_only. - unbind DEVICE from DRIVER1 - bind DEVICE with VFIO-PLATFORM-DRIVER. This time it will be successful because (device-explicit_bind_only drv-explicit_bind_only) is set. - Now when done, unbind the DEVICE from VFIO-PLATFORM-DRIVER. - Now user can re-bind the device with either DRIVER1 or VFIO-PLATFORM- DRIVER. - Now once drv-explicit_bind_only is set in VFIO-PLATFORM-DRIVER, and a new device comes (device - hotplug) then can gets bound to matching drive and not with VFIO-PLATFORM-DRIVER. Otherwise, it looks correct to me. Stuart
RE: RFC: (re-)binding the VFIO platform driver to a platform device
-Original Message- From: Wood Scott-B07421 Sent: Thursday, October 10, 2013 8:53 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Yoder Stuart-B08248; Kim Phillips; Christoffer Dall; Alex Williamson; linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device On Thu, 2013-10-10 at 02:45 -0500, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, October 10, 2013 1:33 AM To: Yoder Stuart-B08248 Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: Ah, think I understand now...yes that works as well, and would be less intrustive. So are you writing a patch? :) I've been meaning to since the previous round of discussion, but I've been busy. Would someone else be able to test it in the context of using it for VFIO? I wish I could have but I do not have vfio-platform stuff. VFIO PCI without new_id would also be a useful test. I will do that :) -Bharat -Scott
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-10 at 02:45 -0500, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, October 10, 2013 1:33 AM To: Yoder Stuart-B08248 Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote: Ah, think I understand now...yes that works as well, and would be less intrustive. So are you writing a patch? :) I've been meaning to since the previous round of discussion, but I've been busy. Would someone else be able to test it in the context of using it for VFIO? I wish I could have but I do not have vfio-platform stuff. VFIO PCI without new_id would also be a useful test. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-10 at 03:01 -0500, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On
Behalf Of
Kim Phillips
Sent: Thursday, October 10, 2013 8:36 AM
To: Wood Scott-B07421
Cc: Yoder Stuart-B08248; Wood Scott-B07421; christoffer.d...@linaro.org;
alex.william...@redhat.com; linux-kernel@vger.kernel.org;
a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395; Bhushan
Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org;
k...@vger.kernel.org; gre...@linuxfoundation.org
Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 9 Oct 2013 15:03:19 -0500
Scott Wood scottw...@freescale.com wrote:
On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
From: Wood Scott-B07421
Sent: Wednesday, October 09, 2013 2:22 PM
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott
mentioned,
thanks for bringing this up again.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite --
bind
*only* through sysfs. Greg KH was skeptical and wanted to see a
patch before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
I've been meaning to since the previous round of discussion, but I've
been busy. Would someone else be able to test it in the context of
using it for VFIO?
yes - see below.
Otherwise, that looks about right, for the driver side (though
driver_attach could error out earlier rather than testing it inside
the loop).
I've made the changes you suggested and tested the resulting diff below on
an
arndale board. I successfully performed the following sequence of commands
after first changing the i2c@12C8 node in the device tree to be
exclusively
compatible with vfio:
This is not the intended usage. Leave the device tree alone, add a
wildcard option to platform_match() and use it in VFIO, and set
drv-sysfs_bind_only in VFIO.
diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 73f6c29..da81442
100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv,
const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL (drv-sysfs_bind_only ||
+ driver_match_device(drv, dev))) {
Should not we check
if (dev dev-driver == NULL
(device-explicit_bind_only drv-explicit_bind_only) ||
driver_match_device(drv, dev)))
device-sysfs_bind_only would be a separate patch.
As for drv-sysfs_bind_only, that does not override
driver_match_device(). Wildcard matches are separate and are handled by
individual bus match functions. This function does not need to be
changed at all for drv-sysfs_bind_only.
-Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 9 Oct 2013 15:03:19 -0500
Scott Wood wrote:
> On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, October 09, 2013 2:22 PM
> > >
> > > On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> > > > Have been thinking about this issue some more. As Scott mentioned,
thanks for bringing this up again.
> > > There's already a "bool suppress_bind_attrs" to prevent sysfs
> > > bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
> > > *only* through sysfs. Greg KH was skeptical and wanted to see a patch
> > > before any further discussion.
> >
> > Ah, think I understand now...yes that works as well, and would be
> > less intrustive. So are you writing a patch? :)
>
> I've been meaning to since the previous round of discussion, but I've
> been busy. Would someone else be able to test it in the context of
> using it for VFIO?
yes - see below.
> Otherwise, that looks about right, for the driver side (though
> driver_attach could error out earlier rather than testing it inside the
> loop).
I've made the changes you suggested and tested the resulting diff below
on an arndale board. I successfully performed the following sequence of
commands after first changing the i2c@12C8 node in the device tree
to be exclusively compatible with "vfio":
===
# ls -l /sys/bus/platform/drivers/vfio-platform/
total 0
--w--- 1 root root 4096 Sep 24 19:17 bind
--w--- 1 root root 4096 Sep 24 19:13 uevent
--w--- 1 root root 4096 Sep 24 19:18 unbind
# ls -l /sys/bus/platform/drivers/s3c-i2c
total 0
lrwxrwxrwx 1 root root0 Sep 24 19:11 12c6.i2c ->
../../../../devices/12c6.i2c
lrwxrwxrwx 1 root root0 Sep 24 19:11 12c9.i2c ->
../../../../devices/12c9.i2c
lrwxrwxrwx 1 root root0 Sep 24 19:20 12ce.i2c ->
../../../../devices/12ce.i2c
--w--- 1 root root 4096 Sep 24 19:18 bind
--w--- 1 root root 4096 Sep 24 19:11 uevent
--w--- 1 root root 4096 Sep 24 19:17 unbind
# ls -l /sys/devices/12c8.i2c/driver # this is the one with the 'vfio'
compatible
ls: cannot access /sys/devices/12c8.i2c/driver: No such file or directory
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:18 /sys/devices/12ce.i2c/driver ->
../../bus/platform/drivers/s3c-i2c
# echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/unbind
# ls -l /sys/devices/12ce.i2c/driver
ls: cannot access /sys/devices/12ce.i2c/driver: No such file or directory
# echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver ->
../../bus/platform/drivers/vfio-platform
# echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/unbind
# ls -l /sys/devices/12ce.i2c/driver
# echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind
[ 722.137524] s3c-i2c 12ce.i2c: slave address 0x38
[ 722.141037] s3c-i2c 12ce.i2c: bus frequency set to 65 KHz
[ 722.150605] s3c-i2c 12ce.i2c: i2c-8: S3C I2C adapter
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver ->
../../bus/platform/drivers/s3c-i2c
#
so it's correctly not allowing 'vfio' driver to bind to a device tree
compatible it's declared, and it then can bind the i2c @ 12ce device
to the vfio-platform driver, and unbind and bind it back to the i2c
driver.
For clarity's sake, before this diff, the command:
echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind
would error with:
echo: write error: No such device
> The other half of fixing the raciness is to ensure that the device
> doesn't get bound back to a non-VFIO driver (e.g. due to a module load
> or new_id). The solution I proposed for that was a similar
> explicit-bind-only flag for a device, that the user sets through sysfs
> prior to unbinding. This would also be useful in non-VFIO contexts to
> simply say "I don't want to use this device at all".
I can take a look at doing this if you're still busy.
Thanks,
Kim
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 73f6c29..da81442 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
+ if (dev && dev->driver == NULL && (drv->sysfs_bind_only ||
+ driver_match_device(drv, dev))) {
if (dev->parent)/* Needed for USB */
device_lock(dev->parent);
device_lock(dev);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..6f85279 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
>
> > -Original Message-
> > From: Wood Scott-B07421
> > Sent: Wednesday, October 09, 2013 2:22 PM
> > To: Yoder Stuart-B08248
> > Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson;
> > linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com;
> > ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777;
> > peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org;
> > gre...@linuxfoundation.org
> > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform
> > device
> >
> > On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> > > Have been thinking about this issue some more. As Scott mentioned,
> > > 'wildcard' matching for a driver can be fairly done in the platform
> > > bus driver. We could add a new flag to the platform driver struct:
> > >
> > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > index 4f8bef3..4d6cf14 100644
> > > --- a/drivers/base/platform.c
> > > +++ b/drivers/base/platform.c
> > > @@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
> > struct device_driver *drv)
> > > struct platform_device *pdev = to_platform_device(dev);
> > > struct platform_driver *pdrv = to_platform_driver(drv);
> > >
> > > + /* the driver matches any device */
> > > + if (pdrv->match_any)
> > > + return 1;
> > > +
> > > /* Attempt an OF style match first */
> > > if (of_driver_match_device(dev, drv))
> > > return 1;
> > >
> > > However, the more problematic issue is that a bus driver has no way to
> > > differentiate from an explicit bind request via sysfs and a bind that
> > > happened through bus probing.
> >
> > Again, I think the wildcard match should be orthogonal to "don't bind by
> > default" as far as the mechanism goes.
> >
> > There's already a "bool suppress_bind_attrs" to prevent sysfs
> > bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
> > *only* through sysfs. Greg KH was skeptical and wanted to see a patch
> > before any further discussion.
>
> Ah, think I understand now...yes that works as well, and would be
> less intrustive. So are you writing a patch? :)
I've been meaning to since the previous round of discussion, but I've
been busy. Would someone else be able to test it in the context of
using it for VFIO?
> It would be something like this, right?
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 35fa368..c9a61ea 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> void *data)
> {
> struct device *dev = data;
>
> - if (!driver_match_device(drv, dev))
> + if (!drv->explicit_bind_only && !driver_match_device(drv, dev))
> return 0;
if (drv->explicit_bind_only || !driver_match_device(drv, dev))
return 0;
> return driver_probe_device(drv, dev);
> @@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
> * is an error.
> */
>
> - if (!driver_match_device(drv, dev))
> + if (!drv->explicit_bind_only && !driver_match_device(drv, dev))
> return 0;
Likewise -- or error out earlier in driver_attach().
Otherwise, that looks about right, for the driver side (though
driver_attach could error out earlier rather than testing it inside the
loop).
The other half of fixing the raciness is to ensure that the device
doesn't get bound back to a non-VFIO driver (e.g. due to a module load
or new_id). The solution I proposed for that was a similar
explicit-bind-only flag for a device, that the user sets through sysfs
prior to unbinding. This would also be useful in non-VFIO contexts to
simply say "I don't want to use this device at all".
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 12:16 -0700, gre...@linuxfoundation.org wrote:
> On Wed, Oct 09, 2013 at 07:02:25PM +, Yoder Stuart-B08248 wrote:
> > Have been thinking about this issue some more. As Scott mentioned,
> > 'wildcard' matching for a driver can be fairly done in the platform
> > bus driver. We could add a new flag to the platform driver struct:
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 4f8bef3..4d6cf14 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
> > device_driver *drv)
> > struct platform_device *pdev = to_platform_device(dev);
> > struct platform_driver *pdrv = to_platform_driver(drv);
> >
> > + /* the driver matches any device */
> > + if (pdrv->match_any)
> > + return 1;
> > +
> > /* Attempt an OF style match first */
> > if (of_driver_match_device(dev, drv))
> > return 1;
> >
> > However, the more problematic issue is that a bus driver has no way to
> > differentiate from an explicit bind request via sysfs and a bind that
> > happened through bus probing.
>
> That was by design, nice to see I implemented it properly :)
>
> > I think something like the new flag in the snippet below would enable the
> > platform
> > bus to support platform drivers that only bind on explicit request:
> >
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > index 4c289ab..daf6d24 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv,
> > const char *buf,
> > int err = -ENODEV;
> >
> > dev = bus_find_device_by_name(bus, NULL, buf);
> > - if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
> > + if (dev && dev->driver == NULL && driver_match_device(drv, dev, 1))
> > {
>
> Magic flags are the spawn of your favorite anti-$DIETY. I'm never going
> to accept that, sorry.
>
> If you really want to do something "special" for the platform bus, then
> do it only for the platform bus. But even then, you'll find me arguing
> that you really don't want to do it at all, sorry.
It's not (or shouldn't be) special for the platform bus. The "don't
bind by default" flag (note that I am *not* referring to the above code
snippet) would be useful for VFIO PCI as well, as it would replace the
hacky and racy usage of new_id.
> I'm still yet to be convinced this is even an issue at all, but maybe
> that's just the jetlag talking...
If this is because you think we should use new_id, we just had a
discussion in this thread about why that's no good.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, October 09, 2013 2:22 PM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson;
> linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com;
> ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777;
> peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org;
> gre...@linuxfoundation.org
> Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform
> device
>
> On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> > Have been thinking about this issue some more. As Scott mentioned,
> > 'wildcard' matching for a driver can be fairly done in the platform
> > bus driver. We could add a new flag to the platform driver struct:
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 4f8bef3..4d6cf14 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
> struct device_driver *drv)
> > struct platform_device *pdev = to_platform_device(dev);
> > struct platform_driver *pdrv = to_platform_driver(drv);
> >
> > + /* the driver matches any device */
> > + if (pdrv->match_any)
> > + return 1;
> > +
> > /* Attempt an OF style match first */
> > if (of_driver_match_device(dev, drv))
> > return 1;
> >
> > However, the more problematic issue is that a bus driver has no way to
> > differentiate from an explicit bind request via sysfs and a bind that
> > happened through bus probing.
>
> Again, I think the wildcard match should be orthogonal to "don't bind by
> default" as far as the mechanism goes.
>
> There's already a "bool suppress_bind_attrs" to prevent sysfs
> bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
> *only* through sysfs. Greg KH was skeptical and wanted to see a patch
> before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
It would be something like this, right?
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..c9a61ea 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv, void
*data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!drv->explicit_bind_only && !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!drv->explicit_bind_only && !driver_match_device(drv, dev))
return 0;
if (dev->parent)/* Needed for USB */
diff --git a/include/linux/device.h b/include/linux/device.h
index 2a9d6ed..f2be980 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -236,6 +236,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool explicit_bind_only;/* enables bind/unbind via sysfs only */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
Stuart
N�r��yb�X��ǧv�^�)Þº{.n�+{zX����ܨ}ï¿½ï¿½ï¿½Æ z�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jÇ«y�m��@A�a���
0��h���i
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 09, 2013 at 07:02:25PM +, Yoder Stuart-B08248 wrote:
> Have been thinking about this issue some more. As Scott mentioned,
> 'wildcard' matching for a driver can be fairly done in the platform
> bus driver. We could add a new flag to the platform driver struct:
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4f8bef3..4d6cf14 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
> device_driver *drv)
> struct platform_device *pdev = to_platform_device(dev);
> struct platform_driver *pdrv = to_platform_driver(drv);
>
> + /* the driver matches any device */
> + if (pdrv->match_any)
> + return 1;
> +
> /* Attempt an OF style match first */
> if (of_driver_match_device(dev, drv))
> return 1;
>
> However, the more problematic issue is that a bus driver has no way to
> differentiate from an explicit bind request via sysfs and a bind that
> happened through bus probing.
That was by design, nice to see I implemented it properly :)
> I think something like the new flag in the snippet below would enable the
> platform
> bus to support platform drivers that only bind on explicit request:
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 4c289ab..daf6d24 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv,
> const char *buf,
> int err = -ENODEV;
>
> dev = bus_find_device_by_name(bus, NULL, buf);
> - if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
> + if (dev && dev->driver == NULL && driver_match_device(drv, dev, 1)) {
Magic flags are the spawn of your favorite anti-$DIETY. I'm never going
to accept that, sorry.
If you really want to do something "special" for the platform bus, then
do it only for the platform bus. But even then, you'll find me arguing
that you really don't want to do it at all, sorry.
I'm still yet to be convinced this is even an issue at all, but maybe
that's just the jetlag talking...
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
> Have been thinking about this issue some more. As Scott mentioned,
> 'wildcard' matching for a driver can be fairly done in the platform
> bus driver. We could add a new flag to the platform driver struct:
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4f8bef3..4d6cf14 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
> device_driver *drv)
> struct platform_device *pdev = to_platform_device(dev);
> struct platform_driver *pdrv = to_platform_driver(drv);
>
> + /* the driver matches any device */
> + if (pdrv->match_any)
> + return 1;
> +
> /* Attempt an OF style match first */
> if (of_driver_match_device(dev, drv))
> return 1;
>
> However, the more problematic issue is that a bus driver has no way to
> differentiate from an explicit bind request via sysfs and a bind that
> happened through bus probing.
Again, I think the wildcard match should be orthogonal to "don't bind by
default" as far as the mechanism goes.
There's already a "bool suppress_bind_attrs" to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
*only* through sysfs. Greg KH was skeptical and wanted to see a patch
before any further discussion.
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 2cbc677..7a15ef3 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -114,9 +114,9 @@ extern void driver_detach(struct device_driver *drv);
> extern int driver_probe_device(struct device_driver *drv, struct device
> *dev);
> extern void driver_deferred_probe_del(struct device *dev);
> static inline int driver_match_device(struct device_driver *drv,
> - struct device *dev)
> + struct device *dev, int explicit_bind)
> {
> - return drv->bus->match ? drv->bus->match(dev, drv) : 1;
> + return drv->bus->match ? drv->bus->match(dev, drv, explicit_bind) : 1;
> }
>
> Of, course the above change would need to be propagated to the different
> bus drivers that implement the 'match' function.
...which would not be a problem with my approach, because you could
handle it in the callers of driver_match_device().
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv->match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
I think something like the new flag in the snippet below would enable the
platform
bus to support platform drivers that only bind on explicit request:
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..daf6d24 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
+ if (dev && dev->driver == NULL && driver_match_device(drv, dev, 1)) {
if (dev->parent)/* Needed for USB */
device_lock(dev->parent);
device_lock(dev);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..bb53785 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv, void
*data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!driver_match_device(drv, dev, 0))
return 0;
return driver_probe_device(drv, dev);
@@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!driver_match_device(drv, dev, 0))
return 0;
if (dev->parent)/* Needed for USB */
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 2cbc677..7a15ef3 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -114,9 +114,9 @@ extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device *dev);
extern void driver_deferred_probe_del(struct device *dev);
static inline int driver_match_device(struct device_driver *drv,
- struct device *dev)
+ struct device *dev, int explicit_bind)
{
- return drv->bus->match ? drv->bus->match(dev, drv) : 1;
+ return drv->bus->match ? drv->bus->match(dev, drv, explicit_bind) : 1;
}
Of, course the above change would need to be propagated to the different
bus drivers that implement the 'match' function.
Regards,
Stuart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
I think something like the new flag in the snippet below would enable the
platform
bus to support platform drivers that only bind on explicit request:
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..daf6d24 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL driver_match_device(drv, dev, 1)) {
if (dev-parent)/* Needed for USB */
device_lock(dev-parent);
device_lock(dev);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..bb53785 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv, void
*data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!driver_match_device(drv, dev, 0))
return 0;
return driver_probe_device(drv, dev);
@@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!driver_match_device(drv, dev, 0))
return 0;
if (dev-parent)/* Needed for USB */
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 2cbc677..7a15ef3 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -114,9 +114,9 @@ extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device *dev);
extern void driver_deferred_probe_del(struct device *dev);
static inline int driver_match_device(struct device_driver *drv,
- struct device *dev)
+ struct device *dev, int explicit_bind)
{
- return drv-bus-match ? drv-bus-match(dev, drv) : 1;
+ return drv-bus-match ? drv-bus-match(dev, drv, explicit_bind) : 1;
}
Of, course the above change would need to be propagated to the different
bus drivers that implement the 'match' function.
Regards,
Stuart
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
Again, I think the wildcard match should be orthogonal to don't bind by
default as far as the mechanism goes.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
*only* through sysfs. Greg KH was skeptical and wanted to see a patch
before any further discussion.
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 2cbc677..7a15ef3 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -114,9 +114,9 @@ extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device
*dev);
extern void driver_deferred_probe_del(struct device *dev);
static inline int driver_match_device(struct device_driver *drv,
- struct device *dev)
+ struct device *dev, int explicit_bind)
{
- return drv-bus-match ? drv-bus-match(dev, drv) : 1;
+ return drv-bus-match ? drv-bus-match(dev, drv, explicit_bind) : 1;
}
Of, course the above change would need to be propagated to the different
bus drivers that implement the 'match' function.
...which would not be a problem with my approach, because you could
handle it in the callers of driver_match_device().
-Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 09, 2013 at 07:02:25PM +, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
That was by design, nice to see I implemented it properly :)
I think something like the new flag in the snippet below would enable the
platform
bus to support platform drivers that only bind on explicit request:
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..daf6d24 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv,
const char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL driver_match_device(drv, dev, 1)) {
Magic flags are the spawn of your favorite anti-$DIETY. I'm never going
to accept that, sorry.
If you really want to do something special for the platform bus, then
do it only for the platform bus. But even then, you'll find me arguing
that you really don't want to do it at all, sorry.
I'm still yet to be convinced this is even an issue at all, but maybe
that's just the jetlag talking...
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
-Original Message-
From: Wood Scott-B07421
Sent: Wednesday, October 09, 2013 2:22 PM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson;
linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com;
ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777;
peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org;
gre...@linuxfoundation.org
Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform
device
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
struct device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
Again, I think the wildcard match should be orthogonal to don't bind by
default as far as the mechanism goes.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
*only* through sysfs. Greg KH was skeptical and wanted to see a patch
before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
It would be something like this, right?
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..c9a61ea 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv, void
*data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv, dev))
return 0;
if (dev-parent)/* Needed for USB */
diff --git a/include/linux/device.h b/include/linux/device.h
index 2a9d6ed..f2be980 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -236,6 +236,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool explicit_bind_only;/* enables bind/unbind via sysfs only */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
Stuart
N�r��yb�X��ǧv�^�)Þº{.n�+{zX����ܨ}ï¿½ï¿½ï¿½Æ z�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jÇ«y�m��@A�a���
0��h���i
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 12:16 -0700, gre...@linuxfoundation.org wrote:
On Wed, Oct 09, 2013 at 07:02:25PM +, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev, struct
device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
That was by design, nice to see I implemented it properly :)
I think something like the new flag in the snippet below would enable the
platform
bus to support platform drivers that only bind on explicit request:
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..daf6d24 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,7 @@ static ssize_t bind_store(struct device_driver *drv,
const char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL driver_match_device(drv, dev, 1))
{
Magic flags are the spawn of your favorite anti-$DIETY. I'm never going
to accept that, sorry.
If you really want to do something special for the platform bus, then
do it only for the platform bus. But even then, you'll find me arguing
that you really don't want to do it at all, sorry.
It's not (or shouldn't be) special for the platform bus. The don't
bind by default flag (note that I am *not* referring to the above code
snippet) would be useful for VFIO PCI as well, as it would replace the
hacky and racy usage of new_id.
I'm still yet to be convinced this is even an issue at all, but maybe
that's just the jetlag talking...
If this is because you think we should use new_id, we just had a
discussion in this thread about why that's no good.
-Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
-Original Message-
From: Wood Scott-B07421
Sent: Wednesday, October 09, 2013 2:22 PM
To: Yoder Stuart-B08248
Cc: Wood Scott-B07421; Kim Phillips; Christoffer Dall; Alex Williamson;
linux-kernel@vger.kernel.org; a.mota...@virtualopensystems.com;
ag...@suse.de; Sethi Varun-B16395; Bhushan Bharat-R65777;
peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org;
gre...@linuxfoundation.org
Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform
device
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
'wildcard' matching for a driver can be fairly done in the platform
bus driver. We could add a new flag to the platform driver struct:
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..4d6cf14 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -727,6 +727,10 @@ static int platform_match(struct device *dev,
struct device_driver *drv)
struct platform_device *pdev = to_platform_device(dev);
struct platform_driver *pdrv = to_platform_driver(drv);
+ /* the driver matches any device */
+ if (pdrv-match_any)
+ return 1;
+
/* Attempt an OF style match first */
if (of_driver_match_device(dev, drv))
return 1;
However, the more problematic issue is that a bus driver has no way to
differentiate from an explicit bind request via sysfs and a bind that
happened through bus probing.
Again, I think the wildcard match should be orthogonal to don't bind by
default as far as the mechanism goes.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
*only* through sysfs. Greg KH was skeptical and wanted to see a patch
before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
I've been meaning to since the previous round of discussion, but I've
been busy. Would someone else be able to test it in the context of
using it for VFIO?
It would be something like this, right?
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..c9a61ea 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv, dev))
return 0;
if (drv-explicit_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -450,7 +450,7 @@ static int __driver_attach(struct device *dev, void *data)
* is an error.
*/
- if (!driver_match_device(drv, dev))
+ if (!drv-explicit_bind_only !driver_match_device(drv, dev))
return 0;
Likewise -- or error out earlier in driver_attach().
Otherwise, that looks about right, for the driver side (though
driver_attach could error out earlier rather than testing it inside the
loop).
The other half of fixing the raciness is to ensure that the device
doesn't get bound back to a non-VFIO driver (e.g. due to a module load
or new_id). The solution I proposed for that was a similar
explicit-bind-only flag for a device, that the user sets through sysfs
prior to unbinding. This would also be useful in non-VFIO contexts to
simply say I don't want to use this device at all.
-Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 9 Oct 2013 15:03:19 -0500
Scott Wood scottw...@freescale.com wrote:
On Wed, 2013-10-09 at 14:44 -0500, Yoder Stuart-B08248 wrote:
From: Wood Scott-B07421
Sent: Wednesday, October 09, 2013 2:22 PM
On Wed, 2013-10-09 at 14:02 -0500, Yoder Stuart-B08248 wrote:
Have been thinking about this issue some more. As Scott mentioned,
thanks for bringing this up again.
There's already a bool suppress_bind_attrs to prevent sysfs
bind/unbind. I suggested a similar flag to mean the oppsosite -- bind
*only* through sysfs. Greg KH was skeptical and wanted to see a patch
before any further discussion.
Ah, think I understand now...yes that works as well, and would be
less intrustive. So are you writing a patch? :)
I've been meaning to since the previous round of discussion, but I've
been busy. Would someone else be able to test it in the context of
using it for VFIO?
yes - see below.
Otherwise, that looks about right, for the driver side (though
driver_attach could error out earlier rather than testing it inside the
loop).
I've made the changes you suggested and tested the resulting diff below
on an arndale board. I successfully performed the following sequence of
commands after first changing the i2c@12C8 node in the device tree
to be exclusively compatible with vfio:
===
# ls -l /sys/bus/platform/drivers/vfio-platform/
total 0
--w--- 1 root root 4096 Sep 24 19:17 bind
--w--- 1 root root 4096 Sep 24 19:13 uevent
--w--- 1 root root 4096 Sep 24 19:18 unbind
# ls -l /sys/bus/platform/drivers/s3c-i2c
total 0
lrwxrwxrwx 1 root root0 Sep 24 19:11 12c6.i2c -
../../../../devices/12c6.i2c
lrwxrwxrwx 1 root root0 Sep 24 19:11 12c9.i2c -
../../../../devices/12c9.i2c
lrwxrwxrwx 1 root root0 Sep 24 19:20 12ce.i2c -
../../../../devices/12ce.i2c
--w--- 1 root root 4096 Sep 24 19:18 bind
--w--- 1 root root 4096 Sep 24 19:11 uevent
--w--- 1 root root 4096 Sep 24 19:17 unbind
# ls -l /sys/devices/12c8.i2c/driver # this is the one with the 'vfio'
compatible
ls: cannot access /sys/devices/12c8.i2c/driver: No such file or directory
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:18 /sys/devices/12ce.i2c/driver -
../../bus/platform/drivers/s3c-i2c
# echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/unbind
# ls -l /sys/devices/12ce.i2c/driver
ls: cannot access /sys/devices/12ce.i2c/driver: No such file or directory
# echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver -
../../bus/platform/drivers/vfio-platform
# echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/unbind
# ls -l /sys/devices/12ce.i2c/driver
# echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind
[ 722.137524] s3c-i2c 12ce.i2c: slave address 0x38
[ 722.141037] s3c-i2c 12ce.i2c: bus frequency set to 65 KHz
[ 722.150605] s3c-i2c 12ce.i2c: i2c-8: S3C I2C adapter
# ls -l /sys/devices/12ce.i2c/driver
lrwxrwxrwx 1 root root 0 Sep 24 19:21 /sys/devices/12ce.i2c/driver -
../../bus/platform/drivers/s3c-i2c
#
so it's correctly not allowing 'vfio' driver to bind to a device tree
compatible it's declared, and it then can bind the i2c @ 12ce device
to the vfio-platform driver, and unbind and bind it back to the i2c
driver.
For clarity's sake, before this diff, the command:
echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind
would error with:
echo: write error: No such device
The other half of fixing the raciness is to ensure that the device
doesn't get bound back to a non-VFIO driver (e.g. due to a module load
or new_id). The solution I proposed for that was a similar
explicit-bind-only flag for a device, that the user sets through sysfs
prior to unbinding. This would also be useful in non-VFIO contexts to
simply say I don't want to use this device at all.
I can take a look at doing this if you're still busy.
Thanks,
Kim
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 73f6c29..da81442 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL (drv-sysfs_bind_only ||
+ driver_match_device(drv, dev))) {
if (dev-parent)/* Needed for USB */
device_lock(dev-parent);
device_lock(dev);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 35fa368..6f85279 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv, void
*data)
{
struct device *dev
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, Oct 03, 2013 at 02:11:34PM -0500, Scott Wood wrote: > On Thu, 2013-10-03 at 11:54 -0700, gre...@linuxfoundation.org wrote: > > On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: > > > What it looks like we do still want from the driver core is the ability > > > for a driver to say that it should not be bound to a device except via > > > explicit sysfs bind, > > > > You can do that today by not providing any device ids in your driver > > structure, relying on the dynamic ids the driver core creates. > > I'm not sure what you mean by dynamic ids, The "new_id" file in sysfs for a driver. > but if the driver doesn't provide any match data, then > driver_match_device() will return 0 and bind_store() will fail. bind/unbind in sysfs can override this, I think, maybe it needs to be combined with the new_id file to work properly, it's been a long time since I last looked at that code path. > > > and the ability for a user to say that a device should not be bound to > > > a driver except via explicit sysfs bind. > > > > That's not going to happen, as how can the kernel know a specific device > > is going to want this, before it asks the drivers about it? > > This would normally be set by the user prior to unbinding from a > different driver, though it would also be nice to be able to set it at > boot time (e.g. via the kernel command line). Do that for your driver then, if you really want this, but it's not going into the driver core, sorry. It should never be parsing kernel command lines, nor should you. > > Or, just don't ever create a driver that matches that device, then rely > > on userspace to do the binding explicitly. > > That breaks the normal use case where you want the device to be bound to > the normal driver without doing anything special. And again, > driver_match_device() will cause the bind to fail. So, you are asking for something that really is impossible from what I can tell. Please figure out _exactly_ the semantics of what you want, as I'm totally confused and am giving up on this thread without seeing a patch anywhere. greg "back to patch review" k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-03 at 11:54 -0700, gre...@linuxfoundation.org wrote: > On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: > > What it looks like we do still want from the driver core is the ability > > for a driver to say that it should not be bound to a device except via > > explicit sysfs bind, > > You can do that today by not providing any device ids in your driver > structure, relying on the dynamic ids the driver core creates. I'm not sure what you mean by dynamic ids, but if the driver doesn't provide any match data, then driver_match_device() will return 0 and bind_store() will fail. > > and the ability for a user to say that a device should not be bound to > > a driver except via explicit sysfs bind. > > That's not going to happen, as how can the kernel know a specific device > is going to want this, before it asks the drivers about it? This would normally be set by the user prior to unbinding from a different driver, though it would also be nice to be able to set it at boot time (e.g. via the kernel command line). > Or, just don't ever create a driver that matches that device, then rely > on userspace to do the binding explicitly. That breaks the normal use case where you want the device to be bound to the normal driver without doing anything special. And again, driver_match_device() will cause the bind to fail. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: > What it looks like we do still want from the driver core is the ability > for a driver to say that it should not be bound to a device except via > explicit sysfs bind, You can do that today by not providing any device ids in your driver structure, relying on the dynamic ids the driver core creates. > and the ability for a user to say that a device should not be bound to > a driver except via explicit sysfs bind. That's not going to happen, as how can the kernel know a specific device is going to want this, before it asks the drivers about it? Or, just don't ever create a driver that matches that device, then rely on userspace to do the binding explicitly. Either way, no driver core changes are needed from what I can tell. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 16:40 -0700, gre...@linuxfoundation.org wrote: > On Wed, Oct 02, 2013 at 04:35:15PM -0500, Scott Wood wrote: > > On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: > > > On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: > > > > I don't see any equivalent functionality to PCI_ANY_ID for platform > > > > devices. > > > > > > Nor should it. If you are wanting to bind platform devices to different > > > things based on "ids" or "strings" or something else, then you had > > > better not be using a platform device because that is not what you have > > > anymore. > > > > I don't see how anything could be considered a platform device under > > your definition. > > Devices that you just "know" are at a specific memory location ahead of > time. > > > Even before all the device tree stuff came along, > > platform devices were still bound based on strings. > > Not all of them, there are lots that are not, look at ISA devices on a > PC platform for one example (your pc speaker, keyboard controller, > etc.) Using platform devices to let board files provide this information to driver files was a big improvement over hacking up the drivers directly to know about all sorts of different boards/SoCs. Once you split that knowledge into a different place you need a way of matching the two. BTW, this is done with the pc speaker as far as I can tell -- arch/x86/kernel/pcspeaker.c registers a device using the string "pcspkr" (as do some non-PC platforms). > > And even if we did still have a separate OF platform bus, my point about > > there not being a wildcard match applies to of_device_id as well. It > > certainly is not the case that "this is already there, and has been for > > years with no problems". > > That's an OF problem then, feel free to fix it there, but not in the > driver core with a crazy "ignore this bus type string" hack :) Even if we did this for OF and ACPI, you'd still have a problem if you want to use VFIO with a platform device that only has plain old platform data; thus, the platform bus (not driver core) seems to be the appropriate place for a wildcard match if we end up needing it. It may be moot though, since for platform devices we may want explicit kernel support for a device even with vfio, in order to reset/quiesce it between users. What it looks like we do still want from the driver core is the ability for a driver to say that it should not be bound to a device except via explicit sysfs bind, and the ability for a user to say that a device should not be bound to a driver except via explicit sysfs bind. This is a separate issue from making driver_match_device() happy (in some earlier e-mails in the thread these two issues were not properly separated). -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 16:40 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 04:35:15PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: I don't see any equivalent functionality to PCI_ANY_ID for platform devices. Nor should it. If you are wanting to bind platform devices to different things based on ids or strings or something else, then you had better not be using a platform device because that is not what you have anymore. I don't see how anything could be considered a platform device under your definition. Devices that you just know are at a specific memory location ahead of time. Even before all the device tree stuff came along, platform devices were still bound based on strings. Not all of them, there are lots that are not, look at ISA devices on a PC platform for one example (your pc speaker, keyboard controller, etc.) Using platform devices to let board files provide this information to driver files was a big improvement over hacking up the drivers directly to know about all sorts of different boards/SoCs. Once you split that knowledge into a different place you need a way of matching the two. BTW, this is done with the pc speaker as far as I can tell -- arch/x86/kernel/pcspeaker.c registers a device using the string pcspkr (as do some non-PC platforms). And even if we did still have a separate OF platform bus, my point about there not being a wildcard match applies to of_device_id as well. It certainly is not the case that this is already there, and has been for years with no problems. That's an OF problem then, feel free to fix it there, but not in the driver core with a crazy ignore this bus type string hack :) Even if we did this for OF and ACPI, you'd still have a problem if you want to use VFIO with a platform device that only has plain old platform data; thus, the platform bus (not driver core) seems to be the appropriate place for a wildcard match if we end up needing it. It may be moot though, since for platform devices we may want explicit kernel support for a device even with vfio, in order to reset/quiesce it between users. What it looks like we do still want from the driver core is the ability for a driver to say that it should not be bound to a device except via explicit sysfs bind, and the ability for a user to say that a device should not be bound to a driver except via explicit sysfs bind. This is a separate issue from making driver_match_device() happy (in some earlier e-mails in the thread these two issues were not properly separated). -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: What it looks like we do still want from the driver core is the ability for a driver to say that it should not be bound to a device except via explicit sysfs bind, You can do that today by not providing any device ids in your driver structure, relying on the dynamic ids the driver core creates. and the ability for a user to say that a device should not be bound to a driver except via explicit sysfs bind. That's not going to happen, as how can the kernel know a specific device is going to want this, before it asks the drivers about it? Or, just don't ever create a driver that matches that device, then rely on userspace to do the binding explicitly. Either way, no driver core changes are needed from what I can tell. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, 2013-10-03 at 11:54 -0700, gre...@linuxfoundation.org wrote: On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: What it looks like we do still want from the driver core is the ability for a driver to say that it should not be bound to a device except via explicit sysfs bind, You can do that today by not providing any device ids in your driver structure, relying on the dynamic ids the driver core creates. I'm not sure what you mean by dynamic ids, but if the driver doesn't provide any match data, then driver_match_device() will return 0 and bind_store() will fail. and the ability for a user to say that a device should not be bound to a driver except via explicit sysfs bind. That's not going to happen, as how can the kernel know a specific device is going to want this, before it asks the drivers about it? This would normally be set by the user prior to unbinding from a different driver, though it would also be nice to be able to set it at boot time (e.g. via the kernel command line). Or, just don't ever create a driver that matches that device, then rely on userspace to do the binding explicitly. That breaks the normal use case where you want the device to be bound to the normal driver without doing anything special. And again, driver_match_device() will cause the bind to fail. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Thu, Oct 03, 2013 at 02:11:34PM -0500, Scott Wood wrote: On Thu, 2013-10-03 at 11:54 -0700, gre...@linuxfoundation.org wrote: On Thu, Oct 03, 2013 at 01:33:27PM -0500, Scott Wood wrote: What it looks like we do still want from the driver core is the ability for a driver to say that it should not be bound to a device except via explicit sysfs bind, You can do that today by not providing any device ids in your driver structure, relying on the dynamic ids the driver core creates. I'm not sure what you mean by dynamic ids, The new_id file in sysfs for a driver. but if the driver doesn't provide any match data, then driver_match_device() will return 0 and bind_store() will fail. bind/unbind in sysfs can override this, I think, maybe it needs to be combined with the new_id file to work properly, it's been a long time since I last looked at that code path. and the ability for a user to say that a device should not be bound to a driver except via explicit sysfs bind. That's not going to happen, as how can the kernel know a specific device is going to want this, before it asks the drivers about it? This would normally be set by the user prior to unbinding from a different driver, though it would also be nice to be able to set it at boot time (e.g. via the kernel command line). Do that for your driver then, if you really want this, but it's not going into the driver core, sorry. It should never be parsing kernel command lines, nor should you. Or, just don't ever create a driver that matches that device, then rely on userspace to do the binding explicitly. That breaks the normal use case where you want the device to be bound to the normal driver without doing anything special. And again, driver_match_device() will cause the bind to fail. So, you are asking for something that really is impossible from what I can tell. Please figure out _exactly_ the semantics of what you want, as I'm totally confused and am giving up on this thread without seeing a patch anywhere. greg back to patch review k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 04:35:15PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: > > On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: > > > On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: > > > > On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > > > > > What's wrong with a non-vfio-specific flag that a driver can set, > > > > > > that > > > > > > indicates that the driver is willing to try to bind to any device > > > > > > on the > > > > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > > > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > > > > very specific case. What you're suggesting would let users specify > > > > > that > > > > > a serial driver should handle a NIC hardware, no? That sounds much > > > > > much > > > > > worse to me. > > > > > > > > You can do that today, with any PCI driver (or USB driver as well), just > > > > use the bind/unbind files in sysfs and you had better "know" what you > > > > are doing... > > > > > > sysfs bind won't work if it driver_match_device() fails. PCI has > > > PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver > > > should not bind to a device except when explicitly requested via sysfs > > > bind. > > > > > > I don't see any equivalent functionality to PCI_ANY_ID for platform > > > devices. > > > > Nor should it. If you are wanting to bind platform devices to different > > things based on "ids" or "strings" or something else, then you had > > better not be using a platform device because that is not what you have > > anymore. > > I don't see how anything could be considered a platform device under > your definition. Devices that you just "know" are at a specific memory location ahead of time. > Even before all the device tree stuff came along, > platform devices were still bound based on strings. Not all of them, there are lots that are not, look at ISA devices on a PC platform for one example (your pc speaker, keyboard controller, etc.) > > Yes, I know the OF stuff uses platform devices, and again, it's one > > reason why I don't like it at all. So fix OF devices "properly", > > creating your own bus and device type, and then you will not have these > > issues. > > That's what we used to have... It was merged with platform bus because > so many devices may be probed multiple different ways (device tree, > platform data, ACPI, etc). And I still say that was a mistake I should have never let happen. I think you should handle binding devices to multiple busses in the driver code for the different drivers, like we already do today for lots of different devices (USB host controllers being one example.) > OF is not a bus. It's a way to describe the device tree to the kernel, and as such, it's a "bus" as far as the driver model is concerned. Lots of things are "busses" for the driver core that you wouldn't think of as a "bus". A better way to think of busses in the driver core is as a "subsystem". In fact, we want to change busses to "subsystem" one of these days, udev has supported that for over 5 years now for when we eventually get around to it... > A platform device discovered from OF is still a platform device, just > as an i2c device discovered from OF is still an i2c device. Devices don't change what they are just because of what "subsystem" they are created from. Again, look at USB host controllers as an example of that. > If you don't like devices that don't sit on some formalized bus and can > be described in more than one way, fine, but that won't make them go > away. Think of "subsystem" instead, that should make more sense. > And even if we did still have a separate OF platform bus, my point about > there not being a wildcard match applies to of_device_id as well. It > certainly is not the case that "this is already there, and has been for > years with no problems". That's an OF problem then, feel free to fix it there, but not in the driver core with a crazy "ignore this bus type string" hack :) > > greg "I should never have let platform devices be created" k-h > > The alternative is what? A bunch of duplicated code, with a different > bus type for every SoC family, just so you can put a name on it? The amount of duplicated code should be really small. If it's too large, let me know and I'll make driver core helpers for it. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: > On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: > > On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: > > > On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > > > indicates that the driver is willing to try to bind to any device on > > > > > the > > > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > > > very specific case. What you're suggesting would let users specify that > > > > a serial driver should handle a NIC hardware, no? That sounds much much > > > > worse to me. > > > > > > You can do that today, with any PCI driver (or USB driver as well), just > > > use the bind/unbind files in sysfs and you had better "know" what you > > > are doing... > > > > sysfs bind won't work if it driver_match_device() fails. PCI has > > PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver > > should not bind to a device except when explicitly requested via sysfs > > bind. > > > > I don't see any equivalent functionality to PCI_ANY_ID for platform > > devices. > > Nor should it. If you are wanting to bind platform devices to different > things based on "ids" or "strings" or something else, then you had > better not be using a platform device because that is not what you have > anymore. I don't see how anything could be considered a platform device under your definition. Even before all the device tree stuff came along, platform devices were still bound based on strings. > Yes, I know the OF stuff uses platform devices, and again, it's one > reason why I don't like it at all. So fix OF devices "properly", > creating your own bus and device type, and then you will not have these > issues. That's what we used to have... It was merged with platform bus because so many devices may be probed multiple different ways (device tree, platform data, ACPI, etc). OF is not a bus. A platform device discovered from OF is still a platform device, just as an i2c device discovered from OF is still an i2c device. If you don't like devices that don't sit on some formalized bus and can be described in more than one way, fine, but that won't make them go away. And even if we did still have a separate OF platform bus, my point about there not being a wildcard match applies to of_device_id as well. It certainly is not the case that "this is already there, and has been for years with no problems". > greg "I should never have let platform devices be created" k-h The alternative is what? A bunch of duplicated code, with a different bus type for every SoC family, just so you can put a name on it? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: > > On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > > indicates that the driver is willing to try to bind to any device on the > > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > > very specific case. What you're suggesting would let users specify that > > > a serial driver should handle a NIC hardware, no? That sounds much much > > > worse to me. > > > > You can do that today, with any PCI driver (or USB driver as well), just > > use the bind/unbind files in sysfs and you had better "know" what you > > are doing... > > sysfs bind won't work if it driver_match_device() fails. PCI has > PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver > should not bind to a device except when explicitly requested via sysfs > bind. > > I don't see any equivalent functionality to PCI_ANY_ID for platform > devices. Nor should it. If you are wanting to bind platform devices to different things based on "ids" or "strings" or something else, then you had better not be using a platform device because that is not what you have anymore. Yes, I know the OF stuff uses platform devices, and again, it's one reason why I don't like it at all. So fix OF devices "properly", creating your own bus and device type, and then you will not have these issues. thanks, greg "I should never have let platform devices be created" k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: > On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > indicates that the driver is willing to try to bind to any device on the > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > very specific case. What you're suggesting would let users specify that > > a serial driver should handle a NIC hardware, no? That sounds much much > > worse to me. > > You can do that today, with any PCI driver (or USB driver as well), just > use the bind/unbind files in sysfs and you had better "know" what you > are doing... sysfs bind won't work if it driver_match_device() fails. PCI has PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver should not bind to a device except when explicitly requested via sysfs bind. I don't see any equivalent functionality to PCI_ANY_ID for platform devices. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:39:43PM -0700, gre...@linuxfoundation.org wrote: > On Wed, Oct 02, 2013 at 09:27:38PM +0100, Christoffer Dall wrote: > > > > What you're suggesting would let users specify that > > > > a serial driver should handle a NIC hardware, no? That sounds much much > > > > worse to me. > > > > > > The flag (and wildcard match, if applicable) would be set by the driver, > > > not by the user. Whereas PCI's "new_id" and the "new_compatible" being > > > suggested in this thread would allow exactly that, assuming the driver > > > doesn't reject the device in the probe function. > > > > > ok, fair enough, but still, I don't see the _generic_ need for having a > > kernel feature that allows some random driver to bind to a device, but > > then again, I'm not an authority in this area. > > Again, this is already there, and has been for years with no problems, > so I really don't understand the issue you have with it. > As I said, I'm not an authority, just sounded to me like we were trying to build something very generic to solve a very specific case, but I certainly don't want to invent problems that don't exist. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:37:35PM -0700, gre...@linuxfoundation.org wrote: > On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > indicates that the driver is willing to try to bind to any device on the > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > very specific case. What you're suggesting would let users specify that > > a serial driver should handle a NIC hardware, no? That sounds much much > > worse to me. > > You can do that today, with any PCI driver (or USB driver as well), just > use the bind/unbind files in sysfs and you had better "know" what you > are doing... > OK, yikes, ignore my comment then. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 09:27:38PM +0100, Christoffer Dall wrote: > > > What you're suggesting would let users specify that > > > a serial driver should handle a NIC hardware, no? That sounds much much > > > worse to me. > > > > The flag (and wildcard match, if applicable) would be set by the driver, > > not by the user. Whereas PCI's "new_id" and the "new_compatible" being > > suggested in this thread would allow exactly that, assuming the driver > > doesn't reject the device in the probe function. > > > ok, fair enough, but still, I don't see the _generic_ need for having a > kernel feature that allows some random driver to bind to a device, but > then again, I'm not an authority in this area. Again, this is already there, and has been for years with no problems, so I really don't understand the issue you have with it. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: > > What's wrong with a non-vfio-specific flag that a driver can set, that > > indicates that the driver is willing to try to bind to any device on the > > bus if explicitly requested via the existing sysfs bind mechanism? > > > It sounds more hackish to me to invent some 'generic' flag to solve a > very specific case. What you're suggesting would let users specify that > a serial driver should handle a NIC hardware, no? That sounds much much > worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better "know" what you are doing... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 03:14:02PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 11:43 -0700, Christoffer Dall wrote: > > On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > > > On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > > > > > > > -Original Message- > > > > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > > To: Alex Williamson > > > > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > > > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; > > > > > ag...@suse.de; > > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > > > > k...@vger.kernel.org > > > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > > > > device > > > > > > > > > > Wouldn't a sysfs file to add compatibility strings to the > > > > > vfio-platform > > > > > driver make driver_match_device return true and make everyone happy? > > > > > > > > I had a similar thought. Why can't we do something like: > > > > > > > > echo "fsl,i2c" > > > > > /sys/bus/platform/drivers/vfio-platform/new_compatible > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > > > > > > > The first steps tell vfio-platform to register itself to handle > > > > "fsl,i2c" compatible devices. The second step does the bind. > > > > > > Needing to specify the compatible is hacky (we already know what device > > > we want to bind; why do we need to scrounge up more information than > > > that, and add a new sysfs interface for extending compatible matches, > > > and a more flexible data structure to back that up?), and is racy on > > > buses that can hotplug (which driver gets the new device?). > > > > Why hacky? It seems quite reasonable to me that the user has to tell a > > subsystem that from a certain point it should be capable of handling > > some device. > > But the reason that driver can handle the device has nothing to do with > the compatible -- it can handle any device on the bus (except for > limitations checked for in the probe function), but it's a policy > decision whether we want it to handle any particular device (as opposed > to a particular type of device). > > Now, if we end up requiring a device-aware kernel stub for VFIO platform > devices (as was discussed for handling reset and such), we won't want a > wildcard match, but we'd still want to say that devices only get bound > to vfio upon explicit request. We also would not want userspace adding > to vfio's compatible list in that case. So perhaps a flag for "only > bind on explicit request" should be separate from the ability to supply > a wildcard match. VFIO PCI could still use the wildcard match. > I don't disagree on your observations here, the question is only how it fits with the existing driver/device/bus code. I just don't think having a series of flag and having to test all sorts of combination of those flags to see if any driver can bind to some device sounds very nice, if we always have the case that, either: (1) There's one and only one driver for a device (2) There's the driver itself, and now the user asked for VFIO to bind to a device as well. If we need something more flexible overall for the binding, then yes, some set of appropriate flags is probably a good idea, but if we're only trying to solve (2), it seems better to me to keep changes as isolated to VFIO as possible. > > As for the data structure, isn't this a simple linked list? > > It could be, but currently it's an array, which is what all the drivers > are expecting to provide. Adding a second parallel mechanism (like PCI > dynid) seems excessive compared to adding a wildcard match (on PCI such > a mechanism happened to already exist, which made it easy to use for > this, even if it wasn't necessarily the best approach). And then what > happens on non-device-tree platform devices? > > > The problem with the race seems to be a common problem that hasn't even > > been solved for PCI yet, so I'm wondering if this is not an orthogonal > > issue with a separate solution, such as a priority or something like > > that. > > > > Yes, once you've added the new_compatible to the vfio-platform driver, > > it's up for g
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 21:13 +0100, Christoffer Dall wrote: > On Wed, Oct 02, 2013 at 03:04:15PM -0500, Kim Phillips wrote: > > On Wed, 2 Oct 2013 11:43:30 -0700 > > Christoffer Dall wrote: > > > > > On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > > indicates that the driver is willing to try to bind to any device on the > > > > bus if explicitly requested via the existing sysfs bind mechanism? > > > > > > > It sounds more hackish to me to invent some 'generic' flag to solve a > > > very specific case. What you're suggesting would let users specify that > > > a serial driver should handle a NIC hardware, no? That sounds much much > > > worse to me. > > > > I thought that was the nature of VFIO drivers...it's a 'meta-' driver, > > used for enabling userspace drivers at large. > > > Yes, vfio is a meta driver, therefore it needs to be able to do > something special, but the generic driver/device/bus matching framework > doesn't need an extra generic feature allowing you to bind driver X to > device Y for all combinations of X and Y depending on some flag... Not all combinations of X and Y. Only instances of X that advertise that this is OK. > Someone please correct me if there are more use cases for this and this > is in fact worth a generic solution. Note that the wildcard match that I suggested in the e-mail I just sent would likely be implemented by the bus match code -- not by generic driver model code. It would still be less intrusive than implementing a dynamic match mechanism for each bus type (and for device tree, ACPI, etc in the case of platform bus). -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 11:43 -0700, Christoffer Dall wrote: > On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > > On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > > > > > -Original Message- > > > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > To: Alex Williamson > > > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > > > k...@vger.kernel.org > > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > > > device > > > > > > > > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > > > > driver make driver_match_device return true and make everyone happy? > > > > > > I had a similar thought. Why can't we do something like: > > > > > > echo "fsl,i2c" > /sys/bus/platform/drivers/vfio-platform/new_compatible > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > > > > > The first steps tell vfio-platform to register itself to handle > > > "fsl,i2c" compatible devices. The second step does the bind. > > > > Needing to specify the compatible is hacky (we already know what device > > we want to bind; why do we need to scrounge up more information than > > that, and add a new sysfs interface for extending compatible matches, > > and a more flexible data structure to back that up?), and is racy on > > buses that can hotplug (which driver gets the new device?). > > Why hacky? It seems quite reasonable to me that the user has to tell a > subsystem that from a certain point it should be capable of handling > some device. But the reason that driver can handle the device has nothing to do with the compatible -- it can handle any device on the bus (except for limitations checked for in the probe function), but it's a policy decision whether we want it to handle any particular device (as opposed to a particular type of device). Now, if we end up requiring a device-aware kernel stub for VFIO platform devices (as was discussed for handling reset and such), we won't want a wildcard match, but we'd still want to say that devices only get bound to vfio upon explicit request. We also would not want userspace adding to vfio's compatible list in that case. So perhaps a flag for "only bind on explicit request" should be separate from the ability to supply a wildcard match. VFIO PCI could still use the wildcard match. > As for the data structure, isn't this a simple linked list? It could be, but currently it's an array, which is what all the drivers are expecting to provide. Adding a second parallel mechanism (like PCI dynid) seems excessive compared to adding a wildcard match (on PCI such a mechanism happened to already exist, which made it easy to use for this, even if it wasn't necessarily the best approach). And then what happens on non-device-tree platform devices? > The problem with the race seems to be a common problem that hasn't even > been solved for PCI yet, so I'm wondering if this is not an orthogonal > issue with a separate solution, such as a priority or something like > that. > > Yes, once you've added the new_compatible to the vfio-platform driver, > it's up for grabs from both the new and the old driver, but that could > be solved by always making sure that the vfio-platform driver is checked > first. ...which is the opposite of what you want if a different device of the same type is plugged in after you add the device type ID to vfio. A "bind only by request" flag would avoid that problem. As for the possibility that the normal driver would claim it (maybe due to the bus being rescanned after a hotplug event?), that could be addressed with a mechanism to atomically unbind-and-rebind, or (perhaps easier) by making it so that once a device has been explicitly unbound, it can only be bound again by explicit request (which would also allow a user to say "I don't want to use this device at all" without it getting rebound later). Better still would be an independent "don't bind by default" flag that the user can set in sysfs (this is different from the driver's "don't bind by default" flag that is set by the driver), so that the action is reversible, and so a user can set this policy regardless of whether a driver has been loaded yet. > > What's wrong with a non-vfio-s
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 03:04:15PM -0500, Kim Phillips wrote: > On Wed, 2 Oct 2013 11:43:30 -0700 > Christoffer Dall wrote: > > > On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > > > On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > > > > > > > -Original Message- > > > > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > > To: Alex Williamson > > > > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > > > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; > > > > > ag...@suse.de; > > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > > > > k...@vger.kernel.org > > > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > > > > device > > > > > > > > > > Wouldn't a sysfs file to add compatibility strings to the > > > > > vfio-platform > > > > > driver make driver_match_device return true and make everyone happy? > > > > > > > > I had a similar thought. Why can't we do something like: > > > > > > > > echo "fsl,i2c" > > > > > /sys/bus/platform/drivers/vfio-platform/new_compatible > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > > > > > > > The first steps tell vfio-platform to register itself to handle > > > > "fsl,i2c" compatible devices. The second step does the bind. > > > > > > Needing to specify the compatible is hacky (we already know what device > > > we want to bind; why do we need to scrounge up more information than > > > that, and add a new sysfs interface for extending compatible matches, > > > and a more flexible data structure to back that up?), and is racy on > > > buses that can hotplug (which driver gets the new device?). > > > > Why hacky? It seems quite reasonable to me that the user has to tell a > > subsystem that from a certain point it should be capable of handling > > some device. > > I think what Scott is saying is that the first echo above is somewhat > redundant with the second: they're both talking to the vfio-platform > driver about an i2c device. > ok, fair enough, I didn't commit particularly to that interface, but having a special hook for checking a flag kind of like the strcmp hack you posted, just seems weird to me; it would be better if the vfio driver can add the bind string to the list of compatible devices it can bind to, and then use the generic bind mechanism in the kernel. But I'm honestly not familiar enough with the implementaiton specific bits of the syfs interface to argue how the changes are for one method vs. the other. > > As for the data structure, isn't this a simple linked list? > > > > The problem with the race seems to be a common problem that hasn't even > > been solved for PCI yet, so I'm wondering if this is not an orthogonal > > issue with a separate solution, such as a priority or something like > > that. > > I agree; adding 'direct'/'atomic' functionality to the existing bind > sysfs file, i.e., bind_store() logic to perform device_release_driver() > logic if dev->driver != NULL, all under the same device_lock() is an > independent problem from binding the VFIO platform driver to a platform > device. > > > Yes, once you've added the new_compatible to the vfio-platform driver, > > it's up for grabs from both the new and the old driver, but that could > > be solved by always making sure that the vfio-platform driver is checked > > first. > > not sure I understand the priority problem here - haven't looked at PCI > yet - but, given the above 'atomic bind' functionality described above, > the new and old driver wouldn't be requesting to bind to the same > device simultaneously, no? > AFAIU, the problem occurs with hotplug. If you add the compatibility string to a new driver and then hotplug a device with the string, then which driver gets the device? > > (I'm not familiar with these data structures, but I would imagine > > something like re-inserting the vfio-platform driver in the > > list/tree/... whenever adding a new_compatible value might possibly be > > one solution). > > > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > > indicates that the driver is willing to try t
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2 Oct 2013 11:43:30 -0700 Christoffer Dall wrote: > On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > > On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > > > > > -Original Message- > > > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > To: Alex Williamson > > > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > > > k...@vger.kernel.org > > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > > > device > > > > > > > > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > > > > driver make driver_match_device return true and make everyone happy? > > > > > > I had a similar thought. Why can't we do something like: > > > > > > echo "fsl,i2c" > /sys/bus/platform/drivers/vfio-platform/new_compatible > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > > > > > The first steps tell vfio-platform to register itself to handle > > > "fsl,i2c" compatible devices. The second step does the bind. > > > > Needing to specify the compatible is hacky (we already know what device > > we want to bind; why do we need to scrounge up more information than > > that, and add a new sysfs interface for extending compatible matches, > > and a more flexible data structure to back that up?), and is racy on > > buses that can hotplug (which driver gets the new device?). > > Why hacky? It seems quite reasonable to me that the user has to tell a > subsystem that from a certain point it should be capable of handling > some device. I think what Scott is saying is that the first echo above is somewhat redundant with the second: they're both talking to the vfio-platform driver about an i2c device. > As for the data structure, isn't this a simple linked list? > > The problem with the race seems to be a common problem that hasn't even > been solved for PCI yet, so I'm wondering if this is not an orthogonal > issue with a separate solution, such as a priority or something like > that. I agree; adding 'direct'/'atomic' functionality to the existing bind sysfs file, i.e., bind_store() logic to perform device_release_driver() logic if dev->driver != NULL, all under the same device_lock() is an independent problem from binding the VFIO platform driver to a platform device. > Yes, once you've added the new_compatible to the vfio-platform driver, > it's up for grabs from both the new and the old driver, but that could > be solved by always making sure that the vfio-platform driver is checked > first. not sure I understand the priority problem here - haven't looked at PCI yet - but, given the above 'atomic bind' functionality described above, the new and old driver wouldn't be requesting to bind to the same device simultaneously, no? > (I'm not familiar with these data structures, but I would imagine > something like re-inserting the vfio-platform driver in the > list/tree/... whenever adding a new_compatible value might possibly be > one solution). > > > What's wrong with a non-vfio-specific flag that a driver can set, that > > indicates that the driver is willing to try to bind to any device on the > > bus if explicitly requested via the existing sysfs bind mechanism? > > > It sounds more hackish to me to invent some 'generic' flag to solve a > very specific case. What you're suggesting would let users specify that > a serial driver should handle a NIC hardware, no? That sounds much much > worse to me. I thought that was the nature of VFIO drivers...it's a 'meta-' driver, used for enabling userspace drivers at large. Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > > > -Original Message- > > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > To: Alex Williamson > > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > > k...@vger.kernel.org > > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > > device > > > > > > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > > > driver make driver_match_device return true and make everyone happy? > > > > I had a similar thought. Why can't we do something like: > > > > echo "fsl,i2c" > /sys/bus/platform/drivers/vfio-platform/new_compatible > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > > > The first steps tell vfio-platform to register itself to handle > > "fsl,i2c" compatible devices. The second step does the bind. > > Needing to specify the compatible is hacky (we already know what device > we want to bind; why do we need to scrounge up more information than > that, and add a new sysfs interface for extending compatible matches, > and a more flexible data structure to back that up?), and is racy on > buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. As for the data structure, isn't this a simple linked list? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. (I'm not familiar with these data structures, but I would imagine something like re-inserting the vfio-platform driver in the list/tree/... whenever adding a new_compatible value might possibly be one solution). > > What's wrong with a non-vfio-specific flag that a driver can set, that > indicates that the driver is willing to try to bind to any device on the > bus if explicitly requested via the existing sysfs bind mechanism? > It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > -Original Message- > > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > > Sent: Wednesday, October 02, 2013 10:14 AM > > To: Alex Williamson > > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > > k...@vger.kernel.org > > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > > device > > > > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > > driver make driver_match_device return true and make everyone happy? > > I had a similar thought. Why can't we do something like: > > echo "fsl,i2c" > /sys/bus/platform/drivers/vfio-platform/new_compatible > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind > > The first steps tell vfio-platform to register itself to handle > "fsl,i2c" compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
> -Original Message- > From: Christoffer Dall [mailto:christoffer.d...@linaro.org] > Sent: Wednesday, October 02, 2013 10:14 AM > To: Alex Williamson > Cc: Kim Phillips; gre...@linuxfoundation.org; linux- > ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; > k...@vger.kernel.org > Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform > device > > On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: > > On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: > > > On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: > > > > On Tue, 1 Oct 2013 13:00:54 -0700 > > > > Greg Kroah-Hartman wrote: > > > > > > > > > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > > > > > Hi, > > > > > > > > > > > > Santosh and I are having a problem figuring out how to enable > binding > > > > > > (and re-binding) platform devices to a platform VFIO driver > (see > > > > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > > > > > > > Binding platform drivers currently depends on a string match in > the > > > > > > device node's compatible entry. On an arndale, one can > currently > > > > > > rebind the same device to the same driver like so: > > > > > > > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c- > i2c/12ce.i2c/driver/unbind > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > > > > > > > And one can bind it to the vfio-dt driver, as Antonis > instructs, by > > > > > > appending a 'vfio-dt' string to the device tree compatible > entry for > > > > > > the device. Then this would work: > > > > > > > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c- > i2c/12ce.i2c/driver/unbind > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > > > > > > > Consequently, the hack patch below [2] allows any platform > device to be > > > > > > bound to the vfio-dt driver, without making changes to the > device > > > > > > tree. It's a hack because I don't see having any driver name > specific > > > > > > code in drivers/base/bus.c being upstream acceptable. > > > > > > > > > > You are correct. > > > > > > > > > > What is wrong with just doing the above unbind/bind things > through > > > > > sysfs, that is what it is there for, right? > > > > > > > > The bind fails because the compatible string in the device tree > doesn't > > > > match that of the VFIO platform driver, so driver_match_device > always > > > > returns false. > > > > > > > It sounds like this is not going to be pretty almost no matter what > > > we'll end up doing: Inherently VFIO is going to bind to a device > without > > > the device tree entry for that device ever saying anything about > VFIO. > > > > > > How is this solved for PCI? Can we use some analogy from that work > to > > > construct the missing piece? > > > > PCI supports a dynamic ID table for driver/device matching, see > > pci_add_dynid(). The problem is that this gets a little sloppy for the > > period where you have multiple drivers that can claim the same device, > > especially in the presence of hotplug. Thus the desire to improve the > > situation with some kind of direct binding interface. Thanks, > > > So that's called on the vfio pci driver? > > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo "fsl,i2c" > /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c > /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle "fsl,i2c" compatible devices. The second step does the bind. Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 16:14 +0100, Christoffer Dall wrote: > On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: > > On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: > > > On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: > > > > On Tue, 1 Oct 2013 13:00:54 -0700 > > > > Greg Kroah-Hartman wrote: > > > > > > > > > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > > > > > Hi, > > > > > > > > > > > > Santosh and I are having a problem figuring out how to enable > > > > > > binding > > > > > > (and re-binding) platform devices to a platform VFIO driver (see > > > > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > > > > > > > Binding platform drivers currently depends on a string match in the > > > > > > device node's compatible entry. On an arndale, one can currently > > > > > > rebind the same device to the same driver like so: > > > > > > > > > > > > echo 12ce.i2c > > > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > > > > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > > > > > appending a 'vfio-dt' string to the device tree compatible entry for > > > > > > the device. Then this would work: > > > > > > > > > > > > echo 12ce.i2c > > > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > > > > > > > Consequently, the hack patch below [2] allows any platform device > > > > > > to be > > > > > > bound to the vfio-dt driver, without making changes to the device > > > > > > tree. It's a hack because I don't see having any driver name > > > > > > specific > > > > > > code in drivers/base/bus.c being upstream acceptable. > > > > > > > > > > You are correct. > > > > > > > > > > What is wrong with just doing the above unbind/bind things through > > > > > sysfs, that is what it is there for, right? > > > > > > > > The bind fails because the compatible string in the device tree doesn't > > > > match that of the VFIO platform driver, so driver_match_device always > > > > returns false. > > > > > > > It sounds like this is not going to be pretty almost no matter what > > > we'll end up doing: Inherently VFIO is going to bind to a device without > > > the device tree entry for that device ever saying anything about VFIO. > > > > > > How is this solved for PCI? Can we use some analogy from that work to > > > construct the missing piece? > > > > PCI supports a dynamic ID table for driver/device matching, see > > pci_add_dynid(). The problem is that this gets a little sloppy for the > > period where you have multiple drivers that can claim the same device, > > especially in the presence of hotplug. Thus the desire to improve the > > situation with some kind of direct binding interface. Thanks, > > > So that's called on the vfio pci driver? This happens at the PCI bus driver level, all PCI drivers support dynamic IDs with a sysfs entry for adding and removing them. vfio-pci starts with an empty ID table and all it sees are .probe() callbacks when the dynamic table is updated and a match is made. > Wouldn't a sysfs file to add compatibility strings to the vfio-platform > driver make driver_match_device return true and make everyone happy? Seems like it. Since you don't have a bus driver providing that infrastructure for you the driver would need to do it by itself. > There would be an issue of binding priority to solve, I guess similar to > the PCI problem, but then at least the two device types would share a > common orthogonal challenge. Perhaps some sort of "force_bind" sysfs entry created by the driver that can unbind the existing driver and skip the match code. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: > On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: > > On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: > > > On Tue, 1 Oct 2013 13:00:54 -0700 > > > Greg Kroah-Hartman wrote: > > > > > > > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > > > > Hi, > > > > > > > > > > Santosh and I are having a problem figuring out how to enable binding > > > > > (and re-binding) platform devices to a platform VFIO driver (see > > > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > > > > > Binding platform drivers currently depends on a string match in the > > > > > device node's compatible entry. On an arndale, one can currently > > > > > rebind the same device to the same driver like so: > > > > > > > > > > echo 12ce.i2c > > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > > > > appending a 'vfio-dt' string to the device tree compatible entry for > > > > > the device. Then this would work: > > > > > > > > > > echo 12ce.i2c > > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > > > > > Consequently, the hack patch below [2] allows any platform device to > > > > > be > > > > > bound to the vfio-dt driver, without making changes to the device > > > > > tree. It's a hack because I don't see having any driver name specific > > > > > code in drivers/base/bus.c being upstream acceptable. > > > > > > > > You are correct. > > > > > > > > What is wrong with just doing the above unbind/bind things through > > > > sysfs, that is what it is there for, right? > > > > > > The bind fails because the compatible string in the device tree doesn't > > > match that of the VFIO platform driver, so driver_match_device always > > > returns false. > > > > > It sounds like this is not going to be pretty almost no matter what > > we'll end up doing: Inherently VFIO is going to bind to a device without > > the device tree entry for that device ever saying anything about VFIO. > > > > How is this solved for PCI? Can we use some analogy from that work to > > construct the missing piece? > > PCI supports a dynamic ID table for driver/device matching, see > pci_add_dynid(). The problem is that this gets a little sloppy for the > period where you have multiple drivers that can claim the same device, > especially in the presence of hotplug. Thus the desire to improve the > situation with some kind of direct binding interface. Thanks, > So that's called on the vfio pci driver? Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? There would be an issue of binding priority to solve, I guess similar to the PCI problem, but then at least the two device types would share a common orthogonal challenge. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? PCI supports a dynamic ID table for driver/device matching, see pci_add_dynid(). The problem is that this gets a little sloppy for the period where you have multiple drivers that can claim the same device, especially in the presence of hotplug. Thus the desire to improve the situation with some kind of direct binding interface. Thanks, So that's called on the vfio pci driver? Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? There would be an issue of binding priority to solve, I guess similar to the PCI problem, but then at least the two device types would share a common orthogonal challenge. -Christoffer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 16:14 +0100, Christoffer Dall wrote: On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? PCI supports a dynamic ID table for driver/device matching, see pci_add_dynid(). The problem is that this gets a little sloppy for the period where you have multiple drivers that can claim the same device, especially in the presence of hotplug. Thus the desire to improve the situation with some kind of direct binding interface. Thanks, So that's called on the vfio pci driver? This happens at the PCI bus driver level, all PCI drivers support dynamic IDs with a sysfs entry for adding and removing them. vfio-pci starts with an empty ID table and all it sees are .probe() callbacks when the dynamic table is updated and a match is made. Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? Seems like it. Since you don't have a bus driver providing that infrastructure for you the driver would need to do it by itself. There would be an issue of binding priority to solve, I guess similar to the PCI problem, but then at least the two device types would share a common orthogonal challenge. Perhaps some sort of force_bind sysfs entry created by the driver that can unbind the existing driver and skip the match code. Thanks, Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: RFC: (re-)binding the VFIO platform driver to a platform device
-Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device On Tue, Oct 01, 2013 at 08:35:56PM -0600, Alex Williamson wrote: On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c- i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c- i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? PCI supports a dynamic ID table for driver/device matching, see pci_add_dynid(). The problem is that this gets a little sloppy for the period where you have multiple drivers that can claim the same device, especially in the presence of hotplug. Thus the desire to improve the situation with some kind of direct binding interface. Thanks, So that's called on the vfio pci driver? Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. As for the data structure, isn't this a simple linked list? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. (I'm not familiar with these data structures, but I would imagine something like re-inserting the vfio-platform driver in the list/tree/... whenever adding a new_compatible value might possibly be one solution). What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. -Christoffer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2 Oct 2013 11:43:30 -0700 Christoffer Dall christoffer.d...@linaro.org wrote: On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. I think what Scott is saying is that the first echo above is somewhat redundant with the second: they're both talking to the vfio-platform driver about an i2c device. As for the data structure, isn't this a simple linked list? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. I agree; adding 'direct'/'atomic' functionality to the existing bind sysfs file, i.e., bind_store() logic to perform device_release_driver() logic if dev-driver != NULL, all under the same device_lock() is an independent problem from binding the VFIO platform driver to a platform device. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. not sure I understand the priority problem here - haven't looked at PCI yet - but, given the above 'atomic bind' functionality described above, the new and old driver wouldn't be requesting to bind to the same device simultaneously, no? (I'm not familiar with these data structures, but I would imagine something like re-inserting the vfio-platform driver in the list/tree/... whenever adding a new_compatible value might possibly be one solution). What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. I thought that was the nature of VFIO drivers...it's a 'meta-' driver, used for enabling userspace drivers at large. Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 03:04:15PM -0500, Kim Phillips wrote: On Wed, 2 Oct 2013 11:43:30 -0700 Christoffer Dall christoffer.d...@linaro.org wrote: On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. I think what Scott is saying is that the first echo above is somewhat redundant with the second: they're both talking to the vfio-platform driver about an i2c device. ok, fair enough, I didn't commit particularly to that interface, but having a special hook for checking a flag kind of like the strcmp hack you posted, just seems weird to me; it would be better if the vfio driver can add the bind string to the list of compatible devices it can bind to, and then use the generic bind mechanism in the kernel. But I'm honestly not familiar enough with the implementaiton specific bits of the syfs interface to argue how the changes are for one method vs. the other. As for the data structure, isn't this a simple linked list? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. I agree; adding 'direct'/'atomic' functionality to the existing bind sysfs file, i.e., bind_store() logic to perform device_release_driver() logic if dev-driver != NULL, all under the same device_lock() is an independent problem from binding the VFIO platform driver to a platform device. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. not sure I understand the priority problem here - haven't looked at PCI yet - but, given the above 'atomic bind' functionality described above, the new and old driver wouldn't be requesting to bind to the same device simultaneously, no? AFAIU, the problem occurs with hotplug. If you add the compatibility string to a new driver and then hotplug a device with the string, then which driver gets the device? (I'm not familiar with these data structures, but I would imagine something like re-inserting the vfio-platform driver in the list/tree/... whenever adding a new_compatible value might possibly be one solution). What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. I thought that was the nature of VFIO drivers...it's a 'meta-' driver, used for enabling userspace drivers at large. Yes, vfio is a meta driver, therefore it needs to be able to do something special, but the generic driver/device/bus matching framework doesn't need an extra generic feature allowing you to bind driver X to device Y for all combinations of X and Y depending on some flag... Someone please correct me if there are more use cases for this and this is in fact worth a generic
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 11:43 -0700, Christoffer Dall wrote: On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. But the reason that driver can handle the device has nothing to do with the compatible -- it can handle any device on the bus (except for limitations checked for in the probe function), but it's a policy decision whether we want it to handle any particular device (as opposed to a particular type of device). Now, if we end up requiring a device-aware kernel stub for VFIO platform devices (as was discussed for handling reset and such), we won't want a wildcard match, but we'd still want to say that devices only get bound to vfio upon explicit request. We also would not want userspace adding to vfio's compatible list in that case. So perhaps a flag for only bind on explicit request should be separate from the ability to supply a wildcard match. VFIO PCI could still use the wildcard match. As for the data structure, isn't this a simple linked list? It could be, but currently it's an array, which is what all the drivers are expecting to provide. Adding a second parallel mechanism (like PCI dynid) seems excessive compared to adding a wildcard match (on PCI such a mechanism happened to already exist, which made it easy to use for this, even if it wasn't necessarily the best approach). And then what happens on non-device-tree platform devices? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. ...which is the opposite of what you want if a different device of the same type is plugged in after you add the device type ID to vfio. A bind only by request flag would avoid that problem. As for the possibility that the normal driver would claim it (maybe due to the bus being rescanned after a hotplug event?), that could be addressed with a mechanism to atomically unbind-and-rebind, or (perhaps easier) by making it so that once a device has been explicitly unbound, it can only be bound again by explicit request (which would also allow a user to say I don't want to use this device at all without it getting rebound later). Better still would be an independent don't bind by default flag that the user can set in sysfs (this is different from the driver's don't bind by default flag that is set by the driver), so that the action is reversible, and so a user can set this policy regardless of whether a driver has been loaded yet. What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. new_compatible would be to solve an even more specific case, but both new_compatible and a don't-bind-by-default flag could have applications elsewhere. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 21:13 +0100, Christoffer Dall wrote: On Wed, Oct 02, 2013 at 03:04:15PM -0500, Kim Phillips wrote: On Wed, 2 Oct 2013 11:43:30 -0700 Christoffer Dall christoffer.d...@linaro.org wrote: On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. I thought that was the nature of VFIO drivers...it's a 'meta-' driver, used for enabling userspace drivers at large. Yes, vfio is a meta driver, therefore it needs to be able to do something special, but the generic driver/device/bus matching framework doesn't need an extra generic feature allowing you to bind driver X to device Y for all combinations of X and Y depending on some flag... Not all combinations of X and Y. Only instances of X that advertise that this is OK. Someone please correct me if there are more use cases for this and this is in fact worth a generic solution. Note that the wildcard match that I suggested in the e-mail I just sent would likely be implemented by the bus match code -- not by generic driver model code. It would still be less intrusive than implementing a dynamic match mechanism for each bus type (and for device tree, ACPI, etc in the case of platform bus). -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 03:14:02PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 11:43 -0700, Christoffer Dall wrote: On Wed, Oct 02, 2013 at 01:32:38PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Christoffer Dall [mailto:christoffer.d...@linaro.org] Sent: Wednesday, October 02, 2013 10:14 AM To: Alex Williamson Cc: Kim Phillips; gre...@linuxfoundation.org; linux- ker...@vger.kernel.org; a.mota...@virtualopensystems.com; ag...@suse.de; Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan Bharat-R65777; peter.mayd...@linaro.org; santosh.shu...@linaro.org; k...@vger.kernel.org Subject: Re: RFC: (re-)binding the VFIO platform driver to a platform device Wouldn't a sysfs file to add compatibility strings to the vfio-platform driver make driver_match_device return true and make everyone happy? I had a similar thought. Why can't we do something like: echo fsl,i2c /sys/bus/platform/drivers/vfio-platform/new_compatible echo 12ce.i2c /sys/bus/platform/drivers/vfio-platform/bind The first steps tell vfio-platform to register itself to handle fsl,i2c compatible devices. The second step does the bind. Needing to specify the compatible is hacky (we already know what device we want to bind; why do we need to scrounge up more information than that, and add a new sysfs interface for extending compatible matches, and a more flexible data structure to back that up?), and is racy on buses that can hotplug (which driver gets the new device?). Why hacky? It seems quite reasonable to me that the user has to tell a subsystem that from a certain point it should be capable of handling some device. But the reason that driver can handle the device has nothing to do with the compatible -- it can handle any device on the bus (except for limitations checked for in the probe function), but it's a policy decision whether we want it to handle any particular device (as opposed to a particular type of device). Now, if we end up requiring a device-aware kernel stub for VFIO platform devices (as was discussed for handling reset and such), we won't want a wildcard match, but we'd still want to say that devices only get bound to vfio upon explicit request. We also would not want userspace adding to vfio's compatible list in that case. So perhaps a flag for only bind on explicit request should be separate from the ability to supply a wildcard match. VFIO PCI could still use the wildcard match. I don't disagree on your observations here, the question is only how it fits with the existing driver/device/bus code. I just don't think having a series of flag and having to test all sorts of combination of those flags to see if any driver can bind to some device sounds very nice, if we always have the case that, either: (1) There's one and only one driver for a device (2) There's the driver itself, and now the user asked for VFIO to bind to a device as well. If we need something more flexible overall for the binding, then yes, some set of appropriate flags is probably a good idea, but if we're only trying to solve (2), it seems better to me to keep changes as isolated to VFIO as possible. As for the data structure, isn't this a simple linked list? It could be, but currently it's an array, which is what all the drivers are expecting to provide. Adding a second parallel mechanism (like PCI dynid) seems excessive compared to adding a wildcard match (on PCI such a mechanism happened to already exist, which made it easy to use for this, even if it wasn't necessarily the best approach). And then what happens on non-device-tree platform devices? The problem with the race seems to be a common problem that hasn't even been solved for PCI yet, so I'm wondering if this is not an orthogonal issue with a separate solution, such as a priority or something like that. Yes, once you've added the new_compatible to the vfio-platform driver, it's up for grabs from both the new and the old driver, but that could be solved by always making sure that the vfio-platform driver is checked first. ...which is the opposite of what you want if a different device of the same type is plugged in after you add the device type ID to vfio. A bind only by request flag would avoid that problem. ok, then reverse the priority. As for the possibility that the normal driver would claim it (maybe due to the bus being rescanned after a hotplug event?), that could be addressed with a mechanism to atomically unbind-and-rebind, or (perhaps easier) by making it so that once a device has been explicitly unbound, it can only be bound again by explicit request (which would also allow a user to say I don't want to use this device at all without it getting rebound
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 09:27:38PM +0100, Christoffer Dall wrote: What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. The flag (and wildcard match, if applicable) would be set by the driver, not by the user. Whereas PCI's new_id and the new_compatible being suggested in this thread would allow exactly that, assuming the driver doesn't reject the device in the probe function. ok, fair enough, but still, I don't see the _generic_ need for having a kernel feature that allows some random driver to bind to a device, but then again, I'm not an authority in this area. Again, this is already there, and has been for years with no problems, so I really don't understand the issue you have with it. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:37:35PM -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... OK, yikes, ignore my comment then. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 01:39:43PM -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 09:27:38PM +0100, Christoffer Dall wrote: What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. The flag (and wildcard match, if applicable) would be set by the driver, not by the user. Whereas PCI's new_id and the new_compatible being suggested in this thread would allow exactly that, assuming the driver doesn't reject the device in the probe function. ok, fair enough, but still, I don't see the _generic_ need for having a kernel feature that allows some random driver to bind to a device, but then again, I'm not an authority in this area. Again, this is already there, and has been for years with no problems, so I really don't understand the issue you have with it. As I said, I'm not an authority, just sounded to me like we were trying to build something very generic to solve a very specific case, but I certainly don't want to invent problems that don't exist. -Christoffer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... sysfs bind won't work if it driver_match_device() fails. PCI has PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver should not bind to a device except when explicitly requested via sysfs bind. I don't see any equivalent functionality to PCI_ANY_ID for platform devices. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... sysfs bind won't work if it driver_match_device() fails. PCI has PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver should not bind to a device except when explicitly requested via sysfs bind. I don't see any equivalent functionality to PCI_ANY_ID for platform devices. Nor should it. If you are wanting to bind platform devices to different things based on ids or strings or something else, then you had better not be using a platform device because that is not what you have anymore. Yes, I know the OF stuff uses platform devices, and again, it's one reason why I don't like it at all. So fix OF devices properly, creating your own bus and device type, and then you will not have these issues. thanks, greg I should never have let platform devices be created k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... sysfs bind won't work if it driver_match_device() fails. PCI has PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver should not bind to a device except when explicitly requested via sysfs bind. I don't see any equivalent functionality to PCI_ANY_ID for platform devices. Nor should it. If you are wanting to bind platform devices to different things based on ids or strings or something else, then you had better not be using a platform device because that is not what you have anymore. I don't see how anything could be considered a platform device under your definition. Even before all the device tree stuff came along, platform devices were still bound based on strings. Yes, I know the OF stuff uses platform devices, and again, it's one reason why I don't like it at all. So fix OF devices properly, creating your own bus and device type, and then you will not have these issues. That's what we used to have... It was merged with platform bus because so many devices may be probed multiple different ways (device tree, platform data, ACPI, etc). OF is not a bus. A platform device discovered from OF is still a platform device, just as an i2c device discovered from OF is still an i2c device. If you don't like devices that don't sit on some formalized bus and can be described in more than one way, fine, but that won't make them go away. And even if we did still have a separate OF platform bus, my point about there not being a wildcard match applies to of_device_id as well. It certainly is not the case that this is already there, and has been for years with no problems. greg I should never have let platform devices be created k-h The alternative is what? A bunch of duplicated code, with a different bus type for every SoC family, just so you can put a name on it? -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, Oct 02, 2013 at 04:35:15PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 14:16 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: On Wed, 2013-10-02 at 13:37 -0700, gre...@linuxfoundation.org wrote: On Wed, Oct 02, 2013 at 11:43:30AM -0700, Christoffer Dall wrote: What's wrong with a non-vfio-specific flag that a driver can set, that indicates that the driver is willing to try to bind to any device on the bus if explicitly requested via the existing sysfs bind mechanism? It sounds more hackish to me to invent some 'generic' flag to solve a very specific case. What you're suggesting would let users specify that a serial driver should handle a NIC hardware, no? That sounds much much worse to me. You can do that today, with any PCI driver (or USB driver as well), just use the bind/unbind files in sysfs and you had better know what you are doing... sysfs bind won't work if it driver_match_device() fails. PCI has PCI_ANY_ID, so the missing piece for PCI is a way to say that the driver should not bind to a device except when explicitly requested via sysfs bind. I don't see any equivalent functionality to PCI_ANY_ID for platform devices. Nor should it. If you are wanting to bind platform devices to different things based on ids or strings or something else, then you had better not be using a platform device because that is not what you have anymore. I don't see how anything could be considered a platform device under your definition. Devices that you just know are at a specific memory location ahead of time. Even before all the device tree stuff came along, platform devices were still bound based on strings. Not all of them, there are lots that are not, look at ISA devices on a PC platform for one example (your pc speaker, keyboard controller, etc.) Yes, I know the OF stuff uses platform devices, and again, it's one reason why I don't like it at all. So fix OF devices properly, creating your own bus and device type, and then you will not have these issues. That's what we used to have... It was merged with platform bus because so many devices may be probed multiple different ways (device tree, platform data, ACPI, etc). And I still say that was a mistake I should have never let happen. I think you should handle binding devices to multiple busses in the driver code for the different drivers, like we already do today for lots of different devices (USB host controllers being one example.) OF is not a bus. It's a way to describe the device tree to the kernel, and as such, it's a bus as far as the driver model is concerned. Lots of things are busses for the driver core that you wouldn't think of as a bus. A better way to think of busses in the driver core is as a subsystem. In fact, we want to change busses to subsystem one of these days, udev has supported that for over 5 years now for when we eventually get around to it... A platform device discovered from OF is still a platform device, just as an i2c device discovered from OF is still an i2c device. Devices don't change what they are just because of what subsystem they are created from. Again, look at USB host controllers as an example of that. If you don't like devices that don't sit on some formalized bus and can be described in more than one way, fine, but that won't make them go away. Think of subsystem instead, that should make more sense. And even if we did still have a separate OF platform bus, my point about there not being a wildcard match applies to of_device_id as well. It certainly is not the case that this is already there, and has been for years with no problems. That's an OF problem then, feel free to fix it there, but not in the driver core with a crazy ignore this bus type string hack :) greg I should never have let platform devices be created k-h The alternative is what? A bunch of duplicated code, with a different bus type for every SoC family, just so you can put a name on it? The amount of duplicated code should be really small. If it's too large, let me know and I'll make driver core helpers for it. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: > On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: > > On Tue, 1 Oct 2013 13:00:54 -0700 > > Greg Kroah-Hartman wrote: > > > > > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > > > Hi, > > > > > > > > Santosh and I are having a problem figuring out how to enable binding > > > > (and re-binding) platform devices to a platform VFIO driver (see > > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > > > Binding platform drivers currently depends on a string match in the > > > > device node's compatible entry. On an arndale, one can currently > > > > rebind the same device to the same driver like so: > > > > > > > > echo 12ce.i2c > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > > > appending a 'vfio-dt' string to the device tree compatible entry for > > > > the device. Then this would work: > > > > > > > > echo 12ce.i2c > > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > > > Consequently, the hack patch below [2] allows any platform device to be > > > > bound to the vfio-dt driver, without making changes to the device > > > > tree. It's a hack because I don't see having any driver name specific > > > > code in drivers/base/bus.c being upstream acceptable. > > > > > > You are correct. > > > > > > What is wrong with just doing the above unbind/bind things through > > > sysfs, that is what it is there for, right? > > > > The bind fails because the compatible string in the device tree doesn't > > match that of the VFIO platform driver, so driver_match_device always > > returns false. > > > It sounds like this is not going to be pretty almost no matter what > we'll end up doing: Inherently VFIO is going to bind to a device without > the device tree entry for that device ever saying anything about VFIO. > > How is this solved for PCI? Can we use some analogy from that work to > construct the missing piece? PCI supports a dynamic ID table for driver/device matching, see pci_add_dynid(). The problem is that this gets a little sloppy for the period where you have multiple drivers that can claim the same device, especially in the presence of hotplug. Thus the desire to improve the situation with some kind of direct binding interface. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: > On Tue, 1 Oct 2013 13:00:54 -0700 > Greg Kroah-Hartman wrote: > > > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > > Hi, > > > > > > Santosh and I are having a problem figuring out how to enable binding > > > (and re-binding) platform devices to a platform VFIO driver (see > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > Binding platform drivers currently depends on a string match in the > > > device node's compatible entry. On an arndale, one can currently > > > rebind the same device to the same driver like so: > > > > > > echo 12ce.i2c > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > > appending a 'vfio-dt' string to the device tree compatible entry for > > > the device. Then this would work: > > > > > > echo 12ce.i2c > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > Consequently, the hack patch below [2] allows any platform device to be > > > bound to the vfio-dt driver, without making changes to the device > > > tree. It's a hack because I don't see having any driver name specific > > > code in drivers/base/bus.c being upstream acceptable. > > > > You are correct. > > > > What is wrong with just doing the above unbind/bind things through > > sysfs, that is what it is there for, right? > > The bind fails because the compatible string in the device tree doesn't > match that of the VFIO platform driver, so driver_match_device always > returns false. > It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 16:59 -0500, Kim Phillips wrote: > On Tue, 1 Oct 2013 14:15:38 -0500 > Scott Wood wrote: > > > I think the ideal interface would be if you could write the sysfs device > > name into the vfio bind file (or some new file in the same directory), > > and have it claim that device (preferably with an atomic unbind from the > > previous driver). > > ok. ...which apparently is what you are already doing (except for the atomic part). My recollection of how this works on PCI (via new_id) apparently kept me from reading it properly. :-P > > We shouldn't be messing around with compatible > > (either modifying it or telling VFIO which compatibles to look for) when > > we know the specific devices (not just type of devices) we want to bind. > > ok, but I still don't see how to get past driver_match_device()'s > refusal to allow bind a non-compatible driver (or one who's name isn't > in the compatible list). Probably something similar to your hack, except use a flag or some other neutral mechanism rather than a driver name. The flag could be something like "I'll try to bind to any device on this bus, but only if explicitly requested". -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman wrote: > On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > > Hi, > > > > Santosh and I are having a problem figuring out how to enable binding > > (and re-binding) platform devices to a platform VFIO driver (see > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > Binding platform drivers currently depends on a string match in the > > device node's compatible entry. On an arndale, one can currently > > rebind the same device to the same driver like so: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > appending a 'vfio-dt' string to the device tree compatible entry for > > the device. Then this would work: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > Consequently, the hack patch below [2] allows any platform device to be > > bound to the vfio-dt driver, without making changes to the device > > tree. It's a hack because I don't see having any driver name specific > > code in drivers/base/bus.c being upstream acceptable. > > You are correct. > > What is wrong with just doing the above unbind/bind things through > sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 14:17:16 -0500 Scott Wood wrote: > On Tue, 2013-10-01 at 14:15 -0500, Scott Wood wrote: > > On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: > > > Hi, > > > > > > Santosh and I are having a problem figuring out how to enable binding > > > (and re-binding) platform devices to a platform VFIO driver (see > > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > > > Binding platform drivers currently depends on a string match in the > > > device node's compatible entry. On an arndale, one can currently > > > rebind the same device to the same driver like so: > > > > > > echo 12ce.i2c > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > > appending a 'vfio-dt' string to the device tree compatible entry for > > > the device. Then this would work: > > > > > > echo 12ce.i2c > > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > > > Consequently, the hack patch below [2] allows any platform device to be > > > bound to the vfio-dt driver, without making changes to the device > > > tree. It's a hack because I don't see having any driver name specific > > > code in drivers/base/bus.c being upstream acceptable. > > > > Modifying the device tree is the worse part of this. > > I think I missed something. How do you reconcile "without making > changes to the device tree" with "appending a 'vfio-dt' string to the > device tree compatible entry"? one doesn't need to append 'vfio-dt' to the device tree compatibles if one uses the hack that makes bind_store ignore trying to driver_match_device() if the driver is 'vfio-dt'. Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 14:15:38 -0500 Scott Wood wrote: > On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: > > Hi, > > > > Santosh and I are having a problem figuring out how to enable binding > > (and re-binding) platform devices to a platform VFIO driver (see > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > Binding platform drivers currently depends on a string match in the > > device node's compatible entry. On an arndale, one can currently > > rebind the same device to the same driver like so: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > appending a 'vfio-dt' string to the device tree compatible entry for > > the device. Then this would work: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > Consequently, the hack patch below [2] allows any platform device to be > > bound to the vfio-dt driver, without making changes to the device > > tree. It's a hack because I don't see having any driver name specific > > code in drivers/base/bus.c being upstream acceptable. > > Modifying the device tree is the worse part of this. > > Is this part of your later suggestion to make compatible writeable after > boot, or are you talking about messing with the device tree before boot > (putting software config in the device tree, among other ickiness)? writeable after boot > > Alternately, device tree compatible entries may be made writeable after > > boot, e.g.: > > > > echo vfio-platform > /proc/device-tree/i2c\@12CE/compatible > > > > [note s/vfio-dt/vfio-platform/] > > > > but that would require the vfio-platform module be reloaded, thereby > > unbinding it from any existing devices it was bound to: we're > > seeking a more dynamic solution. > > Eww. > > Not to mention that the VFIO user might want to know what the compatible > was, well, technically the user would be able to get that info by reading compatible before writing it, and ideally write the original value back in addition to the new value. > or that we might later want to unbind from VFIO and rebind to the > kernel... I believe that's independent: it would depend on which driver's (VFIO, kernel, or other) sysfs file the device address gets written into. > > Alex Graf (cc'd) proposed an alternate approach: re-write the driver > > name in the device's sysfs entry: > > > > echo "vfio-platform" > > > /sys/bus/platform/devices/101e.rtc/driver/driver_name > > > > The advantage of this approach is that we can achieve the re-bind > > (unbind + bind) as an atomic operation, which alleviates userspace from > > having to coordinate with other device operations (I think VM migration > > is an example case here). > > > > Note that driver_name currently doesn't exist in sysfs, so it would > > either have to be added, or another means developed to rename the > > driver symlink itself: > > I think the ideal interface would be if you could write the sysfs device > name into the vfio bind file (or some new file in the same directory), > and have it claim that device (preferably with an atomic unbind from the > previous driver). ok. > We shouldn't be messing around with compatible > (either modifying it or telling VFIO which compatibles to look for) when > we know the specific devices (not just type of devices) we want to bind. ok, but I still don't see how to get past driver_match_device()'s refusal to allow bind a non-compatible driver (or one who's name isn't in the compatible list). Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: > Hi, > > Santosh and I are having a problem figuring out how to enable binding > (and re-binding) platform devices to a platform VFIO driver (see > Antonis' WIP: [1]) in an upstream-acceptable manner. > > Binding platform drivers currently depends on a string match in the > device node's compatible entry. On an arndale, one can currently > rebind the same device to the same driver like so: > > echo 12ce.i2c > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > appending a 'vfio-dt' string to the device tree compatible entry for > the device. Then this would work: > > echo 12ce.i2c > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > Consequently, the hack patch below [2] allows any platform device to be > bound to the vfio-dt driver, without making changes to the device > tree. It's a hack because I don't see having any driver name specific > code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 14:15 -0500, Scott Wood wrote: > On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: > > Hi, > > > > Santosh and I are having a problem figuring out how to enable binding > > (and re-binding) platform devices to a platform VFIO driver (see > > Antonis' WIP: [1]) in an upstream-acceptable manner. > > > > Binding platform drivers currently depends on a string match in the > > device node's compatible entry. On an arndale, one can currently > > rebind the same device to the same driver like so: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > > appending a 'vfio-dt' string to the device tree compatible entry for > > the device. Then this would work: > > > > echo 12ce.i2c > > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > > > Consequently, the hack patch below [2] allows any platform device to be > > bound to the vfio-dt driver, without making changes to the device > > tree. It's a hack because I don't see having any driver name specific > > code in drivers/base/bus.c being upstream acceptable. > > Modifying the device tree is the worse part of this. I think I missed something. How do you reconcile "without making changes to the device tree" with "appending a 'vfio-dt' string to the device tree compatible entry"? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: > Hi, > > Santosh and I are having a problem figuring out how to enable binding > (and re-binding) platform devices to a platform VFIO driver (see > Antonis' WIP: [1]) in an upstream-acceptable manner. > > Binding platform drivers currently depends on a string match in the > device node's compatible entry. On an arndale, one can currently > rebind the same device to the same driver like so: > > echo 12ce.i2c > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind > > And one can bind it to the vfio-dt driver, as Antonis instructs, by > appending a 'vfio-dt' string to the device tree compatible entry for > the device. Then this would work: > > echo 12ce.i2c > > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind > echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind > > Consequently, the hack patch below [2] allows any platform device to be > bound to the vfio-dt driver, without making changes to the device > tree. It's a hack because I don't see having any driver name specific > code in drivers/base/bus.c being upstream acceptable. Modifying the device tree is the worse part of this. Is this part of your later suggestion to make compatible writeable after boot, or are you talking about messing with the device tree before boot (putting software config in the device tree, among other ickiness)? > Alternately, device tree compatible entries may be made writeable after > boot, e.g.: > > echo vfio-platform > /proc/device-tree/i2c\@12CE/compatible > > [note s/vfio-dt/vfio-platform/] > > but that would require the vfio-platform module be reloaded, thereby > unbinding it from any existing devices it was bound to: we're > seeking a more dynamic solution. Eww. Not to mention that the VFIO user might want to know what the compatible was, or that we might later want to unbind from VFIO and rebind to the kernel... > Alex Graf (cc'd) proposed an alternate approach: re-write the driver > name in the device's sysfs entry: > > echo "vfio-platform" > > /sys/bus/platform/devices/101e.rtc/driver/driver_name > > The advantage of this approach is that we can achieve the re-bind > (unbind + bind) as an atomic operation, which alleviates userspace from > having to coordinate with other device operations (I think VM migration > is an example case here). > > Note that driver_name currently doesn't exist in sysfs, so it would > either have to be added, or another means developed to rename the > driver symlink itself: I think the ideal interface would be if you could write the sysfs device name into the vfio bind file (or some new file in the same directory), and have it claim that device (preferably with an atomic unbind from the previous driver). We shouldn't be messing around with compatible (either modifying it or telling VFIO which compatibles to look for) when we know the specific devices (not just type of devices) we want to bind. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RFC: (re-)binding the VFIO platform driver to a platform device
Hi,
Santosh and I are having a problem figuring out how to enable binding
(and re-binding) platform devices to a platform VFIO driver (see
Antonis' WIP: [1]) in an upstream-acceptable manner.
Binding platform drivers currently depends on a string match in the
device node's compatible entry. On an arndale, one can currently
rebind the same device to the same driver like so:
echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind
echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/bind
And one can bind it to the vfio-dt driver, as Antonis instructs, by
appending a 'vfio-dt' string to the device tree compatible entry for
the device. Then this would work:
echo 12ce.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind
echo 12ce.i2c > /sys/bus/platform/drivers/vfio-dt/bind
Consequently, the hack patch below [2] allows any platform device to be
bound to the vfio-dt driver, without making changes to the device
tree. It's a hack because I don't see having any driver name specific
code in drivers/base/bus.c being upstream acceptable.
Alternately, device tree compatible entries may be made writeable after
boot, e.g.:
echo vfio-platform > /proc/device-tree/i2c\@12CE/compatible
[note s/vfio-dt/vfio-platform/]
but that would require the vfio-platform module be reloaded, thereby
unbinding it from any existing devices it was bound to: we're
seeking a more dynamic solution.
Alex Graf (cc'd) proposed an alternate approach: re-write the driver
name in the device's sysfs entry:
echo "vfio-platform" > /sys/bus/platform/devices/101e.rtc/driver/driver_name
The advantage of this approach is that we can achieve the re-bind
(unbind + bind) as an atomic operation, which alleviates userspace from
having to coordinate with other device operations (I think VM migration
is an example case here).
Note that driver_name currently doesn't exist in sysfs, so it would
either have to be added, or another means developed to rename the
driver symlink itself:
cd /sys/bus/platform/devices/12ce.i2c
ln -s ../../bus/platform/drivers/s5p-ehci /tmp/tmp-link
mv -Tf /tmp/tmp-link driver
So I guess the question is: Is our understanding corret - are we on
the right track at all here? Is the hack below definitely not
acceptable? Is it correct to assume upstream maintainers are against
writing compatible entries to the device tree sysfs at runtime? Would
a driver_name be acceptable to add to sysfs, or should we investigate
something like the atomic mv command above further?
Thanks,
Kim
[1] Note that in this RFC, 'vfio-dt' is the name for the driver (-dt for
device tree) which has already been pointed out as a misnomer and
should probably be rewritten as 'vfio-platform':
http://lists.linux-foundation.org/pipermail/iommu/2013-August/006284.html
[2]
>From 6fa383d3f7bb53eb5efbb324c07484191b29543d Mon Sep 17 00:00:00 2001
From: Kim Phillips
Date: Fri, 27 Sep 2013 14:36:04 -0500
Subject: [PATCH] hack/rfc: allow vfio-dt to bind to any platform device
---
drivers/base/bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..1cf08d6 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
+ if (dev && dev->driver == NULL && (driver_match_device(drv, dev) ||
+ !strcmp(drv->name, "vfio-dt"))) {
if (dev->parent)/* Needed for USB */
device_lock(dev->parent);
device_lock(dev);
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RFC: (re-)binding the VFIO platform driver to a platform device
Hi,
Santosh and I are having a problem figuring out how to enable binding
(and re-binding) platform devices to a platform VFIO driver (see
Antonis' WIP: [1]) in an upstream-acceptable manner.
Binding platform drivers currently depends on a string match in the
device node's compatible entry. On an arndale, one can currently
rebind the same device to the same driver like so:
echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind
echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind
And one can bind it to the vfio-dt driver, as Antonis instructs, by
appending a 'vfio-dt' string to the device tree compatible entry for
the device. Then this would work:
echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind
echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind
Consequently, the hack patch below [2] allows any platform device to be
bound to the vfio-dt driver, without making changes to the device
tree. It's a hack because I don't see having any driver name specific
code in drivers/base/bus.c being upstream acceptable.
Alternately, device tree compatible entries may be made writeable after
boot, e.g.:
echo vfio-platform /proc/device-tree/i2c\@12CE/compatible
[note s/vfio-dt/vfio-platform/]
but that would require the vfio-platform module be reloaded, thereby
unbinding it from any existing devices it was bound to: we're
seeking a more dynamic solution.
Alex Graf (cc'd) proposed an alternate approach: re-write the driver
name in the device's sysfs entry:
echo vfio-platform /sys/bus/platform/devices/101e.rtc/driver/driver_name
The advantage of this approach is that we can achieve the re-bind
(unbind + bind) as an atomic operation, which alleviates userspace from
having to coordinate with other device operations (I think VM migration
is an example case here).
Note that driver_name currently doesn't exist in sysfs, so it would
either have to be added, or another means developed to rename the
driver symlink itself:
cd /sys/bus/platform/devices/12ce.i2c
ln -s ../../bus/platform/drivers/s5p-ehci /tmp/tmp-link
mv -Tf /tmp/tmp-link driver
So I guess the question is: Is our understanding corret - are we on
the right track at all here? Is the hack below definitely not
acceptable? Is it correct to assume upstream maintainers are against
writing compatible entries to the device tree sysfs at runtime? Would
a driver_name be acceptable to add to sysfs, or should we investigate
something like the atomic mv command above further?
Thanks,
Kim
[1] Note that in this RFC, 'vfio-dt' is the name for the driver (-dt for
device tree) which has already been pointed out as a misnomer and
should probably be rewritten as 'vfio-platform':
http://lists.linux-foundation.org/pipermail/iommu/2013-August/006284.html
[2]
From 6fa383d3f7bb53eb5efbb324c07484191b29543d Mon Sep 17 00:00:00 2001
From: Kim Phillips kim.phill...@linaro.org
Date: Fri, 27 Sep 2013 14:36:04 -0500
Subject: [PATCH] hack/rfc: allow vfio-dt to bind to any platform device
---
drivers/base/bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 4c289ab..1cf08d6 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -201,7 +201,8 @@ static ssize_t bind_store(struct device_driver *drv, const
char *buf,
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
- if (dev dev-driver == NULL driver_match_device(drv, dev)) {
+ if (dev dev-driver == NULL (driver_match_device(drv, dev) ||
+ !strcmp(drv-name, vfio-dt))) {
if (dev-parent)/* Needed for USB */
device_lock(dev-parent);
device_lock(dev);
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. Modifying the device tree is the worse part of this. Is this part of your later suggestion to make compatible writeable after boot, or are you talking about messing with the device tree before boot (putting software config in the device tree, among other ickiness)? Alternately, device tree compatible entries may be made writeable after boot, e.g.: echo vfio-platform /proc/device-tree/i2c\@12CE/compatible [note s/vfio-dt/vfio-platform/] but that would require the vfio-platform module be reloaded, thereby unbinding it from any existing devices it was bound to: we're seeking a more dynamic solution. Eww. Not to mention that the VFIO user might want to know what the compatible was, or that we might later want to unbind from VFIO and rebind to the kernel... Alex Graf (cc'd) proposed an alternate approach: re-write the driver name in the device's sysfs entry: echo vfio-platform /sys/bus/platform/devices/101e.rtc/driver/driver_name The advantage of this approach is that we can achieve the re-bind (unbind + bind) as an atomic operation, which alleviates userspace from having to coordinate with other device operations (I think VM migration is an example case here). Note that driver_name currently doesn't exist in sysfs, so it would either have to be added, or another means developed to rename the driver symlink itself: I think the ideal interface would be if you could write the sysfs device name into the vfio bind file (or some new file in the same directory), and have it claim that device (preferably with an atomic unbind from the previous driver). We shouldn't be messing around with compatible (either modifying it or telling VFIO which compatibles to look for) when we know the specific devices (not just type of devices) we want to bind. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 14:15 -0500, Scott Wood wrote: On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. Modifying the device tree is the worse part of this. I think I missed something. How do you reconcile without making changes to the device tree with appending a 'vfio-dt' string to the device tree compatible entry? -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 14:15:38 -0500 Scott Wood scottw...@freescale.com wrote: On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. Modifying the device tree is the worse part of this. Is this part of your later suggestion to make compatible writeable after boot, or are you talking about messing with the device tree before boot (putting software config in the device tree, among other ickiness)? writeable after boot Alternately, device tree compatible entries may be made writeable after boot, e.g.: echo vfio-platform /proc/device-tree/i2c\@12CE/compatible [note s/vfio-dt/vfio-platform/] but that would require the vfio-platform module be reloaded, thereby unbinding it from any existing devices it was bound to: we're seeking a more dynamic solution. Eww. Not to mention that the VFIO user might want to know what the compatible was, well, technically the user would be able to get that info by reading compatible before writing it, and ideally write the original value back in addition to the new value. or that we might later want to unbind from VFIO and rebind to the kernel... I believe that's independent: it would depend on which driver's (VFIO, kernel, or other) sysfs file the device address gets written into. Alex Graf (cc'd) proposed an alternate approach: re-write the driver name in the device's sysfs entry: echo vfio-platform /sys/bus/platform/devices/101e.rtc/driver/driver_name The advantage of this approach is that we can achieve the re-bind (unbind + bind) as an atomic operation, which alleviates userspace from having to coordinate with other device operations (I think VM migration is an example case here). Note that driver_name currently doesn't exist in sysfs, so it would either have to be added, or another means developed to rename the driver symlink itself: I think the ideal interface would be if you could write the sysfs device name into the vfio bind file (or some new file in the same directory), and have it claim that device (preferably with an atomic unbind from the previous driver). ok. We shouldn't be messing around with compatible (either modifying it or telling VFIO which compatibles to look for) when we know the specific devices (not just type of devices) we want to bind. ok, but I still don't see how to get past driver_match_device()'s refusal to allow bind a non-compatible driver (or one who's name isn't in the compatible list). Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 14:17:16 -0500 Scott Wood scottw...@freescale.com wrote: On Tue, 2013-10-01 at 14:15 -0500, Scott Wood wrote: On Tue, 2013-10-01 at 13:38 -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. Modifying the device tree is the worse part of this. I think I missed something. How do you reconcile without making changes to the device tree with appending a 'vfio-dt' string to the device tree compatible entry? one doesn't need to append 'vfio-dt' to the device tree compatibles if one uses the hack that makes bind_store ignore trying to driver_match_device() if the driver is 'vfio-dt'. Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, 2013-10-01 at 16:59 -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 14:15:38 -0500 Scott Wood scottw...@freescale.com wrote: I think the ideal interface would be if you could write the sysfs device name into the vfio bind file (or some new file in the same directory), and have it claim that device (preferably with an atomic unbind from the previous driver). ok. ...which apparently is what you are already doing (except for the atomic part). My recollection of how this works on PCI (via new_id) apparently kept me from reading it properly. :-P We shouldn't be messing around with compatible (either modifying it or telling VFIO which compatibles to look for) when we know the specific devices (not just type of devices) we want to bind. ok, but I still don't see how to get past driver_match_device()'s refusal to allow bind a non-compatible driver (or one who's name isn't in the compatible list). Probably something similar to your hack, except use a flag or some other neutral mechanism rather than a driver name. The flag could be something like I'll try to bind to any device on this bus, but only if explicitly requested. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? -Christoffer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: (re-)binding the VFIO platform driver to a platform device
On Wed, 2013-10-02 at 02:53 +0100, Christoffer Dall wrote: On Tue, Oct 01, 2013 at 05:02:44PM -0500, Kim Phillips wrote: On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Tue, Oct 01, 2013 at 01:38:31PM -0500, Kim Phillips wrote: Hi, Santosh and I are having a problem figuring out how to enable binding (and re-binding) platform devices to a platform VFIO driver (see Antonis' WIP: [1]) in an upstream-acceptable manner. Binding platform drivers currently depends on a string match in the device node's compatible entry. On an arndale, one can currently rebind the same device to the same driver like so: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/bind And one can bind it to the vfio-dt driver, as Antonis instructs, by appending a 'vfio-dt' string to the device tree compatible entry for the device. Then this would work: echo 12ce.i2c /sys/bus/platform/drivers/s3c-i2c/12ce.i2c/driver/unbind echo 12ce.i2c /sys/bus/platform/drivers/vfio-dt/bind Consequently, the hack patch below [2] allows any platform device to be bound to the vfio-dt driver, without making changes to the device tree. It's a hack because I don't see having any driver name specific code in drivers/base/bus.c being upstream acceptable. You are correct. What is wrong with just doing the above unbind/bind things through sysfs, that is what it is there for, right? The bind fails because the compatible string in the device tree doesn't match that of the VFIO platform driver, so driver_match_device always returns false. It sounds like this is not going to be pretty almost no matter what we'll end up doing: Inherently VFIO is going to bind to a device without the device tree entry for that device ever saying anything about VFIO. How is this solved for PCI? Can we use some analogy from that work to construct the missing piece? PCI supports a dynamic ID table for driver/device matching, see pci_add_dynid(). The problem is that this gets a little sloppy for the period where you have multiple drivers that can claim the same device, especially in the presence of hotplug. Thus the desire to improve the situation with some kind of direct binding interface. Thanks, Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
