Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 11:08:55PM +, Stuart Yoder wrote: > > > How will it "know not to grab the device"? The knowledge of whether > > the > > > binding was explicitly requested or not does not get passed through to > > > the probe function. > > > > Nor should it, as a driver should not know, nor care about this. > > > > It's up to the BUS to handle this if it really wants to, and I'm afraid > > that I really am not convinced that the driver core needs to handle it > > either. > > > > But again, as you don't have anything that could actually use this code > > that is mergable, it's a totally moot point, sorry. > > Understand, but what assumption do we develop vfio-plaform with? That > a driver core 'sysfs_bind_only' flag is not an option, period? If that > is the case we need to go back to square one and invent some new mechanism > to bind devices to the vfio-platform driver. Ok, why the total confusion between PCI and platform devices here? Why keep mentioning both of them in a semi-interchangeable way? > I guess it would need to be the platform bus equivalent of new_id. Then add it, there's nothing stopping the platform bus from supporting this. > But, then we're left with the potential racy situation where multiple drivers > can potentially grab a device and it's ambiguous and non-deterministic at to > which > driver binds to it. Welcome to life in hotpluggable systems, this is nothing new :) Seriously, userspace has the ability to sort this all out after devices show up, use it. Don't try to create convoluted schemes in the driver core that are confusing and don't really seem to work. People have asked for a "priority" of drivers binding to devices for over a decade now, as "other" operating systems have it. Turns out, no one has ever needed it badly enough to actually implement it... good luck, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Thursday, December 19, 2013 4:32 PM > To: Wood Scott-B07421 > Cc: Yoder Stuart-B08248; Kim Phillips; linux-kernel@vger.kernel.org; > k...@vger.kernel.org; Bhushan Bharat-R65777; christoffer.d...@linaro.org; > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > ag...@suse.de; Sethi Varun-B16395 > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag > to allow binding via sysfs only > > On Thu, Dec 19, 2013 at 04:15:03PM -0600, Scott Wood wrote: > > On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: > > > On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > > > > > Sent: Thursday, December 19, 2013 2:34 PM > > > > > To: Wood Scott-B07421 > > > > > Cc: Kim Phillips; linux-kernel@vger.kernel.org; > k...@vger.kernel.org; > > > > > Bhushan Bharat-R65777; Yoder Stuart-B08248; > christoffer.d...@linaro.org; > > > > > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > > > > > ag...@suse.de; Sethi Varun-B16395 > > > > > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new > device_driver flag > > > > > to allow binding via sysfs only > > > > > > > > > > No. But you can use bind/unbind along with the existing new_id > file to > > > > > get what you want today. > > > > > > > > Yes, but that only works for PCI. > > > > > > No, not only PCI. > > > > > > > There is no such concept for platform drivers. > > > > > > Then fix that. > > > > We've already explained why that would be bad. > > No you haven't, or if you have, my squirrel-brain doesn't remember it... > > > > Or make your device not be a platform device, odds are that's the > better > > > solution in the end, right? > > > > How would that solve anything? We'd just be talking about there not > > being such a mechanism for the device tree "bus" instead. > > Nope, you could add it there, like PCI and other busses have. > > > > > > I don't like this patch as we are adding lots of special and odd > logic > > > > > to the core, for use by almost no one, which ensures that it will > never > > > > > get tested, and will probably get broken in some subtle way in > the > > > > > future. > > > > > > > > It certainly will be used by users of vfio-platform. > > > > > > > > Here is the problem-- the new platform device "match_any_dev" > mechanism > > > > in patch 2 of this series is not going to work without > "sysfs_bind_only". > > > > A platform driver that just sets "match_any_dev" will grab any or > all > > > > platform devices during normal bus probing. > > > > > > No it will not, it will fail in the probe function as it knows to not > > > grab the device, just like any driver for other busses that say it > can > > > "handle all Intel PCI devices" and the like. > > > > How will it "know not to grab the device"? The knowledge of whether > the > > binding was explicitly requested or not does not get passed through to > > the probe function. > > Nor should it, as a driver should not know, nor care about this. > > It's up to the BUS to handle this if it really wants to, and I'm afraid > that I really am not convinced that the driver core needs to handle it > either. > > But again, as you don't have anything that could actually use this code > that is mergable, it's a totally moot point, sorry. Understand, but what assumption do we develop vfio-plaform with? That a driver core 'sysfs_bind_only' flag is not an option, period? If that is the case we need to go back to square one and invent some new mechanism to bind devices to the vfio-platform driver. I guess it would need to be the platform bus equivalent of new_id. But, then we're left with the potential racy situation where multiple drivers can potentially grab a device and it's ambiguous and non-deterministic at to which driver binds to it. Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 04:15:03PM -0600, Scott Wood wrote: > On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: > > On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: > > > > > > > > > > -Original Message- > > > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > > > > Sent: Thursday, December 19, 2013 2:34 PM > > > > To: Wood Scott-B07421 > > > > Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; > > > > Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; > > > > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > > > > ag...@suse.de; Sethi Varun-B16395 > > > > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag > > > > to allow binding via sysfs only > > > > > > > > No. But you can use bind/unbind along with the existing new_id file to > > > > get what you want today. > > > > > > Yes, but that only works for PCI. > > > > No, not only PCI. > > > > > There is no such concept for platform drivers. > > > > Then fix that. > > We've already explained why that would be bad. No you haven't, or if you have, my squirrel-brain doesn't remember it... > > Or make your device not be a platform device, odds are that's the better > > solution in the end, right? > > How would that solve anything? We'd just be talking about there not > being such a mechanism for the device tree "bus" instead. Nope, you could add it there, like PCI and other busses have. > > > > I don't like this patch as we are adding lots of special and odd logic > > > > to the core, for use by almost no one, which ensures that it will never > > > > get tested, and will probably get broken in some subtle way in the > > > > future. > > > > > > It certainly will be used by users of vfio-platform. > > > > > > Here is the problem-- the new platform device "match_any_dev" mechanism > > > in patch 2 of this series is not going to work without "sysfs_bind_only". > > > A platform driver that just sets "match_any_dev" will grab any or all > > > platform devices during normal bus probing. > > > > No it will not, it will fail in the probe function as it knows to not > > grab the device, just like any driver for other busses that say it can > > "handle all Intel PCI devices" and the like. > > How will it "know not to grab the device"? The knowledge of whether the > binding was explicitly requested or not does not get passed through to > the probe function. Nor should it, as a driver should not know, nor care about this. It's up to the BUS to handle this if it really wants to, and I'm afraid that I really am not convinced that the driver core needs to handle it either. But again, as you don't have anything that could actually use this code that is mergable, it's a totally moot point, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: > On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: > > > > > > > -Original Message- > > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > > > Sent: Thursday, December 19, 2013 2:34 PM > > > To: Wood Scott-B07421 > > > Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; > > > Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; > > > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > > > ag...@suse.de; Sethi Varun-B16395 > > > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag > > > to allow binding via sysfs only > > > > > > No. But you can use bind/unbind along with the existing new_id file to > > > get what you want today. > > > > Yes, but that only works for PCI. > > No, not only PCI. > > > There is no such concept for platform drivers. > > Then fix that. We've already explained why that would be bad. > Or make your device not be a platform device, odds are that's the better > solution in the end, right? How would that solve anything? We'd just be talking about there not being such a mechanism for the device tree "bus" instead. > > > If you just happen to bind a device to a wrong > > > driver for a while, that's not really a problem, right? > > > > It's annoying but not the end of the world. Lots of bugs are "not the end of the world" but that doesn't mean we don't fix them. It certainly doesn't mean we duplicate the source of the bug in new subsystems. > > > I don't like this patch as we are adding lots of special and odd logic > > > to the core, for use by almost no one, which ensures that it will never > > > get tested, and will probably get broken in some subtle way in the > > > future. > > > > It certainly will be used by users of vfio-platform. > > > > Here is the problem-- the new platform device "match_any_dev" mechanism > > in patch 2 of this series is not going to work without "sysfs_bind_only". > > A platform driver that just sets "match_any_dev" will grab any or all > > platform devices during normal bus probing. > > No it will not, it will fail in the probe function as it knows to not > grab the device, just like any driver for other busses that say it can > "handle all Intel PCI devices" and the like. How will it "know not to grab the device"? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. It would be a shame to have to implement code in VFIO, and a userspace interface to go along with it, to keep track of which devices have been authorized to be bound when we already have sysfs bind -- we just need a way to differentiate that from automatic binding. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: > > > > -Original Message- > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > > Sent: Thursday, December 19, 2013 2:34 PM > > To: Wood Scott-B07421 > > Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; > > Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; > > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > > ag...@suse.de; Sethi Varun-B16395 > > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag > > to allow binding via sysfs only > > > > On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: > > > On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: > > > > On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: > > > > > VFIO supports pass-through of devices to user space - for sake > > > > > of illustration, say a PCI e1000 device: > > > > > > > > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs > > > > > - the vfio-pci driver is told via new_id that it now handles e1000 > > devices > > > > > - the e1000 is explicitly bound to vfio-pci through sysfs > > > > > > > > > > However, now we have two drivers in the system that both handle > > e1000 > > > > > devices. A hotplug event could then occur and it is ambiguous as > > to which > > > > > driver will claim the device. The desired semantics is that vfio- > > pci is > > > > > only bound to devices by explicit request in sysfs. This patch > > makes this > > > > > possible by introducing a sysfs_bind_only flag in struct > > device_driver. > > > > > > > > Why deal with this at all and not just deal with the "bind" sysfs > > file > > > > instead? That way no driver core logic needs to be changed at all, > > and > > > > your userspace tools know _exactly_ which device is being bound to > > the > > > > new device. > > > > > > > > Don't mess with the "new_id" file for stuff like this, as you point > > out, > > > > it's "tricky"... > > > > > > As discussed before, "bind" does not bypass the ID checks, and thus it > > > does not work without either "new_id" or a wildcard match. > > > > Ah, forgot about that. > > > > > Or are you proposing changing "bind" so that it does bypass the ID > > > checks? Or perhaps a new "force_bind" file that does? > > > > No. But you can use bind/unbind along with the existing new_id file to > > get what you want today. > > Yes, but that only works for PCI. No, not only PCI. > There is no such concept for platform drivers. Then fix that. Or make your device not be a platform device, odds are that's the better solution in the end, right? > > If you just happen to bind a device to a wrong > > driver for a while, that's not really a problem, right? > > It's annoying but not the end of the world. > > > I don't like this patch as we are adding lots of special and odd logic > > to the core, for use by almost no one, which ensures that it will never > > get tested, and will probably get broken in some subtle way in the > > future. > > It certainly will be used by users of vfio-platform. > > Here is the problem-- the new platform device "match_any_dev" mechanism > in patch 2 of this series is not going to work without "sysfs_bind_only". > A platform driver that just sets "match_any_dev" will grab any or all > platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can "handle all Intel PCI devices" and the like. > > Heck, I can't even ensure that you got it right now, with this tiny > > patch, how do you know it works given that there are no users of this > > flag anywhere (hint, you never showed me any...) > > It has been tested on both vfio-pci and vfio-platform in some limited > experiments as far as I know, but that code is not ready for upstream > yet. Then I'll wait until you get something that actually is worth upstreaming before worrying about this again. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Thursday, December 19, 2013 2:34 PM > To: Wood Scott-B07421 > Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; > Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; > alex.william...@redhat.com; a.mota...@virtualopensystems.com; > ag...@suse.de; Sethi Varun-B16395 > Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag > to allow binding via sysfs only > > On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: > > On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: > > > On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: > > > > VFIO supports pass-through of devices to user space - for sake > > > > of illustration, say a PCI e1000 device: > > > > > > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs > > > > - the vfio-pci driver is told via new_id that it now handles e1000 > devices > > > > - the e1000 is explicitly bound to vfio-pci through sysfs > > > > > > > > However, now we have two drivers in the system that both handle > e1000 > > > > devices. A hotplug event could then occur and it is ambiguous as > to which > > > > driver will claim the device. The desired semantics is that vfio- > pci is > > > > only bound to devices by explicit request in sysfs. This patch > makes this > > > > possible by introducing a sysfs_bind_only flag in struct > device_driver. > > > > > > Why deal with this at all and not just deal with the "bind" sysfs > file > > > instead? That way no driver core logic needs to be changed at all, > and > > > your userspace tools know _exactly_ which device is being bound to > the > > > new device. > > > > > > Don't mess with the "new_id" file for stuff like this, as you point > out, > > > it's "tricky"... > > > > As discussed before, "bind" does not bypass the ID checks, and thus it > > does not work without either "new_id" or a wildcard match. > > Ah, forgot about that. > > > Or are you proposing changing "bind" so that it does bypass the ID > > checks? Or perhaps a new "force_bind" file that does? > > No. But you can use bind/unbind along with the existing new_id file to > get what you want today. Yes, but that only works for PCI. There is no such concept for platform drivers. > If you just happen to bind a device to a wrong > driver for a while, that's not really a problem, right? It's annoying but not the end of the world. > I don't like this patch as we are adding lots of special and odd logic > to the core, for use by almost no one, which ensures that it will never > get tested, and will probably get broken in some subtle way in the > future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device "match_any_dev" mechanism in patch 2 of this series is not going to work without "sysfs_bind_only". A platform driver that just sets "match_any_dev" will grab any or all platform devices during normal bus probing. > Heck, I can't even ensure that you got it right now, with this tiny > patch, how do you know it works given that there are no users of this > flag anywhere (hint, you never showed me any...) It has been tested on both vfio-pci and vfio-platform in some limited experiments as far as I know, but that code is not ready for upstream yet. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: > On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: > > On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: > > > VFIO supports pass-through of devices to user space - for sake > > > of illustration, say a PCI e1000 device: > > > > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs > > > - the vfio-pci driver is told via new_id that it now handles e1000 devices > > > - the e1000 is explicitly bound to vfio-pci through sysfs > > > > > > However, now we have two drivers in the system that both handle e1000 > > > devices. A hotplug event could then occur and it is ambiguous as to which > > > driver will claim the device. The desired semantics is that vfio-pci is > > > only bound to devices by explicit request in sysfs. This patch makes this > > > possible by introducing a sysfs_bind_only flag in struct device_driver. > > > > Why deal with this at all and not just deal with the "bind" sysfs file > > instead? That way no driver core logic needs to be changed at all, and > > your userspace tools know _exactly_ which device is being bound to the > > new device. > > > > Don't mess with the "new_id" file for stuff like this, as you point out, > > it's "tricky"... > > As discussed before, "bind" does not bypass the ID checks, and thus it > does not work without either "new_id" or a wildcard match. Ah, forgot about that. > Or are you proposing changing "bind" so that it does bypass the ID > checks? Or perhaps a new "force_bind" file that does? No. But you can use bind/unbind along with the existing new_id file to get what you want today. If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. Heck, I can't even ensure that you got it right now, with this tiny patch, how do you know it works given that there are no users of this flag anywhere (hint, you never showed me any...) greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: > On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: > > VFIO supports pass-through of devices to user space - for sake > > of illustration, say a PCI e1000 device: > > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs > > - the vfio-pci driver is told via new_id that it now handles e1000 devices > > - the e1000 is explicitly bound to vfio-pci through sysfs > > > > However, now we have two drivers in the system that both handle e1000 > > devices. A hotplug event could then occur and it is ambiguous as to which > > driver will claim the device. The desired semantics is that vfio-pci is > > only bound to devices by explicit request in sysfs. This patch makes this > > possible by introducing a sysfs_bind_only flag in struct device_driver. > > Why deal with this at all and not just deal with the "bind" sysfs file > instead? That way no driver core logic needs to be changed at all, and > your userspace tools know _exactly_ which device is being bound to the > new device. > > Don't mess with the "new_id" file for stuff like this, as you point out, > it's "tricky"... As discussed before, "bind" does not bypass the ID checks, and thus it does not work without either "new_id" or a wildcard match. Or are you proposing changing "bind" so that it does bypass the ID checks? Or perhaps a new "force_bind" file that does? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio-pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... As discussed before, bind does not bypass the ID checks, and thus it does not work without either new_id or a wildcard match. Or are you proposing changing bind so that it does bypass the ID checks? Or perhaps a new force_bind file that does? -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio-pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... As discussed before, bind does not bypass the ID checks, and thus it does not work without either new_id or a wildcard match. Ah, forgot about that. Or are you proposing changing bind so that it does bypass the ID checks? Or perhaps a new force_bind file that does? No. But you can use bind/unbind along with the existing new_id file to get what you want today. If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. Heck, I can't even ensure that you got it right now, with this tiny patch, how do you know it works given that there are no users of this flag anywhere (hint, you never showed me any...) greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio- pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... As discussed before, bind does not bypass the ID checks, and thus it does not work without either new_id or a wildcard match. Ah, forgot about that. Or are you proposing changing bind so that it does bypass the ID checks? Or perhaps a new force_bind file that does? No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. There is no such concept for platform drivers. If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? It's annoying but not the end of the world. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. Heck, I can't even ensure that you got it right now, with this tiny patch, how do you know it works given that there are no users of this flag anywhere (hint, you never showed me any...) It has been tested on both vfio-pci and vfio-platform in some limited experiments as far as I know, but that code is not ready for upstream yet. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: -Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio- pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... As discussed before, bind does not bypass the ID checks, and thus it does not work without either new_id or a wildcard match. Ah, forgot about that. Or are you proposing changing bind so that it does bypass the ID checks? Or perhaps a new force_bind file that does? No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. No, not only PCI. There is no such concept for platform drivers. Then fix that. Or make your device not be a platform device, odds are that's the better solution in the end, right? If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? It's annoying but not the end of the world. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can handle all Intel PCI devices and the like. Heck, I can't even ensure that you got it right now, with this tiny patch, how do you know it works given that there are no users of this flag anywhere (hint, you never showed me any...) It has been tested on both vfio-pci and vfio-platform in some limited experiments as far as I know, but that code is not ready for upstream yet. Then I'll wait until you get something that actually is worth upstreaming before worrying about this again. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: -Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. No, not only PCI. There is no such concept for platform drivers. Then fix that. We've already explained why that would be bad. Or make your device not be a platform device, odds are that's the better solution in the end, right? How would that solve anything? We'd just be talking about there not being such a mechanism for the device tree bus instead. If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? It's annoying but not the end of the world. Lots of bugs are not the end of the world but that doesn't mean we don't fix them. It certainly doesn't mean we duplicate the source of the bug in new subsystems. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can handle all Intel PCI devices and the like. How will it know not to grab the device? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. It would be a shame to have to implement code in VFIO, and a userspace interface to go along with it, to keep track of which devices have been authorized to be bound when we already have sysfs bind -- we just need a way to differentiate that from automatic binding. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 04:15:03PM -0600, Scott Wood wrote: On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: -Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. No, not only PCI. There is no such concept for platform drivers. Then fix that. We've already explained why that would be bad. No you haven't, or if you have, my squirrel-brain doesn't remember it... Or make your device not be a platform device, odds are that's the better solution in the end, right? How would that solve anything? We'd just be talking about there not being such a mechanism for the device tree bus instead. Nope, you could add it there, like PCI and other busses have. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can handle all Intel PCI devices and the like. How will it know not to grab the device? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. Nor should it, as a driver should not know, nor care about this. It's up to the BUS to handle this if it really wants to, and I'm afraid that I really am not convinced that the driver core needs to handle it either. But again, as you don't have anything that could actually use this code that is mergable, it's a totally moot point, sorry. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 4:32 PM To: Wood Scott-B07421 Cc: Yoder Stuart-B08248; Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only On Thu, Dec 19, 2013 at 04:15:03PM -0600, Scott Wood wrote: On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: -Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. No, not only PCI. There is no such concept for platform drivers. Then fix that. We've already explained why that would be bad. No you haven't, or if you have, my squirrel-brain doesn't remember it... Or make your device not be a platform device, odds are that's the better solution in the end, right? How would that solve anything? We'd just be talking about there not being such a mechanism for the device tree bus instead. Nope, you could add it there, like PCI and other busses have. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can handle all Intel PCI devices and the like. How will it know not to grab the device? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. Nor should it, as a driver should not know, nor care about this. It's up to the BUS to handle this if it really wants to, and I'm afraid that I really am not convinced that the driver core needs to handle it either. But again, as you don't have anything that could actually use this code that is mergable, it's a totally moot point, sorry. Understand, but what assumption do we develop vfio-plaform with? That a driver core 'sysfs_bind_only' flag is not an option, period? If that is the case we need to go back to square one and invent some new mechanism to bind devices to the vfio-platform driver. I guess it would need to be the platform bus equivalent of new_id. But, then we're left with the potential racy situation where multiple drivers can potentially grab a device and it's ambiguous and non-deterministic at to which driver binds to it. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, Dec 19, 2013 at 11:08:55PM +, Stuart Yoder wrote: How will it know not to grab the device? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. Nor should it, as a driver should not know, nor care about this. It's up to the BUS to handle this if it really wants to, and I'm afraid that I really am not convinced that the driver core needs to handle it either. But again, as you don't have anything that could actually use this code that is mergable, it's a totally moot point, sorry. Understand, but what assumption do we develop vfio-plaform with? That a driver core 'sysfs_bind_only' flag is not an option, period? If that is the case we need to go back to square one and invent some new mechanism to bind devices to the vfio-platform driver. Ok, why the total confusion between PCI and platform devices here? Why keep mentioning both of them in a semi-interchangeable way? I guess it would need to be the platform bus equivalent of new_id. Then add it, there's nothing stopping the platform bus from supporting this. But, then we're left with the potential racy situation where multiple drivers can potentially grab a device and it's ambiguous and non-deterministic at to which driver binds to it. Welcome to life in hotpluggable systems, this is nothing new :) Seriously, userspace has the ability to sort this all out after devices show up, use it. Don't try to create convoluted schemes in the driver core that are confusing and don't really seem to work. People have asked for a priority of drivers binding to devices for over a decade now, as other operating systems have it. Turns out, no one has ever needed it badly enough to actually implement it... good luck, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, Dec 03, 2013 at 04:34:33PM +0100, Jan Kiszka wrote:
> On 2013-12-03 13:34, Kim Phillips wrote:
> > VFIO supports pass-through of devices to user space - for sake
> > of illustration, say a PCI e1000 device:
> >
> > - the e1000 is first unbound from the PCI e1000 driver via sysfs
> > - the vfio-pci driver is told via new_id that it now handles e1000 devices
> > - the e1000 is explicitly bound to vfio-pci through sysfs
> >
> > However, now we have two drivers in the system that both handle e1000
> > devices. A hotplug event could then occur and it is ambiguous as to which
> > driver will claim the device. The desired semantics is that vfio-pci is
> > only bound to devices by explicit request in sysfs. This patch makes this
> > possible by introducing a sysfs_bind_only flag in struct device_driver.
> >
> > Signed-off-by: Stuart Yoder
> > Signed-off-by: Kim Phillips
> > ---
> > rebased onto 3.13-rc2, and reposted from first submission which
> > recieved no comments:
> >
> > https://lkml.org/lkml/2013/10/11/53
> >
> > drivers/base/dd.c | 5 -
> > include/linux/device.h | 2 ++
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 0605176..b83b16d 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> > void *data)
> > {
> > struct device *dev = data;
> >
> > - if (!driver_match_device(drv, dev))
> > + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> > return 0;
> >
> > return driver_probe_device(drv, dev);
> > @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
> > *data)
> > */
> > int driver_attach(struct device_driver *drv)
> > {
> > + if (drv->sysfs_bind_only)
> > + return 0;
> > +
> > return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> > }
> > EXPORT_SYMBOL_GPL(driver_attach);
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 952b010..ed441d1 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
> > bus_type *bus);
> > * @owner: The module owner.
> > * @mod_name: Used for built-in modules.
> > * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> > + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> > * @of_match_table: The open firmware table.
> > * @acpi_match_table: The ACPI match table.
> > * @probe: Called to query the existence of a specific device,
> > @@ -233,6 +234,7 @@ struct device_driver {
> > const char *mod_name; /* used for built-in modules */
> >
> > bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
> > + bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
> >
> > const struct of_device_id *of_match_table;
> > const struct acpi_device_id *acpi_match_table;
> >
>
> I think I only discussed this with Stuart in person at the KVM Forum:
> Why not deriving the property "sysfs bind only" from the fact that a
> device does wild-card binding? Are there use cases that benefit from
> decoupling both features?
The driver core does not know if a bus, or a device, handles "wild card"
binding, so you can't do it at this level, sorry.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: > VFIO supports pass-through of devices to user space - for sake > of illustration, say a PCI e1000 device: > > - the e1000 is first unbound from the PCI e1000 driver via sysfs > - the vfio-pci driver is told via new_id that it now handles e1000 devices > - the e1000 is explicitly bound to vfio-pci through sysfs > > However, now we have two drivers in the system that both handle e1000 > devices. A hotplug event could then occur and it is ambiguous as to which > driver will claim the device. The desired semantics is that vfio-pci is > only bound to devices by explicit request in sysfs. This patch makes this > possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the "bind" sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the "new_id" file for stuff like this, as you point out, it's "tricky"... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio-pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, Dec 03, 2013 at 04:34:33PM +0100, Jan Kiszka wrote:
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000 devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
*data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
bus_type *bus);
* @owner: The module owner.
* @mod_name: Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
The driver core does not know if a bus, or a device, handles wild card
binding, so you can't do it at this level, sorry.
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Mon, 2013-12-09 at 20:12 +0100, Jan Kiszka wrote: > On 2013-12-09 19:58, Kim Phillips wrote: > > On Thu, 5 Dec 2013 16:38:15 -0600 > > Scott Wood wrote: > > > >> What would combining them solve, other than making it more likely that > >> Greg complains about the wildcard because it would no longer be handled > >> at the bus level where all the other matching goes on? > >> > >> They are logically separate things. That doesn't change just because we > >> currently plan to use them together. > > > > Jan? Given the above, what would be the advantage of merging > > sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers' > > match_any_dev)? > > That you cannot configure (likely) meaningless or even harmful (bind-any > + auto-bind) configurations. If you want to put in a check that warns on bind-any plus auto-bind, fine -- but that doesn't mean they should share a mechanism. It's valid to have no-auto-bind without a wildcard match. And FWIW, PCI already has wildcard matches without any no-auto-bind mechanism (it's presumably not intended to specify PCI_ANY_ID for all fields, but it is allowed AFAICT). -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On 2013-12-09 19:58, Kim Phillips wrote:
> On Thu, 5 Dec 2013 16:38:15 -0600
> Scott Wood wrote:
>
>> On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
>>> On Tue, 03 Dec 2013 16:34:33 +0100
>>> Jan Kiszka wrote:
>>>
On 2013-12-03 13:34, Kim Phillips wrote:
> VFIO supports pass-through of devices to user space - for sake
> of illustration, say a PCI e1000 device:
>
> - the e1000 is first unbound from the PCI e1000 driver via sysfs
> - the vfio-pci driver is told via new_id that it now handles e1000 devices
> - the e1000 is explicitly bound to vfio-pci through sysfs
>
> However, now we have two drivers in the system that both handle e1000
> devices. A hotplug event could then occur and it is ambiguous as to which
> driver will claim the device. The desired semantics is that vfio-pci is
> only bound to devices by explicit request in sysfs. This patch makes this
> possible by introducing a sysfs_bind_only flag in struct device_driver.
>
> Signed-off-by: Stuart Yoder
> Signed-off-by: Kim Phillips
> ---
> rebased onto 3.13-rc2, and reposted from first submission which
> recieved no comments:
>
> https://lkml.org/lkml/2013/10/11/53
>
> drivers/base/dd.c | 5 -
> include/linux/device.h | 2 ++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0605176..b83b16d 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> void *data)
> {
> struct device *dev = data;
>
> - if (!driver_match_device(drv, dev))
> + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> return 0;
>
> return driver_probe_device(drv, dev);
> @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
> *data)
> */
> int driver_attach(struct device_driver *drv)
> {
> + if (drv->sysfs_bind_only)
> + return 0;
> +
> return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> }
> EXPORT_SYMBOL_GPL(driver_attach);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 952b010..ed441d1 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
> bus_type *bus);
> * @owner: The module owner.
> * @mod_name:Used for built-in modules.
> * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> * @of_match_table: The open firmware table.
> * @acpi_match_table: The ACPI match table.
> * @probe: Called to query the existence of a specific device,
> @@ -233,6 +234,7 @@ struct device_driver {
> const char *mod_name; /* used for built-in modules */
>
> bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
> + bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
>
> const struct of_device_id *of_match_table;
> const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property "sysfs bind only" from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
>>>
>>> you mean merge the two new flags sysfs_bind_only and platform driver's
>>> match_any_dev into one new single driver flag, right? good question.
>>
>> What would combining them solve, other than making it more likely that
>> Greg complains about the wildcard because it would no longer be handled
>> at the bus level where all the other matching goes on?
>>
>> They are logically separate things. That doesn't change just because we
>> currently plan to use them together.
>
> Jan? Given the above, what would be the advantage of merging
> sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers'
> match_any_dev)?
That you cannot configure (likely) meaningless or even harmful (bind-any
+ auto-bind) configurations.
I didn't follow if Greg expressed his opinion on this or a similar
scenario before. If he prefers separate knobs for a certain reason, he
likely wins.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 5 Dec 2013 16:38:15 -0600
Scott Wood wrote:
> On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
> > On Tue, 03 Dec 2013 16:34:33 +0100
> > Jan Kiszka wrote:
> >
> > > On 2013-12-03 13:34, Kim Phillips wrote:
> > > > VFIO supports pass-through of devices to user space - for sake
> > > > of illustration, say a PCI e1000 device:
> > > >
> > > > - the e1000 is first unbound from the PCI e1000 driver via sysfs
> > > > - the vfio-pci driver is told via new_id that it now handles e1000
> > > > devices
> > > > - the e1000 is explicitly bound to vfio-pci through sysfs
> > > >
> > > > However, now we have two drivers in the system that both handle e1000
> > > > devices. A hotplug event could then occur and it is ambiguous as to
> > > > which
> > > > driver will claim the device. The desired semantics is that vfio-pci is
> > > > only bound to devices by explicit request in sysfs. This patch makes
> > > > this
> > > > possible by introducing a sysfs_bind_only flag in struct device_driver.
> > > >
> > > > Signed-off-by: Stuart Yoder
> > > > Signed-off-by: Kim Phillips
> > > > ---
> > > > rebased onto 3.13-rc2, and reposted from first submission which
> > > > recieved no comments:
> > > >
> > > > https://lkml.org/lkml/2013/10/11/53
> > > >
> > > > drivers/base/dd.c | 5 -
> > > > include/linux/device.h | 2 ++
> > > > 2 files changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > > > index 0605176..b83b16d 100644
> > > > --- a/drivers/base/dd.c
> > > > +++ b/drivers/base/dd.c
> > > > @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver
> > > > *drv, void *data)
> > > > {
> > > > struct device *dev = data;
> > > >
> > > > - if (!driver_match_device(drv, dev))
> > > > + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> > > > return 0;
> > > >
> > > > return driver_probe_device(drv, dev);
> > > > @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
> > > > *data)
> > > > */
> > > > int driver_attach(struct device_driver *drv)
> > > > {
> > > > + if (drv->sysfs_bind_only)
> > > > + return 0;
> > > > +
> > > > return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> > > > }
> > > > EXPORT_SYMBOL_GPL(driver_attach);
> > > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > > index 952b010..ed441d1 100644
> > > > --- a/include/linux/device.h
> > > > +++ b/include/linux/device.h
> > > > @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
> > > > bus_type *bus);
> > > > * @owner: The module owner.
> > > > * @mod_name: Used for built-in modules.
> > > > * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> > > > + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> > > > * @of_match_table: The open firmware table.
> > > > * @acpi_match_table: The ACPI match table.
> > > > * @probe: Called to query the existence of a specific device,
> > > > @@ -233,6 +234,7 @@ struct device_driver {
> > > > const char *mod_name; /* used for built-in
> > > > modules */
> > > >
> > > > bool suppress_bind_attrs; /* disables bind/unbind via
> > > > sysfs */
> > > > + bool sysfs_bind_only; /* only allow bind/unbind via
> > > > sysfs */
> > > >
> > > > const struct of_device_id *of_match_table;
> > > > const struct acpi_device_id *acpi_match_table;
> > >
> > > I think I only discussed this with Stuart in person at the KVM Forum:
> > > Why not deriving the property "sysfs bind only" from the fact that a
> > > device does wild-card binding? Are there use cases that benefit from
> > > decoupling both features?
> >
> > you mean merge the two new flags sysfs_bind_only and platform driver's
> > match_any_dev into one new single driver flag, right? good question.
>
> What would combining them solve, other than making it more likely that
> Greg complains about the wildcard because it would no longer be handled
> at the bus level where all the other matching goes on?
>
> They are logically separate things. That doesn't change just because we
> currently plan to use them together.
Jan? Given the above, what would be the advantage of merging
sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers'
match_any_dev)?
Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 5 Dec 2013 16:38:15 -0600
Scott Wood scottw...@freescale.com wrote:
On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
On Tue, 03 Dec 2013 16:34:33 +0100
Jan Kiszka jan.kis...@siemens.com wrote:
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000
devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to
which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes
this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver
*drv, void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
*data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
bus_type *bus);
* @owner: The module owner.
* @mod_name: Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in
modules */
bool suppress_bind_attrs; /* disables bind/unbind via
sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via
sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
you mean merge the two new flags sysfs_bind_only and platform driver's
match_any_dev into one new single driver flag, right? good question.
What would combining them solve, other than making it more likely that
Greg complains about the wildcard because it would no longer be handled
at the bus level where all the other matching goes on?
They are logically separate things. That doesn't change just because we
currently plan to use them together.
Jan? Given the above, what would be the advantage of merging
sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers'
match_any_dev)?
Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On 2013-12-09 19:58, Kim Phillips wrote:
On Thu, 5 Dec 2013 16:38:15 -0600
Scott Wood scottw...@freescale.com wrote:
On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
On Tue, 03 Dec 2013 16:34:33 +0100
Jan Kiszka jan.kis...@siemens.com wrote:
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000 devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
*data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
bus_type *bus);
* @owner: The module owner.
* @mod_name:Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
you mean merge the two new flags sysfs_bind_only and platform driver's
match_any_dev into one new single driver flag, right? good question.
What would combining them solve, other than making it more likely that
Greg complains about the wildcard because it would no longer be handled
at the bus level where all the other matching goes on?
They are logically separate things. That doesn't change just because we
currently plan to use them together.
Jan? Given the above, what would be the advantage of merging
sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers'
match_any_dev)?
That you cannot configure (likely) meaningless or even harmful (bind-any
+ auto-bind) configurations.
I didn't follow if Greg expressed his opinion on this or a similar
scenario before. If he prefers separate knobs for a certain reason, he
likely wins.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Mon, 2013-12-09 at 20:12 +0100, Jan Kiszka wrote: On 2013-12-09 19:58, Kim Phillips wrote: On Thu, 5 Dec 2013 16:38:15 -0600 Scott Wood scottw...@freescale.com wrote: What would combining them solve, other than making it more likely that Greg complains about the wildcard because it would no longer be handled at the bus level where all the other matching goes on? They are logically separate things. That doesn't change just because we currently plan to use them together. Jan? Given the above, what would be the advantage of merging sysfs_bind_only and (PCI drivers' PCI_ANY_ID and platform drivers' match_any_dev)? That you cannot configure (likely) meaningless or even harmful (bind-any + auto-bind) configurations. If you want to put in a check that warns on bind-any plus auto-bind, fine -- but that doesn't mean they should share a mechanism. It's valid to have no-auto-bind without a wildcard match. And FWIW, PCI already has wildcard matches without any no-auto-bind mechanism (it's presumably not intended to specify PCI_ANY_ID for all fields, but it is allowed AFAICT). -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
> On Tue, 03 Dec 2013 16:34:33 +0100
> Jan Kiszka wrote:
>
> > On 2013-12-03 13:34, Kim Phillips wrote:
> > > VFIO supports pass-through of devices to user space - for sake
> > > of illustration, say a PCI e1000 device:
> > >
> > > - the e1000 is first unbound from the PCI e1000 driver via sysfs
> > > - the vfio-pci driver is told via new_id that it now handles e1000 devices
> > > - the e1000 is explicitly bound to vfio-pci through sysfs
> > >
> > > However, now we have two drivers in the system that both handle e1000
> > > devices. A hotplug event could then occur and it is ambiguous as to which
> > > driver will claim the device. The desired semantics is that vfio-pci is
> > > only bound to devices by explicit request in sysfs. This patch makes this
> > > possible by introducing a sysfs_bind_only flag in struct device_driver.
> > >
> > > Signed-off-by: Stuart Yoder
> > > Signed-off-by: Kim Phillips
> > > ---
> > > rebased onto 3.13-rc2, and reposted from first submission which
> > > recieved no comments:
> > >
> > > https://lkml.org/lkml/2013/10/11/53
> > >
> > > drivers/base/dd.c | 5 -
> > > include/linux/device.h | 2 ++
> > > 2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > > index 0605176..b83b16d 100644
> > > --- a/drivers/base/dd.c
> > > +++ b/drivers/base/dd.c
> > > @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> > > void *data)
> > > {
> > > struct device *dev = data;
> > >
> > > - if (!driver_match_device(drv, dev))
> > > + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> > > return 0;
> > >
> > > return driver_probe_device(drv, dev);
> > > @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
> > > *data)
> > > */
> > > int driver_attach(struct device_driver *drv)
> > > {
> > > + if (drv->sysfs_bind_only)
> > > + return 0;
> > > +
> > > return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> > > }
> > > EXPORT_SYMBOL_GPL(driver_attach);
> > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > index 952b010..ed441d1 100644
> > > --- a/include/linux/device.h
> > > +++ b/include/linux/device.h
> > > @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
> > > bus_type *bus);
> > > * @owner: The module owner.
> > > * @mod_name:Used for built-in modules.
> > > * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> > > + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> > > * @of_match_table: The open firmware table.
> > > * @acpi_match_table: The ACPI match table.
> > > * @probe: Called to query the existence of a specific device,
> > > @@ -233,6 +234,7 @@ struct device_driver {
> > > const char *mod_name; /* used for built-in modules */
> > >
> > > bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
> > > + bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
> > >
> > > const struct of_device_id *of_match_table;
> > > const struct acpi_device_id *acpi_match_table;
> >
> > I think I only discussed this with Stuart in person at the KVM Forum:
> > Why not deriving the property "sysfs bind only" from the fact that a
> > device does wild-card binding? Are there use cases that benefit from
> > decoupling both features?
>
> you mean merge the two new flags sysfs_bind_only and platform driver's
> match_any_dev into one new single driver flag, right? good question.
What would combining them solve, other than making it more likely that
Greg complains about the wildcard because it would no longer be handled
at the bus level where all the other matching goes on?
They are logically separate things. That doesn't change just because we
currently plan to use them together.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, 03 Dec 2013 16:34:33 +0100
Jan Kiszka wrote:
> On 2013-12-03 13:34, Kim Phillips wrote:
> > VFIO supports pass-through of devices to user space - for sake
> > of illustration, say a PCI e1000 device:
> >
> > - the e1000 is first unbound from the PCI e1000 driver via sysfs
> > - the vfio-pci driver is told via new_id that it now handles e1000 devices
> > - the e1000 is explicitly bound to vfio-pci through sysfs
> >
> > However, now we have two drivers in the system that both handle e1000
> > devices. A hotplug event could then occur and it is ambiguous as to which
> > driver will claim the device. The desired semantics is that vfio-pci is
> > only bound to devices by explicit request in sysfs. This patch makes this
> > possible by introducing a sysfs_bind_only flag in struct device_driver.
> >
> > Signed-off-by: Stuart Yoder
> > Signed-off-by: Kim Phillips
> > ---
> > rebased onto 3.13-rc2, and reposted from first submission which
> > recieved no comments:
> >
> > https://lkml.org/lkml/2013/10/11/53
> >
> > drivers/base/dd.c | 5 -
> > include/linux/device.h | 2 ++
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 0605176..b83b16d 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> > void *data)
> > {
> > struct device *dev = data;
> >
> > - if (!driver_match_device(drv, dev))
> > + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> > return 0;
> >
> > return driver_probe_device(drv, dev);
> > @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
> > *data)
> > */
> > int driver_attach(struct device_driver *drv)
> > {
> > + if (drv->sysfs_bind_only)
> > + return 0;
> > +
> > return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> > }
> > EXPORT_SYMBOL_GPL(driver_attach);
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 952b010..ed441d1 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
> > bus_type *bus);
> > * @owner: The module owner.
> > * @mod_name: Used for built-in modules.
> > * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> > + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> > * @of_match_table: The open firmware table.
> > * @acpi_match_table: The ACPI match table.
> > * @probe: Called to query the existence of a specific device,
> > @@ -233,6 +234,7 @@ struct device_driver {
> > const char *mod_name; /* used for built-in modules */
> >
> > bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
> > + bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
> >
> > const struct of_device_id *of_match_table;
> > const struct acpi_device_id *acpi_match_table;
>
> I think I only discussed this with Stuart in person at the KVM Forum:
> Why not deriving the property "sysfs bind only" from the fact that a
> device does wild-card binding? Are there use cases that benefit from
> decoupling both features?
you mean merge the two new flags sysfs_bind_only and platform driver's
match_any_dev into one new single driver flag, right? good question.
I can't think of any, given none of the built-in PCI drivers set ANY_ID
across the {sub}vendor/{sub}device/class{_mask} board for boot-time (or
hotplug) binding. Technically it's possible with the pci_stub driver
by setting its 'ids' module parameter in the kernel command line, but
looking at the commit that adds 'ids', it was only meant to be used to
"prevent built-in drivers from attaching to specific devices," so it's
unlikely anyone would have it set to ANY_ID.
I'll look at what changes are necessary, barring anyone else coming up
with a valid use-case.
Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Tue, 03 Dec 2013 16:34:33 +0100
Jan Kiszka jan.kis...@siemens.com wrote:
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000 devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
*data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
bus_type *bus);
* @owner: The module owner.
* @mod_name: Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
you mean merge the two new flags sysfs_bind_only and platform driver's
match_any_dev into one new single driver flag, right? good question.
I can't think of any, given none of the built-in PCI drivers set ANY_ID
across the {sub}vendor/{sub}device/class{_mask} board for boot-time (or
hotplug) binding. Technically it's possible with the pci_stub driver
by setting its 'ids' module parameter in the kernel command line, but
looking at the commit that adds 'ids', it was only meant to be used to
prevent built-in drivers from attaching to specific devices, so it's
unlikely anyone would have it set to ANY_ID.
I'll look at what changes are necessary, barring anyone else coming up
with a valid use-case.
Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On Thu, 2013-12-05 at 17:45 +, Kim Phillips wrote:
On Tue, 03 Dec 2013 16:34:33 +0100
Jan Kiszka jan.kis...@siemens.com wrote:
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000 devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void
*data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct
bus_type *bus);
* @owner: The module owner.
* @mod_name:Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
you mean merge the two new flags sysfs_bind_only and platform driver's
match_any_dev into one new single driver flag, right? good question.
What would combining them solve, other than making it more likely that
Greg complains about the wildcard because it would no longer be handled
at the bus level where all the other matching goes on?
They are logically separate things. That doesn't change just because we
currently plan to use them together.
-Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On 2013-12-03 13:34, Kim Phillips wrote:
> VFIO supports pass-through of devices to user space - for sake
> of illustration, say a PCI e1000 device:
>
> - the e1000 is first unbound from the PCI e1000 driver via sysfs
> - the vfio-pci driver is told via new_id that it now handles e1000 devices
> - the e1000 is explicitly bound to vfio-pci through sysfs
>
> However, now we have two drivers in the system that both handle e1000
> devices. A hotplug event could then occur and it is ambiguous as to which
> driver will claim the device. The desired semantics is that vfio-pci is
> only bound to devices by explicit request in sysfs. This patch makes this
> possible by introducing a sysfs_bind_only flag in struct device_driver.
>
> Signed-off-by: Stuart Yoder
> Signed-off-by: Kim Phillips
> ---
> rebased onto 3.13-rc2, and reposted from first submission which
> recieved no comments:
>
> https://lkml.org/lkml/2013/10/11/53
>
> drivers/base/dd.c | 5 -
> include/linux/device.h | 2 ++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0605176..b83b16d 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
> void *data)
> {
> struct device *dev = data;
>
> - if (!driver_match_device(drv, dev))
> + if (drv->sysfs_bind_only || !driver_match_device(drv, dev))
> return 0;
>
> return driver_probe_device(drv, dev);
> @@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void *data)
> */
> int driver_attach(struct device_driver *drv)
> {
> + if (drv->sysfs_bind_only)
> + return 0;
> +
> return bus_for_each_dev(drv->bus, NULL, drv, __driver_attach);
> }
> EXPORT_SYMBOL_GPL(driver_attach);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 952b010..ed441d1 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct bus_type
> *bus);
> * @owner: The module owner.
> * @mod_name:Used for built-in modules.
> * @suppress_bind_attrs: Disables bind/unbind via sysfs.
> + * @sysfs_bind_only: Only allow bind/unbind via sysfs.
> * @of_match_table: The open firmware table.
> * @acpi_match_table: The ACPI match table.
> * @probe: Called to query the existence of a specific device,
> @@ -233,6 +234,7 @@ struct device_driver {
> const char *mod_name; /* used for built-in modules */
>
> bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
> + bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
>
> const struct of_device_id *of_match_table;
> const struct acpi_device_id *acpi_match_table;
>
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property "sysfs bind only" from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
On 2013-12-03 13:34, Kim Phillips wrote:
VFIO supports pass-through of devices to user space - for sake
of illustration, say a PCI e1000 device:
- the e1000 is first unbound from the PCI e1000 driver via sysfs
- the vfio-pci driver is told via new_id that it now handles e1000 devices
- the e1000 is explicitly bound to vfio-pci through sysfs
However, now we have two drivers in the system that both handle e1000
devices. A hotplug event could then occur and it is ambiguous as to which
driver will claim the device. The desired semantics is that vfio-pci is
only bound to devices by explicit request in sysfs. This patch makes this
possible by introducing a sysfs_bind_only flag in struct device_driver.
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@linaro.org
---
rebased onto 3.13-rc2, and reposted from first submission which
recieved no comments:
https://lkml.org/lkml/2013/10/11/53
drivers/base/dd.c | 5 -
include/linux/device.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..b83b16d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -389,7 +389,7 @@ static int __device_attach(struct device_driver *drv,
void *data)
{
struct device *dev = data;
- if (!driver_match_device(drv, dev))
+ if (drv-sysfs_bind_only || !driver_match_device(drv, dev))
return 0;
return driver_probe_device(drv, dev);
@@ -476,6 +476,9 @@ static int __driver_attach(struct device *dev, void *data)
*/
int driver_attach(struct device_driver *drv)
{
+ if (drv-sysfs_bind_only)
+ return 0;
+
return bus_for_each_dev(drv-bus, NULL, drv, __driver_attach);
}
EXPORT_SYMBOL_GPL(driver_attach);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ed441d1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,6 +200,7 @@ extern struct klist *bus_get_device_klist(struct bus_type
*bus);
* @owner: The module owner.
* @mod_name:Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @sysfs_bind_only: Only allow bind/unbind via sysfs.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -233,6 +234,7 @@ struct device_driver {
const char *mod_name; /* used for built-in modules */
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
+ bool sysfs_bind_only; /* only allow bind/unbind via sysfs */
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
I think I only discussed this with Stuart in person at the KVM Forum:
Why not deriving the property sysfs bind only from the fact that a
device does wild-card binding? Are there use cases that benefit from
decoupling both features?
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
