Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 09:54 -0600, Alex Williamson wrote: > On Thu, 2014-10-23 at 18:00 +0300, Marcel Apfelbaum wrote: > > On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: > > > On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: > > > > On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: > > > > > On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > > > > > > > > > > > -Original Message- > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > Sent: Wednesday, October 22, 2014 1:33 PM > > > > > > > To: Marcel Apfelbaum > > > > > > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > > > > > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > > > > > > m...@redhat.com; Yoder Stuart-B08248 > > > > > > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > > > > > > devid<->driver mapping. > > > > > > > > > > > > > > [cc+ stuart] > > > > > > > > > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > > > > > Scanning a lot of devices during boot requires a lot of time. > > > > > > > > On other scenarios there is a need to bind a driver to a > > > > > > > > specific slot. > > > > > > > > > > > > > > > > Binding devices to pci-stub driver does not work, > > > > > > > > as it will not differentiate between devices of the > > > > > > > > same type. Using some start scripts is error prone. > > > > > > > > > > > > > > > > The solution leverages driver_override functionality introduced > > > > > > > > by > > > > > > > > > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > > > > > Author: Alex Williamson > > > > > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > > > > > > > > > PCI: Introduce new device binding path using > > > > > > > > pci_dev.driver_override > > > > > > > > > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > > > > > > > > > > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > > > > > --- > > > > > > > > v3 -> v4: > > > > > > > > - Addressed Alex Williamson's comments: > > > > > > > >- Modified the type of driver_override_entry's fields > > > > > > > >- Used PCI_DEVFN when appropriated > > > > > > > >- Removed redundant checks > > > > > > > >- Replaced BUG_ON with pr_err messages > > > > > > > >- Simpler command line parsing > > > > > > > > - Addressed Michael S. Tsirkin comments > > > > > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > > > > > v2 -> v3: > > > > > > > > - Corrected subject line > > > > > > > > v1 -> v2: > > > > > > > > - Addressed Michael S. Tsirkin comments > > > > > > > >- Removed 32 slots limitation > > > > > > > >- Better handling of memory allocation failures > > > > > > > > (preferred BUG_ON over error messages) > > > > > > > > - Addressed Alex Williamson's comments: > > > > > > > >- Modified commit message to show parameter usage more clear. > > > > > > > > - I preferred to re-use parse_args instead of manually using > > > > > > > >strstr in order to better comply with command line parsing > > > > > > > >rules. > > > > > > > > - I didn't use any locking when parsing the command line args > > > > > > > >(see parse_done usage) assuming that first call will be > > > > > > > >early in system boot and no race can
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 18:00 +0300, Marcel Apfelbaum wrote: > On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: > > On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: > > > On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: > > > > On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > > > > > > > > > -Original Message- > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > Sent: Wednesday, October 22, 2014 1:33 PM > > > > > > To: Marcel Apfelbaum > > > > > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > > > > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > > > > > m...@redhat.com; Yoder Stuart-B08248 > > > > > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > > > > > devid<->driver mapping. > > > > > > > > > > > > [cc+ stuart] > > > > > > > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > > > > Scanning a lot of devices during boot requires a lot of time. > > > > > > > On other scenarios there is a need to bind a driver to a specific > > > > > > > slot. > > > > > > > > > > > > > > Binding devices to pci-stub driver does not work, > > > > > > > as it will not differentiate between devices of the > > > > > > > same type. Using some start scripts is error prone. > > > > > > > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > > > > Author: Alex Williamson > > > > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > > > > > > > PCI: Introduce new device binding path using > > > > > > > pci_dev.driver_override > > > > > > > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > > > > --- > > > > > > > v3 -> v4: > > > > > > > - Addressed Alex Williamson's comments: > > > > > > >- Modified the type of driver_override_entry's fields > > > > > > >- Used PCI_DEVFN when appropriated > > > > > > >- Removed redundant checks > > > > > > >- Replaced BUG_ON with pr_err messages > > > > > > >- Simpler command line parsing > > > > > > > - Addressed Michael S. Tsirkin comments > > > > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > > > > v2 -> v3: > > > > > > > - Corrected subject line > > > > > > > v1 -> v2: > > > > > > > - Addressed Michael S. Tsirkin comments > > > > > > >- Removed 32 slots limitation > > > > > > >- Better handling of memory allocation failures > > > > > > > (preferred BUG_ON over error messages) > > > > > > > - Addressed Alex Williamson's comments: > > > > > > >- Modified commit message to show parameter usage more clear. > > > > > > > - I preferred to re-use parse_args instead of manually using > > > > > > >strstr in order to better comply with command line parsing > > > > > > >rules. > > > > > > > - I didn't use any locking when parsing the command line args > > > > > > >(see parse_done usage) assuming that first call will be > > > > > > >early in system boot and no race can occur. Please correct > > > > > > >me if I am wrong. > > > > > > > > > > > > > > Notes: > > > > > > > - I have further ideas on top of this patch based on your > > > > > > > reviews. > > > > > > >I thought of: > > > > > > >- Use wildcards to specify entire buses/devices, something > > > > > > > like: > > >
RE: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
> -Original Message- > From: Marcel Apfelbaum [mailto:marce...@redhat.com] > Sent: Thursday, October 23, 2014 8:37 AM > To: Alex Williamson > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > linux-kernel@vger.kernel.org; mar...@redhat.com; > m...@redhat.com; Yoder Stuart-B08248 > Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver > mapping. > > On Thu, 2014-10-23 at 07:11 -0600, Alex Williamson wrote: > > On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote: > > > On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: > > > > [cc+ stuart] > > > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > > Scanning a lot of devices during boot requires a lot of time. > > > > > On other scenarios there is a need to bind a driver to a specific > > > > > slot. > > > > > > > > > > Binding devices to pci-stub driver does not work, > > > > > as it will not differentiate between devices of the > > > > > same type. Using some start scripts is error prone. > > > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > > Author: Alex Williamson > > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > > > PCI: Introduce new device binding path using > > > > > pci_dev.driver_override > > > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > > --- > > > > > v3 -> v4: > > > > > - Addressed Alex Williamson's comments: > > > > >- Modified the type of driver_override_entry's fields > > > > >- Used PCI_DEVFN when appropriated > > > > >- Removed redundant checks > > > > >- Replaced BUG_ON with pr_err messages > > > > >- Simpler command line parsing > > > > > - Addressed Michael S. Tsirkin comments > > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > > v2 -> v3: > > > > > - Corrected subject line > > > > > v1 -> v2: > > > > > - Addressed Michael S. Tsirkin comments > > > > >- Removed 32 slots limitation > > > > >- Better handling of memory allocation failures > > > > > (preferred BUG_ON over error messages) > > > > > - Addressed Alex Williamson's comments: > > > > >- Modified commit message to show parameter usage more clear. > > > > > - I preferred to re-use parse_args instead of manually using > > > > >strstr in order to better comply with command line parsing > > > > >rules. > > > > > - I didn't use any locking when parsing the command line args > > > > >(see parse_done usage) assuming that first call will be > > > > >early in system boot and no race can occur. Please correct > > > > >me if I am wrong. > > > > > > > > > > Notes: > > > > > - I have further ideas on top of this patch based on your reviews. > > > > >I thought of: > > > > >- Use wildcards to specify entire buses/devices, something like: > > > > > driver[0001:02:*.*]=pci-stub > > > > >- Use comma to separate several devices: > > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > > >- Make domain optional: > > > > > driver[00:03.0]=pci-stub > > > > > > > > > > Comments will be appreciated, > > > > > Thanks, > > > > > Marcel > > > > > Documentation/kernel-parameters.txt | 4 ++ > > > > > drivers/pci/bus.c | 111 > > > > > > > > > > drivers/pci/pci.c | 2 + > > > > > 3 files changed, 117 insertions(+) > > > > > > > > The driver_override feature that we're making use of here is also going > > > > to be supported by platform devices and potentially more bustypes in the > > > > future, so I'm concerned
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: > On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: > > On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: > > > On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > > > > > > > -Original Message- > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > Sent: Wednesday, October 22, 2014 1:33 PM > > > > > To: Marcel Apfelbaum > > > > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > > > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > > > > m...@redhat.com; Yoder Stuart-B08248 > > > > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > > > > devid<->driver mapping. > > > > > > > > > > [cc+ stuart] > > > > > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > > > Scanning a lot of devices during boot requires a lot of time. > > > > > > On other scenarios there is a need to bind a driver to a specific > > > > > > slot. > > > > > > > > > > > > Binding devices to pci-stub driver does not work, > > > > > > as it will not differentiate between devices of the > > > > > > same type. Using some start scripts is error prone. > > > > > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > > > Author: Alex Williamson > > > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > > > > > PCI: Introduce new device binding path using > > > > > > pci_dev.driver_override > > > > > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > > > --- > > > > > > v3 -> v4: > > > > > > - Addressed Alex Williamson's comments: > > > > > >- Modified the type of driver_override_entry's fields > > > > > >- Used PCI_DEVFN when appropriated > > > > > >- Removed redundant checks > > > > > >- Replaced BUG_ON with pr_err messages > > > > > >- Simpler command line parsing > > > > > > - Addressed Michael S. Tsirkin comments > > > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > > > v2 -> v3: > > > > > > - Corrected subject line > > > > > > v1 -> v2: > > > > > > - Addressed Michael S. Tsirkin comments > > > > > >- Removed 32 slots limitation > > > > > >- Better handling of memory allocation failures > > > > > > (preferred BUG_ON over error messages) > > > > > > - Addressed Alex Williamson's comments: > > > > > >- Modified commit message to show parameter usage more clear. > > > > > > - I preferred to re-use parse_args instead of manually using > > > > > >strstr in order to better comply with command line parsing > > > > > >rules. > > > > > > - I didn't use any locking when parsing the command line args > > > > > >(see parse_done usage) assuming that first call will be > > > > > >early in system boot and no race can occur. Please correct > > > > > >me if I am wrong. > > > > > > > > > > > > Notes: > > > > > > - I have further ideas on top of this patch based on your reviews. > > > > > >I thought of: > > > > > >- Use wildcards to specify entire buses/devices, something like: > > > > > > driver[0001:02:*.*]=pci-stub > > > > > >- Use comma to separate several devices: > > > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > > > >- Make domain optional: > > > > > > driver[00:03.0]=pci-stub > > > > > > > > > > > > Comments will be appreciated, > > > > > > Thank
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: > On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: > > On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > > > > > -Original Message- > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > Sent: Wednesday, October 22, 2014 1:33 PM > > > > To: Marcel Apfelbaum > > > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > > > m...@redhat.com; Yoder Stuart-B08248 > > > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > > > devid<->driver mapping. > > > > > > > > [cc+ stuart] > > > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > > Scanning a lot of devices during boot requires a lot of time. > > > > > On other scenarios there is a need to bind a driver to a specific > > > > > slot. > > > > > > > > > > Binding devices to pci-stub driver does not work, > > > > > as it will not differentiate between devices of the > > > > > same type. Using some start scripts is error prone. > > > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > > Author: Alex Williamson > > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > > > PCI: Introduce new device binding path using > > > > > pci_dev.driver_override > > > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > > --- > > > > > v3 -> v4: > > > > > - Addressed Alex Williamson's comments: > > > > >- Modified the type of driver_override_entry's fields > > > > >- Used PCI_DEVFN when appropriated > > > > >- Removed redundant checks > > > > >- Replaced BUG_ON with pr_err messages > > > > >- Simpler command line parsing > > > > > - Addressed Michael S. Tsirkin comments > > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > > v2 -> v3: > > > > > - Corrected subject line > > > > > v1 -> v2: > > > > > - Addressed Michael S. Tsirkin comments > > > > >- Removed 32 slots limitation > > > > >- Better handling of memory allocation failures > > > > > (preferred BUG_ON over error messages) > > > > > - Addressed Alex Williamson's comments: > > > > >- Modified commit message to show parameter usage more clear. > > > > > - I preferred to re-use parse_args instead of manually using > > > > >strstr in order to better comply with command line parsing > > > > >rules. > > > > > - I didn't use any locking when parsing the command line args > > > > >(see parse_done usage) assuming that first call will be > > > > >early in system boot and no race can occur. Please correct > > > > >me if I am wrong. > > > > > > > > > > Notes: > > > > > - I have further ideas on top of this patch based on your reviews. > > > > >I thought of: > > > > >- Use wildcards to specify entire buses/devices, something like: > > > > > driver[0001:02:*.*]=pci-stub > > > > >- Use comma to separate several devices: > > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > > >- Make domain optional: > > > > > driver[00:03.0]=pci-stub > > > > > > > > > > Comments will be appreciated, > > > > > Thanks, > > > > > Marcel > > > > > Documentation/kernel-parameters.txt | 4 ++ > > > > > drivers/pci/bus.c | 111 > > > > > > > > > > drivers/pci/pci.c | 2 + > > > > > 3 files changed, 117 insertions(+) > > > > > > > > The driver_override feature that we're making use of
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: > On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > > > -Original Message- > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > Sent: Wednesday, October 22, 2014 1:33 PM > > > To: Marcel Apfelbaum > > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > > m...@redhat.com; Yoder Stuart-B08248 > > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > > devid<->driver mapping. > > > > > > [cc+ stuart] > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > Scanning a lot of devices during boot requires a lot of time. > > > > On other scenarios there is a need to bind a driver to a specific slot. > > > > > > > > Binding devices to pci-stub driver does not work, > > > > as it will not differentiate between devices of the > > > > same type. Using some start scripts is error prone. > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > Author: Alex Williamson > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > PCI: Introduce new device binding path using > > > > pci_dev.driver_override > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > --- > > > > v3 -> v4: > > > > - Addressed Alex Williamson's comments: > > > >- Modified the type of driver_override_entry's fields > > > >- Used PCI_DEVFN when appropriated > > > >- Removed redundant checks > > > >- Replaced BUG_ON with pr_err messages > > > >- Simpler command line parsing > > > > - Addressed Michael S. Tsirkin comments > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > v2 -> v3: > > > > - Corrected subject line > > > > v1 -> v2: > > > > - Addressed Michael S. Tsirkin comments > > > >- Removed 32 slots limitation > > > >- Better handling of memory allocation failures > > > > (preferred BUG_ON over error messages) > > > > - Addressed Alex Williamson's comments: > > > >- Modified commit message to show parameter usage more clear. > > > > - I preferred to re-use parse_args instead of manually using > > > >strstr in order to better comply with command line parsing > > > >rules. > > > > - I didn't use any locking when parsing the command line args > > > >(see parse_done usage) assuming that first call will be > > > >early in system boot and no race can occur. Please correct > > > >me if I am wrong. > > > > > > > > Notes: > > > > - I have further ideas on top of this patch based on your reviews. > > > >I thought of: > > > >- Use wildcards to specify entire buses/devices, something like: > > > > driver[0001:02:*.*]=pci-stub > > > >- Use comma to separate several devices: > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > >- Make domain optional: > > > > driver[00:03.0]=pci-stub > > > > > > > > Comments will be appreciated, > > > > Thanks, > > > > Marcel > > > > Documentation/kernel-parameters.txt | 4 ++ > > > > drivers/pci/bus.c | 111 > > > > > > > > drivers/pci/pci.c | 2 + > > > > 3 files changed, 117 insertions(+) > > > > > > The driver_override feature that we're making use of here is also going > > > to be supported by platform devices and potentially more bustypes in the > > > future, so I'm concerned that making a pci specific kernel parameter is > > > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for > > > bustypes that support driver_override so we can have a common interface. > > > Perhaps: > > > > > > driver_override=pci,:02:00.0=pci-stub;platform,fakenam
RE: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
> -Original Message- > From: Marcel Apfelbaum [mailto:marce...@redhat.com] > Sent: Thursday, October 23, 2014 7:32 AM > To: Alex Williamson > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > linux-kernel@vger.kernel.org; mar...@redhat.com; > m...@redhat.com; Yoder Stuart-B08248 > Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver > mapping. > > On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: > > [cc+ stuart] > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > Scanning a lot of devices during boot requires a lot of time. > > > On other scenarios there is a need to bind a driver to a specific slot. > > > > > > Binding devices to pci-stub driver does not work, > > > as it will not differentiate between devices of the > > > same type. Using some start scripts is error prone. > > > > > > The solution leverages driver_override functionality introduced by > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > Author: Alex Williamson > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > PCI: Introduce new device binding path using > > > pci_dev.driver_override > > > > > > In order to bind PCI slots to specific drivers use: > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > Signed-off-by: Marcel Apfelbaum > > > --- > > > v3 -> v4: > > > - Addressed Alex Williamson's comments: > > >- Modified the type of driver_override_entry's fields > > >- Used PCI_DEVFN when appropriated > > >- Removed redundant checks > > >- Replaced BUG_ON with pr_err messages > > >- Simpler command line parsing > > > - Addressed Michael S. Tsirkin comments > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > v2 -> v3: > > > - Corrected subject line > > > v1 -> v2: > > > - Addressed Michael S. Tsirkin comments > > >- Removed 32 slots limitation > > >- Better handling of memory allocation failures > > > (preferred BUG_ON over error messages) > > > - Addressed Alex Williamson's comments: > > >- Modified commit message to show parameter usage more clear. > > > - I preferred to re-use parse_args instead of manually using > > >strstr in order to better comply with command line parsing > > >rules. > > > - I didn't use any locking when parsing the command line args > > >(see parse_done usage) assuming that first call will be > > >early in system boot and no race can occur. Please correct > > >me if I am wrong. > > > > > > Notes: > > > - I have further ideas on top of this patch based on your reviews. > > >I thought of: > > >- Use wildcards to specify entire buses/devices, something like: > > > driver[0001:02:*.*]=pci-stub > > >- Use comma to separate several devices: > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > >- Make domain optional: > > > driver[00:03.0]=pci-stub > > > > > > Comments will be appreciated, > > > Thanks, > > > Marcel > > > Documentation/kernel-parameters.txt | 4 ++ > > > drivers/pci/bus.c | 111 > > > > > > drivers/pci/pci.c | 2 + > > > 3 files changed, 117 insertions(+) > > > > The driver_override feature that we're making use of here is also going > > to be supported by platform devices and potentially more bustypes in the > > future, so I'm concerned that making a pci specific kernel parameter is > > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for > > bustypes that support driver_override so we can have a common interface. > The real question here if those bus types/devices would benefit from this > feature, and I also must confess that I have no knowledge of the other buses. > Can anyone confirm that it does make sense for them? We don't have vfio-platform in use yet, so not much actual real world user experience yet. But, I think it makes sense. Especially, given that we are inventing a kernel parameter I think we should design the syntax so that it can work buses can implement support for this. The driver_override mechanism is not bus specific, so let's not make the kernel parameter bus specific. > > Perhaps: > > > > driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform > > > > Finding delimiters that don't conflict may be challenging. Also, can we > > assume that bus-name:dev-name is unique for every bustype? It is for > > pci, platform? > For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea, > can anyone that knows "platform" confirm or deny? Yes, dev-name will be unique. All platform devices are under a single platform bus. Stuart
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: > > > -Original Message- > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: Wednesday, October 22, 2014 1:33 PM > > To: Marcel Apfelbaum > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > m...@redhat.com; Yoder Stuart-B08248 > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > devid<->driver mapping. > > > > [cc+ stuart] > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > Scanning a lot of devices during boot requires a lot of time. > > > On other scenarios there is a need to bind a driver to a specific slot. > > > > > > Binding devices to pci-stub driver does not work, > > > as it will not differentiate between devices of the > > > same type. Using some start scripts is error prone. > > > > > > The solution leverages driver_override functionality introduced by > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > Author: Alex Williamson > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > PCI: Introduce new device binding path using > > > pci_dev.driver_override > > > > > > In order to bind PCI slots to specific drivers use: > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > Signed-off-by: Marcel Apfelbaum > > > --- > > > v3 -> v4: > > > - Addressed Alex Williamson's comments: > > >- Modified the type of driver_override_entry's fields > > >- Used PCI_DEVFN when appropriated > > >- Removed redundant checks > > >- Replaced BUG_ON with pr_err messages > > >- Simpler command line parsing > > > - Addressed Michael S. Tsirkin comments > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > v2 -> v3: > > > - Corrected subject line > > > v1 -> v2: > > > - Addressed Michael S. Tsirkin comments > > >- Removed 32 slots limitation > > >- Better handling of memory allocation failures > > > (preferred BUG_ON over error messages) > > > - Addressed Alex Williamson's comments: > > >- Modified commit message to show parameter usage more clear. > > > - I preferred to re-use parse_args instead of manually using > > >strstr in order to better comply with command line parsing > > >rules. > > > - I didn't use any locking when parsing the command line args > > >(see parse_done usage) assuming that first call will be > > >early in system boot and no race can occur. Please correct > > >me if I am wrong. > > > > > > Notes: > > > - I have further ideas on top of this patch based on your reviews. > > >I thought of: > > >- Use wildcards to specify entire buses/devices, something like: > > > driver[0001:02:*.*]=pci-stub > > >- Use comma to separate several devices: > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > >- Make domain optional: > > > driver[00:03.0]=pci-stub > > > > > > Comments will be appreciated, > > > Thanks, > > > Marcel > > > Documentation/kernel-parameters.txt | 4 ++ > > > drivers/pci/bus.c | 111 > > > > > > drivers/pci/pci.c | 2 + > > > 3 files changed, 117 insertions(+) > > > > The driver_override feature that we're making use of here is also going > > to be supported by platform devices and potentially more bustypes in the > > future, so I'm concerned that making a pci specific kernel parameter is > > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for > > bustypes that support driver_override so we can have a common interface. > > Perhaps: > > > > driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform > > > > Finding delimiters that don't conflict may be challenging. > > I think what you proposed works-- ,=; > > Think that will work for PCI, platform, and the new fsl-mc bus we are working > on. > > > Also, can we > > assume that bus-name:dev-name is unique for every bustype? It is for > > pci, platform? > > I think that has to be the case. > > > It also seems like there's a question of how long should this override > &g
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 13:51 +, Stuart Yoder wrote: > > > -Original Message- > > From: Marcel Apfelbaum [mailto:marce...@redhat.com] > > Sent: Thursday, October 23, 2014 7:32 AM > > To: Alex Williamson > > Cc: linux-...@vger.kernel.org; bhelg...@google.com; > > linux-kernel@vger.kernel.org; mar...@redhat.com; > > m...@redhat.com; Yoder Stuart-B08248 > > Subject: Re: [PATCH v4] PCI: add kernel parameter to override > > devid<->driver mapping. > > > > On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: > > > [cc+ stuart] > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > Scanning a lot of devices during boot requires a lot of time. > > > > On other scenarios there is a need to bind a driver to a specific slot. > > > > > > > > Binding devices to pci-stub driver does not work, > > > > as it will not differentiate between devices of the > > > > same type. Using some start scripts is error prone. > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > Author: Alex Williamson > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > PCI: Introduce new device binding path using > > > > pci_dev.driver_override > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > --- > > > > v3 -> v4: > > > > - Addressed Alex Williamson's comments: > > > >- Modified the type of driver_override_entry's fields > > > >- Used PCI_DEVFN when appropriated > > > >- Removed redundant checks > > > >- Replaced BUG_ON with pr_err messages > > > >- Simpler command line parsing > > > > - Addressed Michael S. Tsirkin comments > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > v2 -> v3: > > > > - Corrected subject line > > > > v1 -> v2: > > > > - Addressed Michael S. Tsirkin comments > > > >- Removed 32 slots limitation > > > >- Better handling of memory allocation failures > > > > (preferred BUG_ON over error messages) > > > > - Addressed Alex Williamson's comments: > > > >- Modified commit message to show parameter usage more clear. > > > > - I preferred to re-use parse_args instead of manually using > > > >strstr in order to better comply with command line parsing > > > >rules. > > > > - I didn't use any locking when parsing the command line args > > > >(see parse_done usage) assuming that first call will be > > > >early in system boot and no race can occur. Please correct > > > >me if I am wrong. > > > > > > > > Notes: > > > > - I have further ideas on top of this patch based on your reviews. > > > >I thought of: > > > >- Use wildcards to specify entire buses/devices, something like: > > > > driver[0001:02:*.*]=pci-stub > > > >- Use comma to separate several devices: > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > >- Make domain optional: > > > > driver[00:03.0]=pci-stub > > > > > > > > Comments will be appreciated, > > > > Thanks, > > > > Marcel > > > > Documentation/kernel-parameters.txt | 4 ++ > > > > drivers/pci/bus.c | 111 > > > > > > > > drivers/pci/pci.c | 2 + > > > > 3 files changed, 117 insertions(+) > > > > > > The driver_override feature that we're making use of here is also going > > > to be supported by platform devices and potentially more bustypes in the > > > future, so I'm concerned that making a pci specific kernel parameter is > > > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for > > > bustypes that support driver_override so we can have a common interface. > > The real question here if those bus types/devices would benefit from this > > feature, and I also must confess that I have no knowledge of the other > >
RE: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, October 22, 2014 1:33 PM
> To: Marcel Apfelbaum
> Cc: linux-...@vger.kernel.org; bhelg...@google.com;
> linux-kernel@vger.kernel.org; mar...@redhat.com;
> m...@redhat.com; Yoder Stuart-B08248
> Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver
> mapping.
>
> [cc+ stuart]
>
> On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
> > Scanning a lot of devices during boot requires a lot of time.
> > On other scenarios there is a need to bind a driver to a specific slot.
> >
> > Binding devices to pci-stub driver does not work,
> > as it will not differentiate between devices of the
> > same type. Using some start scripts is error prone.
> >
> > The solution leverages driver_override functionality introduced by
> >
> > commit: 782a985d7af26db39e86070d28f987cad21313c0
> > Author: Alex Williamson
> > Date: Tue May 20 08:53:21 2014 -0600
> >
> > PCI: Introduce new device binding path using
> > pci_dev.driver_override
> >
> > In order to bind PCI slots to specific drivers use:
> > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
> >
> > Signed-off-by: Marcel Apfelbaum
> > ---
> > v3 -> v4:
> > - Addressed Alex Williamson's comments:
> >- Modified the type of driver_override_entry's fields
> >- Used PCI_DEVFN when appropriated
> >- Removed redundant checks
> >- Replaced BUG_ON with pr_err messages
> >- Simpler command line parsing
> > - Addressed Michael S. Tsirkin comments
> >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
> > v2 -> v3:
> > - Corrected subject line
> > v1 -> v2:
> > - Addressed Michael S. Tsirkin comments
> >- Removed 32 slots limitation
> >- Better handling of memory allocation failures
> > (preferred BUG_ON over error messages)
> > - Addressed Alex Williamson's comments:
> >- Modified commit message to show parameter usage more clear.
> > - I preferred to re-use parse_args instead of manually using
> >strstr in order to better comply with command line parsing
> >rules.
> > - I didn't use any locking when parsing the command line args
> >(see parse_done usage) assuming that first call will be
> >early in system boot and no race can occur. Please correct
> >me if I am wrong.
> >
> > Notes:
> > - I have further ideas on top of this patch based on your reviews.
> >I thought of:
> >- Use wildcards to specify entire buses/devices, something like:
> > driver[0001:02:*.*]=pci-stub
> >- Use comma to separate several devices:
> > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
> >- Make domain optional:
> > driver[00:03.0]=pci-stub
> >
> > Comments will be appreciated,
> > Thanks,
> > Marcel
> > Documentation/kernel-parameters.txt | 4 ++
> > drivers/pci/bus.c | 111
> >
> > drivers/pci/pci.c | 2 +
> > 3 files changed, 117 insertions(+)
>
> The driver_override feature that we're making use of here is also going
> to be supported by platform devices and potentially more bustypes in the
> future, so I'm concerned that making a pci specific kernel parameter is
> too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
> bustypes that support driver_override so we can have a common interface.
> Perhaps:
>
> driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
>
> Finding delimiters that don't conflict may be challenging.
I think what you proposed works-- ,=;
Think that will work for PCI, platform, and the new fsl-mc bus we are working
on.
> Also, can we
> assume that bus-name:dev-name is unique for every bustype? It is for
> pci, platform?
I think that has to be the case.
> It also seems like there's a question of how long should this override
> last and how does the user disable it?
Isn't that a general question for the "driver_overrride" mechanism?
I'm forgetting if the mechanism in the kernel now has a way to disable
it-- e.g. echo /dev/null > /sys/pci/devices/.../driver_override ??
So, it would last until explicitly disabled through sysfs.
> I think with pci-stub.ids=
> $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
> entry to cancel the effect. The only option here seems to be a reboot.
> Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
> this interface? Thanks,
Thanks,
Stuart
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 07:11 -0600, Alex Williamson wrote: > On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote: > > On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: > > > [cc+ stuart] > > > > > > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: > > > > Scanning a lot of devices during boot requires a lot of time. > > > > On other scenarios there is a need to bind a driver to a specific slot. > > > > > > > > Binding devices to pci-stub driver does not work, > > > > as it will not differentiate between devices of the > > > > same type. Using some start scripts is error prone. > > > > > > > > The solution leverages driver_override functionality introduced by > > > > > > > > commit: 782a985d7af26db39e86070d28f987cad21313c0 > > > > Author: Alex Williamson > > > > Date: Tue May 20 08:53:21 2014 -0600 > > > > > > > > PCI: Introduce new device binding path using > > > > pci_dev.driver_override > > > > > > > > In order to bind PCI slots to specific drivers use: > > > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... > > > > > > > > Signed-off-by: Marcel Apfelbaum > > > > --- > > > > v3 -> v4: > > > > - Addressed Alex Williamson's comments: > > > >- Modified the type of driver_override_entry's fields > > > >- Used PCI_DEVFN when appropriated > > > >- Removed redundant checks > > > >- Replaced BUG_ON with pr_err messages > > > >- Simpler command line parsing > > > > - Addressed Michael S. Tsirkin comments > > > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation > > > > v2 -> v3: > > > > - Corrected subject line > > > > v1 -> v2: > > > > - Addressed Michael S. Tsirkin comments > > > >- Removed 32 slots limitation > > > >- Better handling of memory allocation failures > > > > (preferred BUG_ON over error messages) > > > > - Addressed Alex Williamson's comments: > > > >- Modified commit message to show parameter usage more clear. > > > > - I preferred to re-use parse_args instead of manually using > > > >strstr in order to better comply with command line parsing > > > >rules. > > > > - I didn't use any locking when parsing the command line args > > > >(see parse_done usage) assuming that first call will be > > > >early in system boot and no race can occur. Please correct > > > >me if I am wrong. > > > > > > > > Notes: > > > > - I have further ideas on top of this patch based on your reviews. > > > >I thought of: > > > >- Use wildcards to specify entire buses/devices, something like: > > > > driver[0001:02:*.*]=pci-stub > > > >- Use comma to separate several devices: > > > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub > > > >- Make domain optional: > > > > driver[00:03.0]=pci-stub > > > > > > > > Comments will be appreciated, > > > > Thanks, > > > > Marcel > > > > Documentation/kernel-parameters.txt | 4 ++ > > > > drivers/pci/bus.c | 111 > > > > > > > > drivers/pci/pci.c | 2 + > > > > 3 files changed, 117 insertions(+) > > > > > > The driver_override feature that we're making use of here is also going > > > to be supported by platform devices and potentially more bustypes in the > > > future, so I'm concerned that making a pci specific kernel parameter is > > > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for > > > bustypes that support driver_override so we can have a common interface. > > The real question here if those bus types/devices would benefit from this > > feature, and I also must confess that I have no knowledge of the other > > buses. > > Can anyone confirm that it does make sense for them? > > Platform devices are adding vfio support, so I expect the next logical > question will be how to reserve devices for use by vfio at boot. Well, I'll have to learn more about vfio before saying anything, but my question is if it can be deferred or it has to be part of this patch. If the platform devices do not have a slot like hw address, maybe it can be configured separately. I saw this patch as a PCI patch, and not "driver_override" expansion; meaning that I only leveraged an existing feature, not trying to extend it. If I am wrong, please point me to a more robust solution. > > > > Perhaps: > > > > > > driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform > > > > > > Finding delimiters that don't conflict may be challenging. Also, can we > > > assume that bus-name:dev-name is unique for every bustype? It is for > > > pci, platform? > > For PCI, sure the domain:bus:dev.func is unique, for platform I have no > > idea, > > can anyone that knows "platform" confirm or deny? > > > > > > > > It also seems like there's a question of how long should this override > > > last and how does the user disable it? > > > I think with pci-stub.ids= > > > $VENDOR:$DEVICE a user can echo the IDs
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote:
> On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote:
> > [cc+ stuart]
> >
> > On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
> > > Scanning a lot of devices during boot requires a lot of time.
> > > On other scenarios there is a need to bind a driver to a specific slot.
> > >
> > > Binding devices to pci-stub driver does not work,
> > > as it will not differentiate between devices of the
> > > same type. Using some start scripts is error prone.
> > >
> > > The solution leverages driver_override functionality introduced by
> > >
> > > commit: 782a985d7af26db39e86070d28f987cad21313c0
> > > Author: Alex Williamson
> > > Date: Tue May 20 08:53:21 2014 -0600
> > >
> > > PCI: Introduce new device binding path using
> > > pci_dev.driver_override
> > >
> > > In order to bind PCI slots to specific drivers use:
> > > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
> > >
> > > Signed-off-by: Marcel Apfelbaum
> > > ---
> > > v3 -> v4:
> > > - Addressed Alex Williamson's comments:
> > >- Modified the type of driver_override_entry's fields
> > >- Used PCI_DEVFN when appropriated
> > >- Removed redundant checks
> > >- Replaced BUG_ON with pr_err messages
> > >- Simpler command line parsing
> > > - Addressed Michael S. Tsirkin comments
> > >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
> > > v2 -> v3:
> > > - Corrected subject line
> > > v1 -> v2:
> > > - Addressed Michael S. Tsirkin comments
> > >- Removed 32 slots limitation
> > >- Better handling of memory allocation failures
> > > (preferred BUG_ON over error messages)
> > > - Addressed Alex Williamson's comments:
> > >- Modified commit message to show parameter usage more clear.
> > > - I preferred to re-use parse_args instead of manually using
> > >strstr in order to better comply with command line parsing
> > >rules.
> > > - I didn't use any locking when parsing the command line args
> > >(see parse_done usage) assuming that first call will be
> > >early in system boot and no race can occur. Please correct
> > >me if I am wrong.
> > >
> > > Notes:
> > > - I have further ideas on top of this patch based on your reviews.
> > >I thought of:
> > >- Use wildcards to specify entire buses/devices, something like:
> > > driver[0001:02:*.*]=pci-stub
> > >- Use comma to separate several devices:
> > > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
> > >- Make domain optional:
> > > driver[00:03.0]=pci-stub
> > >
> > > Comments will be appreciated,
> > > Thanks,
> > > Marcel
> > > Documentation/kernel-parameters.txt | 4 ++
> > > drivers/pci/bus.c | 111
> > >
> > > drivers/pci/pci.c | 2 +
> > > 3 files changed, 117 insertions(+)
> >
> > The driver_override feature that we're making use of here is also going
> > to be supported by platform devices and potentially more bustypes in the
> > future, so I'm concerned that making a pci specific kernel parameter is
> > too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
> > bustypes that support driver_override so we can have a common interface.
> The real question here if those bus types/devices would benefit from this
> feature, and I also must confess that I have no knowledge of the other buses.
> Can anyone confirm that it does make sense for them?
Platform devices are adding vfio support, so I expect the next logical
question will be how to reserve devices for use by vfio at boot.
> > Perhaps:
> >
> > driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
> >
> > Finding delimiters that don't conflict may be challenging. Also, can we
> > assume that bus-name:dev-name is unique for every bustype? It is for
> > pci, platform?
> For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea,
> can anyone that knows "platform" confirm or deny?
>
> >
> > It also seems like there's a question of how long should this override
> > last and how does the user disable it?
> > I think with pci-stub.ids=
> > $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
> > entry to cancel the effect.
> The way I see it is simple, the override specified in kernel command line
> last as long as the user does not specifically remove it using
> echo "" > /sys/.../driver_override
> and then unbind and bind the device again.
>
> > The only option here seems to be a reboot.
> Please see above
That's only a temporary removal though, if the device is removed and
re-added, either via physical hotplug or sysfs, the override is
re-applied. Thanks,
Alex
> > Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
> > this interface? Thanks,
> While it does not hurt, I see it as optional since a simple removal of
> driver_override and rebind does
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Wed, Oct 22, 2014 at 03:28:29PM -0600, Bjorn Helgaas wrote:
> Hi Marcel,
>
> I'm not quite clear on what the objective is here, so I apologize for
> some questions that probably seem silly.
>
> On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum wrote:
> > Scanning a lot of devices during boot requires a lot of time.
>
> I think what takes a lot of time is the .probe() method for some
> drivers, right? I first thought you meant that it took a long time
> for the PCI core to enumerate a lot of devices, but you're not
> changing anything there.
>
> If the intent is to reduce boot time, I don't think this is a general
> solution. Drivers should be able to schedule asynchronous things in
> their .probe() methods if necessary.
If this worked for all devices, we could just make probe
asynchronous in PCI core.
Unfortunately this doesn't work esp for storage devices
since people expect disks to be available for mount immediately.
If the point of the patch is to speed up boot, we could
try to probe everything in parallel?
Probe is serialized now, right?
> > On other scenarios there is a need to bind a driver to a specific slot.
>
> A short example here would be good. Are you talking about something
> like binding a NIC driver to one device while leaving others unbound
> for use by guests?
>
> > Binding devices to pci-stub driver does not work,
> > as it will not differentiate between devices of the
> > same type.
>
> I assume you mean booting with "pci-stub.ids=$VENDOR:$DEVICE" will
> make pci-stub bind to *all* matching devices, and you only want it to
> bind to some. Maybe pci-stub could be extended to pay attention to
> PCI bus addresses in addition to vendor/device IDs.
>
> > Using some start scripts is error prone.
> >
> > The solution leverages driver_override functionality introduced by
> >
> > commit: 782a985d7af26db39e86070d28f987cad21313c0
> > Author: Alex Williamson
> > Date: Tue May 20 08:53:21 2014 -0600
> >
> > PCI: Introduce new device binding path using pci_dev.driver_override
> >
> > In order to bind PCI slots to specific drivers use:
> > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
>
> If/when you address Alex's comments about other bus types, can you
> also update the changelog to use the canonical commit reference
> format, i.e., 782a985d7af2 ("PCI: Introduce new device binding path
> using pci_dev.driver_override") in this case?
>
> PCI bus numbers are mutable, e.g., they can change with hotplug or
> other configuration changes. But I don't have any better suggestion,
> so I guess all we can do is be aware of this.
We could use slot capability for addressing if that's available.
> Speaking of hotplug, this is only a boot-time kernel parameter, with
> no opportunity to use this, e.g., to add slot/driver pairs, after
> boot. Do you not need that because of Alex's driver_override thing?
> How can we integrate this all together into a coherent whole? I'm a
> little confused as to how this would all be documented in a form
> usable by end-users.
>
> Bjorn
--
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: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Wed, 2014-10-22 at 15:28 -0600, Bjorn Helgaas wrote:
> Hi Marcel,
Hi Bjorn,
Thank you for the review!
>
> I'm not quite clear on what the objective is here, so I apologize for
> some questions that probably seem silly.
I appreciate you took your time to go over it.
>
> On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum wrote:
> > Scanning a lot of devices during boot requires a lot of time.
>
> I think what takes a lot of time is the .probe() method for some
> drivers, right? I first thought you meant that it took a long time
> for the PCI core to enumerate a lot of devices, but you're not
> changing anything there.
Yes indeed.
>
> If the intent is to reduce boot time, I don't think this is a general
> solution. Drivers should be able to schedule asynchronous things in
> their .probe() methods if necessary.
I agree, but sadly we cannot go over *all* existing drivers and fix,
we can of course do the best effort :)
By the way this was not the only reason as you also thought, see bellow
>
> > On other scenarios there is a need to bind a driver to a specific slot.
>
> A short example here would be good. Are you talking about something
> like binding a NIC driver to one device while leaving others unbound
> for use by guests?
Exactly! This is the "perfect" example, thanks!
>
> > Binding devices to pci-stub driver does not work,
> > as it will not differentiate between devices of the
> > same type.
>
> I assume you mean booting with "pci-stub.ids=$VENDOR:$DEVICE" will
> make pci-stub bind to *all* matching devices, and you only want it to
> bind to some.
You are right again.
> Maybe pci-stub could be extended to pay attention to
> PCI bus addresses in addition to vendor/device IDs.
A few thoughts here:
- We will have a race here between the "native" driver and pci-stub, right?
- Why not leverage the existing driver_override feature that is already
there and gives us exactly what we want: slot<->driver mapping?
- Maybe there are other scenarios that can benefit from slot<->driver mapping,
not only pci-stub.
>
> > Using some start scripts is error prone.
> >
> > The solution leverages driver_override functionality introduced by
> >
> > commit: 782a985d7af26db39e86070d28f987cad21313c0
> > Author: Alex Williamson
> > Date: Tue May 20 08:53:21 2014 -0600
> >
> > PCI: Introduce new device binding path using pci_dev.driver_override
> >
> > In order to bind PCI slots to specific drivers use:
> > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
>
> If/when you address Alex's comments about other bus types, can you
> also update the changelog to use the canonical commit reference
> format, i.e., 782a985d7af2 ("PCI: Introduce new device binding path
> using pci_dev.driver_override") in this case?
Sure, thanks for the tip.
>
> PCI bus numbers are mutable, e.g., they can change with hotplug or
> other configuration changes. But I don't have any better suggestion,
> so I guess all we can do is be aware of this.
Well, indeed, there is so much that can be done. (We can listen to an event and
remap...)
>
> Speaking of hotplug, this is only a boot-time kernel parameter, with
> no opportunity to use this, e.g., to add slot/driver pairs, after
> boot. Do you not need that because of Alex's driver_override thing?
Well actually Alex's "driver_override" feature does that for runtime
(adds slot/driver pair), the only thing missing is the boot time
mapping.
> How can we integrate this all together into a coherent whole? I'm a
> little confused as to how this would all be documented in a form
> usable by end-users.
For end-users it will be like this:
They want to create a slot/driver mapping.
In order to do that they will use the "driver_override" feature:
1. Run-time use:
- Use sysfs to edit driver_override file associated with the slot.
2. Boot-time use:
- Use the pci's driver_override parameter.
Thanks,
Marcel
>
> Bjorn
--
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: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote:
> [cc+ stuart]
>
> On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
> > Scanning a lot of devices during boot requires a lot of time.
> > On other scenarios there is a need to bind a driver to a specific slot.
> >
> > Binding devices to pci-stub driver does not work,
> > as it will not differentiate between devices of the
> > same type. Using some start scripts is error prone.
> >
> > The solution leverages driver_override functionality introduced by
> >
> > commit: 782a985d7af26db39e86070d28f987cad21313c0
> > Author: Alex Williamson
> > Date: Tue May 20 08:53:21 2014 -0600
> >
> > PCI: Introduce new device binding path using
> > pci_dev.driver_override
> >
> > In order to bind PCI slots to specific drivers use:
> > pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
> >
> > Signed-off-by: Marcel Apfelbaum
> > ---
> > v3 -> v4:
> > - Addressed Alex Williamson's comments:
> >- Modified the type of driver_override_entry's fields
> >- Used PCI_DEVFN when appropriated
> >- Removed redundant checks
> >- Replaced BUG_ON with pr_err messages
> >- Simpler command line parsing
> > - Addressed Michael S. Tsirkin comments
> >- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
> > v2 -> v3:
> > - Corrected subject line
> > v1 -> v2:
> > - Addressed Michael S. Tsirkin comments
> >- Removed 32 slots limitation
> >- Better handling of memory allocation failures
> > (preferred BUG_ON over error messages)
> > - Addressed Alex Williamson's comments:
> >- Modified commit message to show parameter usage more clear.
> > - I preferred to re-use parse_args instead of manually using
> >strstr in order to better comply with command line parsing
> >rules.
> > - I didn't use any locking when parsing the command line args
> >(see parse_done usage) assuming that first call will be
> >early in system boot and no race can occur. Please correct
> >me if I am wrong.
> >
> > Notes:
> > - I have further ideas on top of this patch based on your reviews.
> >I thought of:
> >- Use wildcards to specify entire buses/devices, something like:
> > driver[0001:02:*.*]=pci-stub
> >- Use comma to separate several devices:
> > driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
> >- Make domain optional:
> > driver[00:03.0]=pci-stub
> >
> > Comments will be appreciated,
> > Thanks,
> > Marcel
> > Documentation/kernel-parameters.txt | 4 ++
> > drivers/pci/bus.c | 111
> >
> > drivers/pci/pci.c | 2 +
> > 3 files changed, 117 insertions(+)
>
> The driver_override feature that we're making use of here is also going
> to be supported by platform devices and potentially more bustypes in the
> future, so I'm concerned that making a pci specific kernel parameter is
> too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
> bustypes that support driver_override so we can have a common interface.
The real question here if those bus types/devices would benefit from this
feature, and I also must confess that I have no knowledge of the other buses.
Can anyone confirm that it does make sense for them?
> Perhaps:
>
> driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
>
> Finding delimiters that don't conflict may be challenging. Also, can we
> assume that bus-name:dev-name is unique for every bustype? It is for
> pci, platform?
For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea,
can anyone that knows "platform" confirm or deny?
>
> It also seems like there's a question of how long should this override
> last and how does the user disable it?
> I think with pci-stub.ids=
> $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
> entry to cancel the effect.
The way I see it is simple, the override specified in kernel command line
last as long as the user does not specifically remove it using
echo "" > /sys/.../driver_override
and then unbind and bind the device again.
> The only option here seems to be a reboot.
Please see above
> Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
> this interface? Thanks,
While it does not hurt, I see it as optional since a simple removal of
driver_override and rebind does the same
Thanks,
Marcel
>
> Alex
>
> > diff --git a/Documentation/kernel-parameters.txt
> > b/Documentation/kernel-parameters.txt
> > index 5ae8608..c1cbb4c 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -2631,6 +2631,10 @@ bytes respectively. Such letter suffixes can also be
> > entirely omitted.
> > pcie_scan_all Scan all possible PCIe devices. Otherwise we
> > only look for one device below a PCIe downstream
> >
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote:
[cc+ stuart]
On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
Scanning a lot of devices during boot requires a lot of time.
On other scenarios there is a need to bind a driver to a specific slot.
Binding devices to pci-stub driver does not work,
as it will not differentiate between devices of the
same type. Using some start scripts is error prone.
The solution leverages driver_override functionality introduced by
commit: 782a985d7af26db39e86070d28f987cad21313c0
Author: Alex Williamson alex.william...@redhat.com
Date: Tue May 20 08:53:21 2014 -0600
PCI: Introduce new device binding path using
pci_dev.driver_override
In order to bind PCI slots to specific drivers use:
pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
v3 - v4:
- Addressed Alex Williamson's comments:
- Modified the type of driver_override_entry's fields
- Used PCI_DEVFN when appropriated
- Removed redundant checks
- Replaced BUG_ON with pr_err messages
- Simpler command line parsing
- Addressed Michael S. Tsirkin comments
- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
v2 - v3:
- Corrected subject line
v1 - v2:
- Addressed Michael S. Tsirkin comments
- Removed 32 slots limitation
- Better handling of memory allocation failures
(preferred BUG_ON over error messages)
- Addressed Alex Williamson's comments:
- Modified commit message to show parameter usage more clear.
- I preferred to re-use parse_args instead of manually using
strstr in order to better comply with command line parsing
rules.
- I didn't use any locking when parsing the command line args
(see parse_done usage) assuming that first call will be
early in system boot and no race can occur. Please correct
me if I am wrong.
Notes:
- I have further ideas on top of this patch based on your reviews.
I thought of:
- Use wildcards to specify entire buses/devices, something like:
driver[0001:02:*.*]=pci-stub
- Use comma to separate several devices:
driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
- Make domain optional:
driver[00:03.0]=pci-stub
Comments will be appreciated,
Thanks,
Marcel
Documentation/kernel-parameters.txt | 4 ++
drivers/pci/bus.c | 111
drivers/pci/pci.c | 2 +
3 files changed, 117 insertions(+)
The driver_override feature that we're making use of here is also going
to be supported by platform devices and potentially more bustypes in the
future, so I'm concerned that making a pci specific kernel parameter is
too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
bustypes that support driver_override so we can have a common interface.
The real question here if those bus types/devices would benefit from this
feature, and I also must confess that I have no knowledge of the other buses.
Can anyone confirm that it does make sense for them?
Perhaps:
driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
Finding delimiters that don't conflict may be challenging. Also, can we
assume that bus-name:dev-name is unique for every bustype? It is for
pci, platform?
For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea,
can anyone that knows platform confirm or deny?
It also seems like there's a question of how long should this override
last and how does the user disable it?
I think with pci-stub.ids=
$VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
entry to cancel the effect.
The way I see it is simple, the override specified in kernel command line
last as long as the user does not specifically remove it using
echo /sys/.../driver_override
and then unbind and bind the device again.
The only option here seems to be a reboot.
Please see above
Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
this interface? Thanks,
While it does not hurt, I see it as optional since a simple removal of
driver_override and rebind does the same
Thanks,
Marcel
Alex
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 5ae8608..c1cbb4c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2631,6 +2631,10 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
pcie_scan_all Scan all possible PCIe devices. Otherwise we
only look for one device below a PCIe downstream
port.
+ driver Provide an override to the devid-driver
mapping
+ for a specific slot.
+
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Wed, 2014-10-22 at 15:28 -0600, Bjorn Helgaas wrote: Hi Marcel, Hi Bjorn, Thank you for the review! I'm not quite clear on what the objective is here, so I apologize for some questions that probably seem silly. I appreciate you took your time to go over it. On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum marce...@redhat.com wrote: Scanning a lot of devices during boot requires a lot of time. I think what takes a lot of time is the .probe() method for some drivers, right? I first thought you meant that it took a long time for the PCI core to enumerate a lot of devices, but you're not changing anything there. Yes indeed. If the intent is to reduce boot time, I don't think this is a general solution. Drivers should be able to schedule asynchronous things in their .probe() methods if necessary. I agree, but sadly we cannot go over *all* existing drivers and fix, we can of course do the best effort :) By the way this was not the only reason as you also thought, see bellow On other scenarios there is a need to bind a driver to a specific slot. A short example here would be good. Are you talking about something like binding a NIC driver to one device while leaving others unbound for use by guests? Exactly! This is the perfect example, thanks! Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. I assume you mean booting with pci-stub.ids=$VENDOR:$DEVICE will make pci-stub bind to *all* matching devices, and you only want it to bind to some. You are right again. Maybe pci-stub could be extended to pay attention to PCI bus addresses in addition to vendor/device IDs. A few thoughts here: - We will have a race here between the native driver and pci-stub, right? - Why not leverage the existing driver_override feature that is already there and gives us exactly what we want: slot-driver mapping? - Maybe there are other scenarios that can benefit from slot-driver mapping, not only pci-stub. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... If/when you address Alex's comments about other bus types, can you also update the changelog to use the canonical commit reference format, i.e., 782a985d7af2 (PCI: Introduce new device binding path using pci_dev.driver_override) in this case? Sure, thanks for the tip. PCI bus numbers are mutable, e.g., they can change with hotplug or other configuration changes. But I don't have any better suggestion, so I guess all we can do is be aware of this. Well, indeed, there is so much that can be done. (We can listen to an event and remap...) Speaking of hotplug, this is only a boot-time kernel parameter, with no opportunity to use this, e.g., to add slot/driver pairs, after boot. Do you not need that because of Alex's driver_override thing? Well actually Alex's driver_override feature does that for runtime (adds slot/driver pair), the only thing missing is the boot time mapping. How can we integrate this all together into a coherent whole? I'm a little confused as to how this would all be documented in a form usable by end-users. For end-users it will be like this: They want to create a slot/driver mapping. In order to do that they will use the driver_override feature: 1. Run-time use: - Use sysfs to edit driver_override file associated with the slot. 2. Boot-time use: - Use the pci's driver_override parameter. Thanks, Marcel Bjorn -- 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: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Wed, Oct 22, 2014 at 03:28:29PM -0600, Bjorn Helgaas wrote: Hi Marcel, I'm not quite clear on what the objective is here, so I apologize for some questions that probably seem silly. On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum marce...@redhat.com wrote: Scanning a lot of devices during boot requires a lot of time. I think what takes a lot of time is the .probe() method for some drivers, right? I first thought you meant that it took a long time for the PCI core to enumerate a lot of devices, but you're not changing anything there. If the intent is to reduce boot time, I don't think this is a general solution. Drivers should be able to schedule asynchronous things in their .probe() methods if necessary. If this worked for all devices, we could just make probe asynchronous in PCI core. Unfortunately this doesn't work esp for storage devices since people expect disks to be available for mount immediately. If the point of the patch is to speed up boot, we could try to probe everything in parallel? Probe is serialized now, right? On other scenarios there is a need to bind a driver to a specific slot. A short example here would be good. Are you talking about something like binding a NIC driver to one device while leaving others unbound for use by guests? Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. I assume you mean booting with pci-stub.ids=$VENDOR:$DEVICE will make pci-stub bind to *all* matching devices, and you only want it to bind to some. Maybe pci-stub could be extended to pay attention to PCI bus addresses in addition to vendor/device IDs. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... If/when you address Alex's comments about other bus types, can you also update the changelog to use the canonical commit reference format, i.e., 782a985d7af2 (PCI: Introduce new device binding path using pci_dev.driver_override) in this case? PCI bus numbers are mutable, e.g., they can change with hotplug or other configuration changes. But I don't have any better suggestion, so I guess all we can do is be aware of this. We could use slot capability for addressing if that's available. Speaking of hotplug, this is only a boot-time kernel parameter, with no opportunity to use this, e.g., to add slot/driver pairs, after boot. Do you not need that because of Alex's driver_override thing? How can we integrate this all together into a coherent whole? I'm a little confused as to how this would all be documented in a form usable by end-users. Bjorn -- 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: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote:
On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote:
[cc+ stuart]
On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
Scanning a lot of devices during boot requires a lot of time.
On other scenarios there is a need to bind a driver to a specific slot.
Binding devices to pci-stub driver does not work,
as it will not differentiate between devices of the
same type. Using some start scripts is error prone.
The solution leverages driver_override functionality introduced by
commit: 782a985d7af26db39e86070d28f987cad21313c0
Author: Alex Williamson alex.william...@redhat.com
Date: Tue May 20 08:53:21 2014 -0600
PCI: Introduce new device binding path using
pci_dev.driver_override
In order to bind PCI slots to specific drivers use:
pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
v3 - v4:
- Addressed Alex Williamson's comments:
- Modified the type of driver_override_entry's fields
- Used PCI_DEVFN when appropriated
- Removed redundant checks
- Replaced BUG_ON with pr_err messages
- Simpler command line parsing
- Addressed Michael S. Tsirkin comments
- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
v2 - v3:
- Corrected subject line
v1 - v2:
- Addressed Michael S. Tsirkin comments
- Removed 32 slots limitation
- Better handling of memory allocation failures
(preferred BUG_ON over error messages)
- Addressed Alex Williamson's comments:
- Modified commit message to show parameter usage more clear.
- I preferred to re-use parse_args instead of manually using
strstr in order to better comply with command line parsing
rules.
- I didn't use any locking when parsing the command line args
(see parse_done usage) assuming that first call will be
early in system boot and no race can occur. Please correct
me if I am wrong.
Notes:
- I have further ideas on top of this patch based on your reviews.
I thought of:
- Use wildcards to specify entire buses/devices, something like:
driver[0001:02:*.*]=pci-stub
- Use comma to separate several devices:
driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
- Make domain optional:
driver[00:03.0]=pci-stub
Comments will be appreciated,
Thanks,
Marcel
Documentation/kernel-parameters.txt | 4 ++
drivers/pci/bus.c | 111
drivers/pci/pci.c | 2 +
3 files changed, 117 insertions(+)
The driver_override feature that we're making use of here is also going
to be supported by platform devices and potentially more bustypes in the
future, so I'm concerned that making a pci specific kernel parameter is
too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
bustypes that support driver_override so we can have a common interface.
The real question here if those bus types/devices would benefit from this
feature, and I also must confess that I have no knowledge of the other buses.
Can anyone confirm that it does make sense for them?
Platform devices are adding vfio support, so I expect the next logical
question will be how to reserve devices for use by vfio at boot.
Perhaps:
driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
Finding delimiters that don't conflict may be challenging. Also, can we
assume that bus-name:dev-name is unique for every bustype? It is for
pci, platform?
For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea,
can anyone that knows platform confirm or deny?
It also seems like there's a question of how long should this override
last and how does the user disable it?
I think with pci-stub.ids=
$VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
entry to cancel the effect.
The way I see it is simple, the override specified in kernel command line
last as long as the user does not specifically remove it using
echo /sys/.../driver_override
and then unbind and bind the device again.
The only option here seems to be a reboot.
Please see above
That's only a temporary removal though, if the device is removed and
re-added, either via physical hotplug or sysfs, the override is
re-applied. Thanks,
Alex
Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
this interface? Thanks,
While it does not hurt, I see it as optional since a simple removal of
driver_override and rebind does the same
Thanks,
Marcel
Alex
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 5ae8608..c1cbb4c 100644
--- a/Documentation/kernel-parameters.txt
+++
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 07:11 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote: On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? Platform devices are adding vfio support, so I expect the next logical question will be how to reserve devices for use by vfio at boot. Well, I'll have to learn more about vfio before saying anything, but my question is if it can be deferred or it has to be part of this patch. If the platform devices do not have a slot like hw address, maybe it can be configured separately. I saw this patch as a PCI patch, and not driver_override expansion; meaning that I only leveraged an existing feature, not trying to extend it. If I am wrong, please point me to a more robust solution. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea, can anyone that knows platform confirm or deny? It also seems like there's a question of how long should this override last and how does the user disable it? I think with pci-stub.ids= $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs entry to cancel the effect. The way I see it is simple, the override specified in kernel command line last as long as the user does not specifically remove it using echo /sys/.../driver_override and then unbind and bind the device again. The only option
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message-
From: Alex Williamson [mailto:alex.william...@redhat.com]
Sent: Wednesday, October 22, 2014 1:33 PM
To: Marcel Apfelbaum
Cc: linux-...@vger.kernel.org; bhelg...@google.com;
linux-kernel@vger.kernel.org; mar...@redhat.com;
m...@redhat.com; Yoder Stuart-B08248
Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver
mapping.
[cc+ stuart]
On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
Scanning a lot of devices during boot requires a lot of time.
On other scenarios there is a need to bind a driver to a specific slot.
Binding devices to pci-stub driver does not work,
as it will not differentiate between devices of the
same type. Using some start scripts is error prone.
The solution leverages driver_override functionality introduced by
commit: 782a985d7af26db39e86070d28f987cad21313c0
Author: Alex Williamson alex.william...@redhat.com
Date: Tue May 20 08:53:21 2014 -0600
PCI: Introduce new device binding path using
pci_dev.driver_override
In order to bind PCI slots to specific drivers use:
pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
v3 - v4:
- Addressed Alex Williamson's comments:
- Modified the type of driver_override_entry's fields
- Used PCI_DEVFN when appropriated
- Removed redundant checks
- Replaced BUG_ON with pr_err messages
- Simpler command line parsing
- Addressed Michael S. Tsirkin comments
- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
v2 - v3:
- Corrected subject line
v1 - v2:
- Addressed Michael S. Tsirkin comments
- Removed 32 slots limitation
- Better handling of memory allocation failures
(preferred BUG_ON over error messages)
- Addressed Alex Williamson's comments:
- Modified commit message to show parameter usage more clear.
- I preferred to re-use parse_args instead of manually using
strstr in order to better comply with command line parsing
rules.
- I didn't use any locking when parsing the command line args
(see parse_done usage) assuming that first call will be
early in system boot and no race can occur. Please correct
me if I am wrong.
Notes:
- I have further ideas on top of this patch based on your reviews.
I thought of:
- Use wildcards to specify entire buses/devices, something like:
driver[0001:02:*.*]=pci-stub
- Use comma to separate several devices:
driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
- Make domain optional:
driver[00:03.0]=pci-stub
Comments will be appreciated,
Thanks,
Marcel
Documentation/kernel-parameters.txt | 4 ++
drivers/pci/bus.c | 111
drivers/pci/pci.c | 2 +
3 files changed, 117 insertions(+)
The driver_override feature that we're making use of here is also going
to be supported by platform devices and potentially more bustypes in the
future, so I'm concerned that making a pci specific kernel parameter is
too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
bustypes that support driver_override so we can have a common interface.
Perhaps:
driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
Finding delimiters that don't conflict may be challenging.
I think what you proposed works-- bus-name,bus-dev=driver;
Think that will work for PCI, platform, and the new fsl-mc bus we are working
on.
Also, can we
assume that bus-name:dev-name is unique for every bustype? It is for
pci, platform?
I think that has to be the case.
It also seems like there's a question of how long should this override
last and how does the user disable it?
Isn't that a general question for the driver_overrride mechanism?
I'm forgetting if the mechanism in the kernel now has a way to disable
it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ??
So, it would last until explicitly disabled through sysfs.
I think with pci-stub.ids=
$VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
entry to cancel the effect. The only option here seems to be a reboot.
Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
this interface? Thanks,
Thanks,
Stuart
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 13:51 +, Stuart Yoder wrote: -Original Message- From: Marcel Apfelbaum [mailto:marce...@redhat.com] Sent: Thursday, October 23, 2014 7:32 AM To: Alex Williamson Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? We don't have vfio-platform in use yet, so not much actual real world user experience yet. But, I think it makes sense. Especially, given that we are inventing a kernel parameter I think we should design the syntax so that it can work buses can implement support for this. The driver_override mechanism is not bus specific, so let's not make the kernel parameter bus specific. Make sense, point taken. I'll come up with something. Thank you Stuart, Marcel Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea, can anyone that knows platform confirm or deny? Yes, dev-name will be unique. All platform devices are under a single platform bus. 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
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ?? So, it would last until explicitly disabled through sysfs. Yes, when you set a driver_override on a device you cancel it by writing a NULL string to the same interface. The problem is that here we have a new entity in the driver scan that keeps re-applying the driver_override as devices are scanned with no way to stop it. So you can certainly undo the immediate effect and bind the device to another driver, but if the device is removed and re-scanned there's no way to stop if from re-applying the override. Thanks, Alex I think with pci-stub.ids= $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs entry to cancel the effect. The only option here seems to be a reboot. Do we need a /sys/bus/pci/driver_overrides
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message- From: Marcel Apfelbaum [mailto:marce...@redhat.com] Sent: Thursday, October 23, 2014 7:32 AM To: Alex Williamson Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? We don't have vfio-platform in use yet, so not much actual real world user experience yet. But, I think it makes sense. Especially, given that we are inventing a kernel parameter I think we should design the syntax so that it can work buses can implement support for this. The driver_override mechanism is not bus specific, so let's not make the kernel parameter bus specific. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea, can anyone that knows platform confirm or deny? Yes, dev-name will be unique. All platform devices are under a single platform bus. Stuart
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ?? So, it would last until explicitly disabled through sysfs. Yes, when you set a driver_override on a device you cancel it by writing a NULL string to the same interface. The problem is that here we have a new entity in the driver scan that keeps re-applying the driver_override as devices are scanned with no way to stop it. So you can certainly undo the immediate effect and bind the device to another driver, but if the device is removed and re-scanned there's no way to stop if from re-applying the override. Thanks, Hi Alex, I checked the above scenario
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ?? So, it would last until explicitly disabled through sysfs. Yes, when you set a driver_override on a device you cancel it by writing a NULL string to the same interface. The problem is that here we have a new entity in the driver scan that keeps re-applying the driver_override as devices are scanned with no way to stop it. So you can certainly undo the immediate effect and bind the device
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ?? So, it would last until explicitly disabled through sysfs. Yes, when you set a driver_override on a device you cancel it by writing a NULL string to the same interface. The problem
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message- From: Marcel Apfelbaum [mailto:marce...@redhat.com] Sent: Thursday, October 23, 2014 8:37 AM To: Alex Williamson Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. On Thu, 2014-10-23 at 07:11 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote: On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? Platform devices are adding vfio support, so I expect the next logical question will be how to reserve devices for use by vfio at boot. Well, I'll have to learn more about vfio before saying anything, but my question is if it can be deferred or it has to be part of this patch. If the platform devices do not have a slot like hw address, maybe it can be configured separately. I saw this patch as a PCI patch, and not driver_override expansion; meaning that I only leveraged an existing feature, not trying to extend it. I think other buses may want to use the same mechanism to specify driver bindings, so I think the main thing is to not define a syntax that makes that problematic. Looking at your original syntax, I think it could work for different bus types. Could have something like: platform=driver[]=foo,driver[]=vfio-platform pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar So no strong opinion on that vs Alex's proposal: driver_override=pci,:02:00.0=pci-stub;platform,=vfio-platform
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 18:00 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
On Thu, 2014-10-23 at 09:54 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 18:00 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 08:49 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 17:33 +0300, Marcel Apfelbaum wrote: On Thu, 2014-10-23 at 07:57 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 13:44 +, Stuart Yoder wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't
Re: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
Hi Marcel,
I'm not quite clear on what the objective is here, so I apologize for
some questions that probably seem silly.
On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum wrote:
> Scanning a lot of devices during boot requires a lot of time.
I think what takes a lot of time is the .probe() method for some
drivers, right? I first thought you meant that it took a long time
for the PCI core to enumerate a lot of devices, but you're not
changing anything there.
If the intent is to reduce boot time, I don't think this is a general
solution. Drivers should be able to schedule asynchronous things in
their .probe() methods if necessary.
> On other scenarios there is a need to bind a driver to a specific slot.
A short example here would be good. Are you talking about something
like binding a NIC driver to one device while leaving others unbound
for use by guests?
> Binding devices to pci-stub driver does not work,
> as it will not differentiate between devices of the
> same type.
I assume you mean booting with "pci-stub.ids=$VENDOR:$DEVICE" will
make pci-stub bind to *all* matching devices, and you only want it to
bind to some. Maybe pci-stub could be extended to pay attention to
PCI bus addresses in addition to vendor/device IDs.
> Using some start scripts is error prone.
>
> The solution leverages driver_override functionality introduced by
>
> commit: 782a985d7af26db39e86070d28f987cad21313c0
> Author: Alex Williamson
> Date: Tue May 20 08:53:21 2014 -0600
>
> PCI: Introduce new device binding path using pci_dev.driver_override
>
> In order to bind PCI slots to specific drivers use:
> pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
If/when you address Alex's comments about other bus types, can you
also update the changelog to use the canonical commit reference
format, i.e., 782a985d7af2 ("PCI: Introduce new device binding path
using pci_dev.driver_override") in this case?
PCI bus numbers are mutable, e.g., they can change with hotplug or
other configuration changes. But I don't have any better suggestion,
so I guess all we can do is be aware of this.
Speaking of hotplug, this is only a boot-time kernel parameter, with
no opportunity to use this, e.g., to add slot/driver pairs, after
boot. Do you not need that because of Alex's driver_override thing?
How can we integrate this all together into a coherent whole? I'm a
little confused as to how this would all be documented in a form
usable by end-users.
Bjorn
--
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: [PATCH v4] PCI: add kernel parameter to override devid<->driver mapping.
[cc+ stuart]
On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
> Scanning a lot of devices during boot requires a lot of time.
> On other scenarios there is a need to bind a driver to a specific slot.
>
> Binding devices to pci-stub driver does not work,
> as it will not differentiate between devices of the
> same type. Using some start scripts is error prone.
>
> The solution leverages driver_override functionality introduced by
>
> commit: 782a985d7af26db39e86070d28f987cad21313c0
> Author: Alex Williamson
> Date: Tue May 20 08:53:21 2014 -0600
>
> PCI: Introduce new device binding path using pci_dev.driver_override
>
> In order to bind PCI slots to specific drivers use:
> pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
>
> Signed-off-by: Marcel Apfelbaum
> ---
> v3 -> v4:
> - Addressed Alex Williamson's comments:
>- Modified the type of driver_override_entry's fields
>- Used PCI_DEVFN when appropriated
>- Removed redundant checks
>- Replaced BUG_ON with pr_err messages
>- Simpler command line parsing
> - Addressed Michael S. Tsirkin comments
>- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
> v2 -> v3:
> - Corrected subject line
> v1 -> v2:
> - Addressed Michael S. Tsirkin comments
>- Removed 32 slots limitation
>- Better handling of memory allocation failures
> (preferred BUG_ON over error messages)
> - Addressed Alex Williamson's comments:
>- Modified commit message to show parameter usage more clear.
> - I preferred to re-use parse_args instead of manually using
>strstr in order to better comply with command line parsing
>rules.
> - I didn't use any locking when parsing the command line args
>(see parse_done usage) assuming that first call will be
>early in system boot and no race can occur. Please correct
>me if I am wrong.
>
> Notes:
> - I have further ideas on top of this patch based on your reviews.
>I thought of:
>- Use wildcards to specify entire buses/devices, something like:
> driver[0001:02:*.*]=pci-stub
>- Use comma to separate several devices:
> driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
>- Make domain optional:
> driver[00:03.0]=pci-stub
>
> Comments will be appreciated,
> Thanks,
> Marcel
> Documentation/kernel-parameters.txt | 4 ++
> drivers/pci/bus.c | 111
>
> drivers/pci/pci.c | 2 +
> 3 files changed, 117 insertions(+)
The driver_override feature that we're making use of here is also going
to be supported by platform devices and potentially more bustypes in the
future, so I'm concerned that making a pci specific kernel parameter is
too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
bustypes that support driver_override so we can have a common interface.
Perhaps:
driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
Finding delimiters that don't conflict may be challenging. Also, can we
assume that bus-name:dev-name is unique for every bustype? It is for
pci, platform?
It also seems like there's a question of how long should this override
last and how does the user disable it? I think with pci-stub.ids=
$VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
entry to cancel the effect. The only option here seems to be a reboot.
Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
this interface? Thanks,
Alex
> diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> index 5ae8608..c1cbb4c 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2631,6 +2631,10 @@ bytes respectively. Such letter suffixes can also be
> entirely omitted.
> pcie_scan_all Scan all possible PCIe devices. Otherwise we
> only look for one device below a PCIe downstream
> port.
> + driver Provide an override to the devid<->driver
> mapping
> + for a specific slot.
> + Bind PCI slot 0001:02:03.4 to pci-stub by:
> + driver[0001:02:03.4]=pci-stub
>
> pcie_aspm= [PCIE] Forcibly enable or disable PCIe Active State
> Power
> Management.
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 73aef51..b49f5cc 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -15,6 +15,8 @@
> #include
> #include
>
> +#include
> +
> #include "pci.h"
>
> void pci_add_resource_offset(struct list_head *resources, struct resource
> *res,
> @@ -230,6 +232,114 @@ EXPORT_SYMBOL(pci_bus_alloc_resource);
>
> void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
>
> +struct driver_override_entry {
> + u16 domain;
> + u8 bus;
> + u8 devfn;
> + char
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
[cc+ stuart]
On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote:
Scanning a lot of devices during boot requires a lot of time.
On other scenarios there is a need to bind a driver to a specific slot.
Binding devices to pci-stub driver does not work,
as it will not differentiate between devices of the
same type. Using some start scripts is error prone.
The solution leverages driver_override functionality introduced by
commit: 782a985d7af26db39e86070d28f987cad21313c0
Author: Alex Williamson alex.william...@redhat.com
Date: Tue May 20 08:53:21 2014 -0600
PCI: Introduce new device binding path using pci_dev.driver_override
In order to bind PCI slots to specific drivers use:
pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,...
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
v3 - v4:
- Addressed Alex Williamson's comments:
- Modified the type of driver_override_entry's fields
- Used PCI_DEVFN when appropriated
- Removed redundant checks
- Replaced BUG_ON with pr_err messages
- Simpler command line parsing
- Addressed Michael S. Tsirkin comments
- removed DRIVER_OVERRIDE_NAME_LENGTH limitation
v2 - v3:
- Corrected subject line
v1 - v2:
- Addressed Michael S. Tsirkin comments
- Removed 32 slots limitation
- Better handling of memory allocation failures
(preferred BUG_ON over error messages)
- Addressed Alex Williamson's comments:
- Modified commit message to show parameter usage more clear.
- I preferred to re-use parse_args instead of manually using
strstr in order to better comply with command line parsing
rules.
- I didn't use any locking when parsing the command line args
(see parse_done usage) assuming that first call will be
early in system boot and no race can occur. Please correct
me if I am wrong.
Notes:
- I have further ideas on top of this patch based on your reviews.
I thought of:
- Use wildcards to specify entire buses/devices, something like:
driver[0001:02:*.*]=pci-stub
- Use comma to separate several devices:
driver[0001:02:03.4,0001:02:04.0,...]=pci-stub
- Make domain optional:
driver[00:03.0]=pci-stub
Comments will be appreciated,
Thanks,
Marcel
Documentation/kernel-parameters.txt | 4 ++
drivers/pci/bus.c | 111
drivers/pci/pci.c | 2 +
3 files changed, 117 insertions(+)
The driver_override feature that we're making use of here is also going
to be supported by platform devices and potentially more bustypes in the
future, so I'm concerned that making a pci specific kernel parameter is
too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for
bustypes that support driver_override so we can have a common interface.
Perhaps:
driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform
Finding delimiters that don't conflict may be challenging. Also, can we
assume that bus-name:dev-name is unique for every bustype? It is for
pci, platform?
It also seems like there's a question of how long should this override
last and how does the user disable it? I think with pci-stub.ids=
$VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs
entry to cancel the effect. The only option here seems to be a reboot.
Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for
this interface? Thanks,
Alex
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 5ae8608..c1cbb4c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2631,6 +2631,10 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
pcie_scan_all Scan all possible PCIe devices. Otherwise we
only look for one device below a PCIe downstream
port.
+ driver Provide an override to the devid-driver
mapping
+ for a specific slot.
+ Bind PCI slot 0001:02:03.4 to pci-stub by:
+ driver[0001:02:03.4]=pci-stub
pcie_aspm= [PCIE] Forcibly enable or disable PCIe Active State
Power
Management.
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 73aef51..b49f5cc 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -15,6 +15,8 @@
#include linux/proc_fs.h
#include linux/slab.h
+#include asm/setup.h
+
#include pci.h
void pci_add_resource_offset(struct list_head *resources, struct resource
*res,
@@ -230,6 +232,114 @@ EXPORT_SYMBOL(pci_bus_alloc_resource);
void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
+struct driver_override_entry {
+ u16 domain;
+ u8 bus;
+ u8 devfn;
+ char *driver_name;
+ struct
Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
Hi Marcel, I'm not quite clear on what the objective is here, so I apologize for some questions that probably seem silly. On Mon, Oct 20, 2014 at 8:04 AM, Marcel Apfelbaum marce...@redhat.com wrote: Scanning a lot of devices during boot requires a lot of time. I think what takes a lot of time is the .probe() method for some drivers, right? I first thought you meant that it took a long time for the PCI core to enumerate a lot of devices, but you're not changing anything there. If the intent is to reduce boot time, I don't think this is a general solution. Drivers should be able to schedule asynchronous things in their .probe() methods if necessary. On other scenarios there is a need to bind a driver to a specific slot. A short example here would be good. Are you talking about something like binding a NIC driver to one device while leaving others unbound for use by guests? Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. I assume you mean booting with pci-stub.ids=$VENDOR:$DEVICE will make pci-stub bind to *all* matching devices, and you only want it to bind to some. Maybe pci-stub could be extended to pay attention to PCI bus addresses in addition to vendor/device IDs. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... If/when you address Alex's comments about other bus types, can you also update the changelog to use the canonical commit reference format, i.e., 782a985d7af2 (PCI: Introduce new device binding path using pci_dev.driver_override) in this case? PCI bus numbers are mutable, e.g., they can change with hotplug or other configuration changes. But I don't have any better suggestion, so I guess all we can do is be aware of this. Speaking of hotplug, this is only a boot-time kernel parameter, with no opportunity to use this, e.g., to add slot/driver pairs, after boot. Do you not need that because of Alex's driver_override thing? How can we integrate this all together into a coherent whole? I'm a little confused as to how this would all be documented in a form usable by end-users. Bjorn -- 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/
