RE: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Alex, I think the patch looks good, but did have a question about the hot-adding device scenario above. How do you get the field to be set internally when the device is first discovered? What hook is there to do that prior to the driver match processing? Thanks, Stuart ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Thu, 2014-04-10 at 20:52 +, Stuart Yoder wrote: Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Alex, I think the patch looks good, but did have a question about the hot-adding device scenario above. How do you get the field to be set internally when the device is first discovered? What hook is there to do that prior to the driver match processing? Devices already need to be added to iommu groups prior to driver probe which happens either by the iommu driver calling bus_register_notifier() and looking for BUS_NOTIFY_ADD_DEVICE or they can register an add_device callback in their iommu_ops and let the iommu subsystem register the notifier. When a device is then added to an iommu group, there is yet another notifier. vfio calls iommu_group_register_notifier() and gets told via IOMMU_GROUP_NOTIFY_ADD_DEVICE when a device is added to a group. We currently have a WARN_ON when a device is added to a live group - vfio_group_nb_add_dev(). I expect we'd just replace that with detecting the bus_type of the device and allocating a string with the proper vfio driver for the bus. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Thu, 2014-04-10 at 15:15 -0600, Alex Williamson wrote: On Thu, 2014-04-10 at 20:52 +, Stuart Yoder wrote: Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Alex, I think the patch looks good, but did have a question about the hot-adding device scenario above. How do you get the field to be set internally when the device is first discovered? What hook is there to do that prior to the driver match processing? Devices already need to be added to iommu groups prior to driver probe which happens either by the iommu driver calling bus_register_notifier() and looking for BUS_NOTIFY_ADD_DEVICE or they can register an add_device callback in their iommu_ops and let the iommu subsystem register the notifier. When a device is then added to an iommu group, there is yet another notifier. vfio calls iommu_group_register_notifier() and gets told via IOMMU_GROUP_NOTIFY_ADD_DEVICE when a device is added to a group. We currently have a WARN_ON when a device is added to a live group - vfio_group_nb_add_dev(). I expect we'd just replace that with detecting the bus_type of the device and allocating a string with the proper vfio driver for the bus. Thanks, For instance, the below should work for PCI devices. I imagine we'd also want to print out some useful log message for this as well. Thanks, Alex --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -486,7 +486,14 @@ static int vfio_group_nb_add_dev(struct vfio_group *group, if (!atomic_read(group-container_users)) return 0; - /* TODO Prevent device auto probing */ + /* Only let PCI devices added to a live group bind to vfio-pci */ + if (dev_is_pci(dev)) { + struct pci_dev *pdev = to_pci_dev(dev); + pdev-driver_override = kstrdup(vfio-pci, GFP_KERNEL); + if (pdev-driver_override) + return 0; + } + WARN(Device %s added to live group %d!\n, dev_name(dev), iommu_group_id(group-iommu_group)); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
Needed by platform device drivers, such as the vfio-platform driver [1], in order to bypass the existing OF, ACPI, id_table and name string matches, and successfully be able to be bound to any device, like so: echo vfio-platform /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet /sys/bus/platform/drivers_probe This mimics PCI: Introduce new device binding path using pci_dev.driver_override [2], which is an interface enhancement for more deterministic PCI device binding, e.g., when in the presence of hotplug. [1] http://lkml.iu.edu/hypermail/linux/kernel/1402.1/00177.html [2] http://thread.gmane.org/gmane.linux.kernel.iommu/4605 Signed-off-by: Kim Phillips kim.phill...@freescale.com --- if this looks ok, should it be included in the next version of the vfio-platform submission series, like last time ([1] above)? Documentation/ABI/testing/sysfs-bus-platform | 17 ++ drivers/base/platform.c | 46 include/linux/platform_device.h | 1 + 3 files changed, 64 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform new file mode 100644 index 000..6b14a6a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-platform @@ -0,0 +1,17 @@ +What: /sys/bus/platform/devices/.../driver_override +Date: April 2014 +Contact: Kim Phillips kim.phill...@freescale.com +Description: + This file allows the driver for a device to be specified + which will override standard OF, ACPI, ID table, and name + matching. When specified, only a driver with a name matching + the value written to driver_override will have an opportunity + to bind to the device. The override may be cleared by + writing an empty string (ex. echo driver_override), returning + the device to standard matching rules binding. Writing to + driver_override does not automatically unbind the device from + its current driver or make any attempt to automatically load + the specified driver name. If no driver with a matching name + is currently loaded in the kernel, no match will be found. + This also allows devices to opt-out of driver binding using + a driver_override name such as none. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index e714709..ded1db1 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -22,6 +22,7 @@ #include linux/pm_runtime.h #include linux/idr.h #include linux/acpi.h +#include linux/limits.h #include base.h #include power/power.h @@ -690,8 +691,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, } static DEVICE_ATTR_RO(modalias); +static ssize_t driver_override_store(struct device *dev, +struct device_attribute *attr, +const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old = pdev-driver_override; + + if (count PATH_MAX) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + while (strlen(driver_override) + driver_override[strlen(driver_override) - 1] == '\n') + driver_override[strlen(driver_override) - 1] = '\0'; + + if (strlen(driver_override)) { + pdev-driver_override = driver_override; + } else { + kfree(driver_override); + pdev-driver_override = NULL; + } + + kfree(old); + + return count; +} + +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + + return sprintf(buf, %s\n, pdev-driver_override); +} +static DEVICE_ATTR_RW(driver_override); + + static struct attribute *platform_dev_attrs[] = { dev_attr_modalias.attr, + dev_attr_driver_override.attr, NULL, }; ATTRIBUTE_GROUPS(platform_dev); @@ -747,6 +789,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); + /* When driver_override is set, only bind to the matching driver */ + if (pdev-driver_override) + return !strcmp(pdev-driver_override, drv-name); + /* Attempt an OF style match first */ if (of_driver_match_device(dev, drv)) return 1;
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Tue, 1 Apr 2014 17:23:24 -0700 Greg KH gre...@linuxfoundation.org wrote: On Tue, Apr 01, 2014 at 06:52:12PM -0500, Kim Phillips wrote: On Tue, 01 Apr 2014 10:28:54 -0600 Alex Williamson alex.william...@redhat.com wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Sounds like Greg likes this approach more than {drv,dev}_sysfs_only. I have made no such judgement, I only pointed out that if you ok. If no-one chimes in in favour of one or the other, driver_override works for platform devices. modify/add/remove a sysfs file, it needs to have documentation for it. ok, so the platform device implementation should add a new Documentation/ABI/testing/sysfs-bus-platform... The diff below is the result of duplicating and converting this patch for platform devices, and, indeed, binding a device to the vfio-platform driver succeeds with: echo vfio-platform /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet /sys/bus/platform/drivers_probe However, it's almost pure duplication modulo the bus match code. The only other place I can see where to put the common bus check is drivers/base/base.h:driver_match_device(), which I'm guessing is off-limits? So should we leave this as per-bus code, and somehow refactor driver_override_{show,store}? If you can provide a way for this to be done in a bus-independant way, like we did for new_id and the like, I'd be open to reviewing it. I may be blind, but I don't see any new_id-related code shared between drivers/pci/pci-driver.c and, e.g., drivers/usb/serial/bus.c, nor do I see anything new_id related in drivers/base/. So if we are to follow the current model, the PCI and platform device implementations should be maintained separately. Kim ___ iommu mailing list iommu@lists.linux-foundation.org
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Wed, 2014-04-02 at 17:06 -0500, Kim Phillips wrote: On Tue, 1 Apr 2014 17:23:24 -0700 Greg KH gre...@linuxfoundation.org wrote: On Tue, Apr 01, 2014 at 06:52:12PM -0500, Kim Phillips wrote: On Tue, 01 Apr 2014 10:28:54 -0600 Alex Williamson alex.william...@redhat.com wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Sounds like Greg likes this approach more than {drv,dev}_sysfs_only. I have made no such judgement, I only pointed out that if you ok. If no-one chimes in in favour of one or the other, driver_override works for platform devices. modify/add/remove a sysfs file, it needs to have documentation for it. ok, so the platform device implementation should add a new Documentation/ABI/testing/sysfs-bus-platform... The diff below is the result of duplicating and converting this patch for platform devices, and, indeed, binding a device to the vfio-platform driver succeeds with: echo vfio-platform /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet /sys/bus/platform/drivers_probe However, it's almost pure duplication modulo the bus match code. The only other place I can see where to put the common bus check is drivers/base/base.h:driver_match_device(), which I'm guessing is off-limits? So should we leave this as per-bus code, and somehow refactor driver_override_{show,store}? If you can provide a way for this to be done in a bus-independant way, like we did for new_id and the like, I'd be open to reviewing it. I may be blind, but I don't see any new_id-related code shared between drivers/pci/pci-driver.c and, e.g., drivers/usb/serial/bus.c, nor do I see anything new_id related in drivers/base/. So if we are to follow the current model, the PCI and platform device
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Tue, Apr 01, 2014 at 10:28:54AM -0600, Alex Williamson wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Thanks, Alex drivers/pci/pci-driver.c | 25 ++--- drivers/pci/pci-sysfs.c | 40 include/linux/pci.h |1 + 3 files changed, 63 insertions(+), 3 deletions(-) No Documentation/ABI/ update to reflect the ABI you are creating? thanks, greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Tue, 2014-04-01 at 09:47 -0700, Greg KH wrote: On Tue, Apr 01, 2014 at 10:28:54AM -0600, Alex Williamson wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Thanks, Alex drivers/pci/pci-driver.c | 25 ++--- drivers/pci/pci-sysfs.c | 40 include/linux/pci.h |1 + 3 files changed, 63 insertions(+), 3 deletions(-) No Documentation/ABI/ update to reflect the ABI you are creating? Oops, thanks for the reminder. I'd propose this: diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index a3c5a66..55ca6e5 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -250,3 +250,21 @@ Description: valid. For example, writing a 2 to this file when sriov_numvfs is not 0 and not 2 already will return an error. Writing a 10 when the value of sriov_totalvfs is 8 will return an error. + +What: /sys/bus/pci/devices/.../driver_override +Date: April 2014 +Contact: Alex Williamson alex.william...@redhat.com +Description: + This file allows the driver for a device to be specified + which will override standard static and dynamic ID matching. + When specified, only a driver with a name matching the value + written to driver_override will have an opportunity to bind + to the device. The override may be cleared by writing an + empty string (ex. echo driver_override), returning the + device to standard matching rules binding. Writing to + driver_override does not automatically unbind the device from + its current driver or make any attempt to automatically load + the specified driver name. If no driver with a matching name + is currently loaded in the kernel, no match will be found. + This also
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Tue, 01 Apr 2014 10:28:54 -0600 Alex Williamson alex.william...@redhat.com wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Sounds like Greg likes this approach more than {drv,dev}_sysfs_only. The diff below is the result of duplicating and converting this patch for platform devices, and, indeed, binding a device to the vfio-platform driver succeeds with: echo vfio-platform /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet /sys/bus/platform/drivers_probe However, it's almost pure duplication modulo the bus match code. The only other place I can see where to put the common bus check is drivers/base/base.h:driver_match_device(), which I'm guessing is off-limits? So should we leave this as per-bus code, and somehow refactor driver_override_{show,store}? Kim diff --git a/drivers/base/platform.c b/drivers/base/platform.c index bc78848..621c5bd2 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -22,6 +22,7 @@ #include linux/pm_runtime.h #include linux/idr.h #include linux/acpi.h +#include linux/limits.h #include base.h #include power/power.h @@ -693,8 +694,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, } static DEVICE_ATTR_RO(modalias); +static ssize_t driver_override_store(struct device *dev, +struct device_attribute *attr, +const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old = pdev-driver_override; + + if (count PATH_MAX) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + while (strlen(driver_override) + driver_override[strlen(driver_override) - 1] == '\n') + driver_override[strlen(driver_override) - 1] = '\0'; + + if (strlen(driver_override)) { +
Re: [RFC PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
On Tue, Apr 01, 2014 at 11:15:40AM -0600, Alex Williamson wrote: On Tue, 2014-04-01 at 09:47 -0700, Greg KH wrote: On Tue, Apr 01, 2014 at 10:28:54AM -0600, Alex Williamson wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device, ex: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Apologies for the exceptionally long cc list, this is a follow-up to Stuart's Subject: mechanism to allow a driver to bind to any device thread. This is effectively a v2 of the proof-of-concept patch I posted in that thread. This version changes to use a dummy id struct to return on an override match, which removes the collateral damage and greatly simplifies the patch. This feels fairly well baked for PCI and I would expect that platform drivers could do a similar implementation. From there perhaps we can discuss whether there's any advantage to placing driver_override on struct device. The logic for incorporating it into the match still needs to happen per bus driver, so it might only contribute to consistency of the show/store sysfs attributes to move it up to struct device. Please comment. Thanks, Alex drivers/pci/pci-driver.c | 25 ++--- drivers/pci/pci-sysfs.c | 40 include/linux/pci.h |1 + 3 files changed, 63 insertions(+), 3 deletions(-) No Documentation/ABI/ update to reflect the ABI you are creating? Oops, thanks for the reminder. I'd propose this: diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index a3c5a66..55ca6e5 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -250,3 +250,21 @@ Description: valid. For example, writing a 2 to this file when sriov_numvfs is not 0 and not 2 already will return an error. Writing a 10 when the value of sriov_totalvfs is 8 will return an error. + +What:/sys/bus/pci/devices/.../driver_override +Date:April 2014 +Contact: Alex Williamson alex.william...@redhat.com +Description: + This file allows the driver for a device to be specified + which will override standard static and dynamic ID matching. + When specified, only a driver with a name matching the value + written to driver_override will have an opportunity to bind + to the device. The override may be cleared by writing an + empty string (ex. echo driver_override), returning the + device to standard matching rules binding. Writing to + driver_override does not automatically unbind the device from + its current driver or make any attempt to automatically load + the specified driver