| Message ID | 20131001133831.6e46e8e00e09d5d9079fde57@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > echo 12ce0000.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\@12CE0000/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/101e0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 1 Oct 2013 14:15:38 -0500 Scott Wood <scottwood@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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\@12CE0000/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/101e0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 1 Oct 2013 14:17:16 -0500 Scott Wood <scottwood@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 1 Oct 2013 13:00:54 -0700 Greg Kroah-Hartman <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-10-01 at 16:59 -0500, Kim Phillips wrote: > On Tue, 1 Oct 2013 14:15:38 -0500 > Scott Wood <scottwood@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind > > > > > > echo 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Christoffer Dall [mailto:christoffer.dall@linaro.org] > Sent: Wednesday, October 02, 2013 10:14 AM > To: Alex Williamson > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > kvm@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 <gregkh@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 12ce0000.i2c > /sys/bus/platform/drivers/s3c- > i2c/12ce0000.i2c/driver/unbind > > > > > > echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c- > i2c/12ce0000.i2c/driver/unbind > > > > > > echo 12ce0000.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 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-10-02 at 13:25 -0500, Yoder Stuart-B08248 wrote: > > > -----Original Message----- > > From: Christoffer Dall [mailto:christoffer.dall@linaro.org] > > Sent: Wednesday, October 02, 2013 10:14 AM > > To: Alex Williamson > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > kvm@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 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.dall@linaro.org] > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > To: Alex Williamson > > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > > kvm@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 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2 Oct 2013 11:43:30 -0700 Christoffer Dall <christoffer.dall@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.dall@linaro.org] > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > To: Alex Williamson > > > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > > > kvm@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 12ce0000.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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.dall@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.dall@linaro.org] > > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > > To: Alex Williamson > > > > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > > > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > > > > kvm@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 12ce0000.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 solution. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.dall@linaro.org] > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > To: Alex Williamson > > > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > > > kvm@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 12ce0000.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 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. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.dall@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.dall@linaro.org] > > > > > Sent: Wednesday, October 02, 2013 10:14 AM > > > > > To: Alex Williamson > > > > > Cc: Kim Phillips; gregkh@linuxfoundation.org; linux- > > > > > kernel@vger.kernel.org; a.motakis@virtualopensystems.com; agraf@suse.de; > > > > > Yoder Stuart-B08248; Wood Scott-B07421; Sethi Varun-B16395; Bhushan > > > > > Bharat-R65777; peter.maydell@linaro.org; santosh.shukla@linaro.org; > > > > > kvm@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 12ce0000.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 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. > this does sound reasonable... > > > 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. > I'm not a fan of doing something because something "_could_ have applications elsewhere". I think we need concrete cases to back up the choices for doing something in a specific way. I think that VFIO is one very specific deviant from how all other driver/device binding works, and therefore doing something very specific to VFIO, as isolated as possible to VFIO, is the right thing. > > 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. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 02, 2013 at 01:37:35PM -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 02, 2013 at 01:39:43PM -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-10-02 at 13:37 -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 02, 2013 at 04:08:41PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 13:37 -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-10-02 at 14:16 -0700, gregkh@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, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 02, 2013 at 04:35:15PM -0500, Scott Wood wrote: > On Wed, 2013-10-02 at 14:16 -0700, gregkh@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, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-10-02 at 16:40 -0700, gregkh@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, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-10-03 at 11:54 -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 03, 2013 at 02:11:34PM -0500, Scott Wood wrote: > On Thu, 2013-10-03 at 11:54 -0700, gregkh@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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind echo 12ce0000.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 12ce0000.i2c > /sys/bus/platform/drivers/s3c-i2c/12ce0000.i2c/driver/unbind echo 12ce0000.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\@12CE0000/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/101e0000.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/12ce0000.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.phillips@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(-)