Introduce PCI device blacklisting
Subsystems (2)
cover Introduce PCI device blacklisting
This set introduces a method to keep devices from matching driver id tables.
Please see 0001 for more details of the motivation and implementation.
This set currently applies to Bjorn's next (v4.14-rc1) branch.
Jon Derrick (3):
PCI: pci-driver: Introduce pci device delete list
module: Ignore delete_id parameter
Documentation: Add pci device delete_id interface
Documentation/admin-guide/kernel-parameters.txt | 13 ++ drivers/pci/pci-driver.c | 253 +++++++++++++++++++++++- include/linux/pci.h | 1 + kernel/module.c | 7 + 4 files changed, 272 insertions(+), 2 deletions(-)
Signature · 1 line
1.8.3.1
RFC 1/3 PCI: pci-driver: Introduce pci device delete list
This patch introduces a new kernel command line parameter to mask pci device ids from pci driver id tables. This prevents masked devices from automatically binding to both built-in and module drivers.
Devices can be later attached through the driver's sysfs new_id inteface.
The use cases for this are primarily for debugging, eg, being able to prevent attachment before probes are set up. It can also be used to mask off faulty built-in hardware or faulty simulated hardware.
Another use case is to prevent attachment of devices which will be passed to VMs, shortcutting the detachment effort.
The format is similar to the sysfs new_id format. Device ids are specified with:
VVVV:DDDD[:SVVV:SDDD][:CCCC][:MMMM]
Where:
VVVV = Vendor ID
DDDD = Device ID
SVVV = Subvendor ID
SDDD = Subdevice ID
CCCC = Class
MMMM = Class Mask
IDs can be chained with commas.
Examples:
<driver>.delete_id=1234:5678 <driver>.delete_id=ffffffff:ffffffff <driver>.delete_id=ffffffff:ffffffff:ffffffff:ffffffff:010802 <driver>.delete_id=1234:5678,abcd:ef01,2345:ffffffff
--- drivers/pci/pci-driver.c | 253 ++++++++++++++++++++++++++++++++++++++++++++++- include/linux/pci.h | 1 + 2 files changed, 252 insertions(+), 2 deletions(-)
drivers/pci/pci-driver.c +251 -2
@@ -20,6 +20,7 @@#include <linux/pm_runtime.h> #include <linux/suspend.h> #include <linux/kexec.h> +#include <asm/setup.h> #include "pci.h" struct pci_dynid {
@@ -27,6 +28,250 @@ struct pci_dynid {struct pci_device_id id; }; +static struct pci_device_id *pci_match_deleted_ids (struct pci_driver *drv, +struct pci_dev *dev, +bool inverse) +{ +struct pci_dynid *deleteid; +struct pci_device_id *found_id =NULL ; + +spin_lock (&drv->deleteids.lock); +list_for_each_entry (deleteid, &drv->deleteids.list, node) { +if (!inverse &&pci_match_one_device (&deleteid->id, dev)) { + found_id = &deleteid->id; +break ; + } +if (inverse && !pci_match_one_device (&deleteid->id, dev)) { + found_id = &deleteid->id; +break ; + } + } +spin_unlock (&drv->deleteids.lock); + +return found_id; +} + +/** +* pci_match_non_deleted_ids - match dev against not-deleted ids +* @ids: array of PCI device id structures to search in +* @dev: the PCI device structure to match against. +* +* Finds devices in driver id table not matching the delete list and tries to +* match them individually to dev. Returns the matching pci_dev_id structure, +* %NULL if there is no match, of -errno if there was a failure to allocate +* memory for the temporary pci_dev +*/ +static struct pci_device_id *pci_match_non_deleted_ids (struct pci_driver *drv, +struct pci_dev *dev) +{ +const struct pci_device_id *ids = drv->id_table; +struct pci_device_id *match =NULL ; +struct pci_dev *tmpdev; + + tmpdev =kzalloc (sizeof (*tmpdev),GFP_KERNEL ); +if (!tmpdev) +return ERR_PTR (-ENOMEM ); + +while (ids->vendor || ids->subvendor || ids->class_mask) { +struct pci_device_id *found_id =NULL ; + + tmpdev->vendor = ids->vendor; + tmpdev->device = ids->device, + tmpdev->subsystem_vendor = ids->subvendor, + tmpdev->subsystem_device = ids->subdevice, + tmpdev->class = ids->class; + + found_id =pci_match_deleted_ids (drv, tmpdev, true); +if (found_id) { +const struct pci_device_id id[2 ] = { *found_id, {0 ,} }; +if (pci_match_id (id, dev)) { + match = found_id; +break ; + } + } + ids++; + } + +kfree (tmpdev); + +return match; +} + +/** +* pci_add_delete_id - add a new PCI device ID to a built-in driver's blacklist +* @drv: target pci driver +* @vendor: PCI vendor ID +* @device: PCI device ID +* @subvendor: PCI subvendor ID +* @subdevice: PCI subdevice ID +* @class: PCI class +* @class_mask: PCI class mask +* +* Adds a new dynamic pci device ID to this driver's delete list, preventing +* the device from attaching to built-in drivers +* +* CONTEXT: +* Does GFP_KERNEL allocation. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ +static int pci_add_delete_id (struct pci_driver *drv, +unsigned int vendor,unsigned int device, +unsigned int subvendor,unsigned int subdevice, +unsigned int class,unsigned int class_mask) +{ +struct pci_dynid *deleteid; + + deleteid =kzalloc (sizeof (*deleteid),GFP_KERNEL ); +if (!deleteid) +return -ENOMEM ; + + deleteid->id.vendor = vendor; + deleteid->id.device = device; + deleteid->id.subvendor = subvendor; + deleteid->id.subdevice = subdevice; + deleteid->id.class = class; + deleteid->id.class_mask = class_mask; + +spin_lock (&drv->deleteids.lock); +list_add_tail (&deleteid->node, &drv->deleteids.list); +spin_unlock (&drv->deleteids.lock); + +pr_debug ("%s: added %04x:%04x:[%04x:%04x]:[%08x:%08x] to delete list\n" , + drv->driver.name, vendor, device, subvendor, subdevice, + class, class_mask); +return 0 ; +} + +static void pci_free_delete_ids (struct pci_driver *drv) +{ +struct pci_dynid *deleteid, *n; + +spin_lock (&drv->deleteids.lock); +list_for_each_entry_safe (deleteid, n, &drv->deleteids.list, node) { +list_del (&deleteid->node); +kfree (deleteid); + } +spin_unlock (&drv->deleteids.lock); +} + +/** +* pci_parse_and_add_delete_id - parses kernel cmdline for delete ids +* @drv: the driver structure to register +* @ids: string of comma-delimited ids +* +* Parses a comma-delimited list of pci ids and adds them to the driver's +* delete blacklist +* +* Available formats: VVVV:DDDD[:SVVV:SDDD][:CCCC][:MMMM] +* Use full 32-bit format for PCI_ANY_ID (FFFFFFFF) +*/ +static void pci_parse_and_add_delete_id (struct pci_driver *drv,char *ids) +{ +int vendor, device, subvendor, subdevice, class, class_mask; +int count, fields; + +while (*ids) { + count =0 ; + fields =sscanf (ids,"%x:%x:%x:%x:%x:%x%n" , + &vendor, &device, &subvendor, &subdevice, + &class, &class_mask, &count); + +switch (fields) { +case 6 : +break ; +case 5 : +sscanf (ids,"%x:%x:%x:%x:%x%n" , &vendor, &device, + &subvendor, &subdevice, &class, &count); + class_mask =0 ; +break ; +case 4 : +sscanf (ids,"%x:%x:%x:%x%n" , &vendor, &device, + &subvendor, &subdevice, &count); + class_mask =0 ; + class =PCI_ANY_ID ; +break ; +case 2 : +sscanf (ids,"%x:%x%n" , &vendor, &device, &count); + class_mask =0 ; + class =PCI_ANY_ID ; + subvendor = subdevice =PCI_ANY_ID ; +break ; +default : +pr_err ("%s: Can't parse delete_id parameter: %s\n" , + drv->driver.name, ids); +return ; + } + +if (pci_add_delete_id (drv, vendor, device, + subvendor, subdevice, class, class_mask)) +break ; + + ids += count; +if (*ids !=',' ) { +/* End of param or invalid format */ +break ; + } + ids++; + } +} + +static int pci_boot_param_cb (char *param,char *val, +const char *modname,void *arg) +{ +struct pci_driver *drv = arg; +size_t len; +char *p; + +/* Missing arg on right side of <param>= */ +if (!val) +return 0 ; + + p =strchr (param,'.' ); +if (!p) +return 0 ; + +if (strcmp (p +1 ,"delete_id" )) +return 0 ; + + len = p - param; +if (len !=strlen (drv->driver.name) || + (strncmp (param, drv->driver.name, len))) +return 0 ; + +pci_parse_and_add_delete_id (drv, val); + +return 0 ; +} + +static void __pci_init_delete_ids (struct pci_driver *drv) +{ +static char cmdline[COMMAND_LINE_SIZE ]; +char doing[strlen ("pci-driver() params" ) +strlen (drv->driver.name) +1 ]; + +strcpy (cmdline, saved_command_line); +snprintf (doing,sizeof (doing),"pci-driver(%s) params" , + drv->driver.name); +parse_args (doing, cmdline,NULL , +0 ,0 ,0 , drv, &pci_boot_param_cb); +} + +static void pci_init_delete_ids (struct pci_driver *drv) +{ +spin_lock_init (&drv->deleteids.lock); +INIT_LIST_HEAD (&drv->deleteids.list); + +/* +* Most configurations won't need to blacklist any device ids. +* Shortcut those cases before calling into parse_args. +*/ +if (!strstr (saved_command_line,"delete_id" )) +return ; + +__pci_init_delete_ids (drv); +} +/** * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices * @drv: target pci driver
@@ -124,7 +369,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf, pdev->subsystem_device = subdevice; pdev->class = class; -if (pci_match_id (pdrv->id_table, pdev)) +if (pci_match_non_deleted_ids (pdrv, pdev)) retval = -EEXIST ;kfree (pdev);
@@ -269,7 +514,8 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, }spin_unlock (&drv->dynids.lock); -if (!found_id) +/* Check the device blacklist before matching the driver id table */ +if (!found_id && !pci_match_deleted_ids (drv, dev, false)) found_id =pci_match_id (drv->id_table, dev);/* driver_override will always match, send a dummy id */
@@ -1310,6 +1556,8 @@ int __pci_register_driver(struct pci_driver *drv, struct module *owner,spin_lock_init (&drv->dynids.lock);INIT_LIST_HEAD (&drv->dynids.list); +pci_init_delete_ids (drv); +/* register with core */ return driver_register (&drv->driver); }
@@ -1328,6 +1576,7 @@ int __pci_register_driver(struct pci_driver *drv, struct module *owner,void pci_unregister_driver (struct pci_driver *drv) {driver_unregister (&drv->driver); +pci_free_delete_ids (drv);pci_free_dynids (drv); }EXPORT_SYMBOL (pci_unregister_driver);
include/linux/pci.h +1 -0
@@ -757,6 +757,7 @@ struct pci_driver {const struct attribute_group **groups;struct device_driver driver;struct pci_dynids dynids; +struct pci_dynids deleteids; };#define to_pci_driver (drv) container_of(drv, struct pci_driver, driver)
Signature · 1 line
1.8.3.1
Re: [RFC 1/3] PCI: pci-driver: Introduce pci device delete list
Quoted 7 lines · Jon Derrick
This patch introduces a new kernel command line parameter to mask pci device ids from pci driver id tables. This prevents masked devices from automatically binding to both built-in and module drivers.
Devices can be later attached through the driver's sysfs
new_idinteface.
Quoted 6 lines
The use cases for this are primarily for debugging, eg, being able to prevent attachment before probes are set up. It can also be used to mask off faulty built-in hardware or faulty simulated hardware.
Another use case is to prevent attachment of devices which will be passed to VMs, shortcutting the detachment effort.
Is the "shortcut" really that big of a deal? unbind actually causes problems? Is this an attempt to deal with devices being handled by more than one driver and then you want to manually bind it later on?
Quoted 13 lines
The format is similar to the sysfs
new_idformat. Device ids are specified with:VVVV:DDDD[:SVVV:SDDD][:CCCC][:MMMM]
Where:
VVVV = Vendor ID
DDDD = Device ID
SVVV = Subvendor ID
SDDD = Subdevice ID
CCCC = Class
MMMM = Class Mask
Quoted 7 lines
IDs can be chained with commas.
Examples:
<driver>.delete_id=1234:5678 <driver>.delete_id=ffffffff:ffffffff <driver>.delete_id=ffffffff:ffffffff:ffffffff:ffffffff:010802 <driver>.delete_id=1234:5678,abcd:ef01,2345:ffffffff
What about drivers that handle more than one bus type (i.e. USB and PCI?) This format is specific to PCI, yet you are defining it as a "global" for all drivers :(
This feels hacky, what is the real reason for this? It feels like we have so many different ways to blacklist and unbind and bind devices to drivers already, why add yet-another way?
thanks,
greg k-h
Re: [RFC 1/3] PCI: pci-driver: Introduce pci device delete list
Hi Greg,
Quoted 40 lines · Greg Kroah-Hartman
Quoted 7 lines · Jon Derrick
This patch introduces a new kernel command line parameter to mask pci device ids from pci driver id tables. This prevents masked devices from automatically binding to both built-in and module drivers.
Devices can be later attached through the driver's sysfs
new_idinteface.Quoted 6 lines
The use cases for this are primarily for debugging, eg, being able to prevent attachment before probes are set up. It can also be used to mask off faulty built-in hardware or faulty simulated hardware.
Another use case is to prevent attachment of devices which will be passed to VMs, shortcutting the detachment effort.
Is the "shortcut" really that big of a deal? unbind actually causes problems? Is this an attempt to deal with devices being handled by more than one driver and then you want to manually bind it later on?
Quoted 13 lines
The format is similar to the sysfs
new_idformat. Device ids are specified with:VVVV:DDDD[:SVVV:SDDD][:CCCC][:MMMM]
Where:
VVVV = Vendor ID
DDDD = Device ID
SVVV = Subvendor ID
SDDD = Subdevice ID
CCCC = Class
MMMM = Class MaskQuoted 7 lines
IDs can be chained with commas.
Examples:
<driver>.delete_id=1234:5678 <driver>.delete_id=ffffffff:ffffffff <driver>.delete_id=ffffffff:ffffffff:ffffffff:ffffffff:010802 <driver>.delete_id=1234:5678,abcd:ef01,2345:ffffffff
Quoted 4 lines
What about drivers that handle more than one bus type (i.e. USB and PCI?) This format is specific to PCI, yet you are defining it as a "global" for all drivers :(
I was hoping to extend it to other bus types as well, but just wanted some early feedback on the idea. Pci was the easier implementation for me. Could prepending pci:, usb:, etc on the format be an option?
Quoted 4 lines
This feels hacky, what is the real reason for this? It feels like we have so many different ways to blacklist and unbind and bind devices to drivers already, why add yet-another way?
I ran into an issue a while back where I needed to disable a device from a built-in driver to perform a regression test. I worked around that issue by doing an initcall_blacklist on the pci-driver declaration, but that also preventing later binding because the driver was never registered.
I've also had issues with remote systems where pluggable devices were broken or otherwise non-responsive, and it would have been nice to have been able to keep them from binding on module loading as a temporary workaround until someone could have removed those devices (though impossible on built-in hardware).
I'm not sure about hacky; I think the implementation in this patch (1/3) is pretty clean :). I'm not familiar with all the different ways of blacklisting. Is there another way to work around the issues I listed above?
I understand the concern about having it exist in the format <driver>. and only supporting one or a few bus types. I have another set that extends the pci= parameter instead.
Quoted 4 lines
thanks,
greg k-h
Best,
Jon
Re: [RFC 1/3] PCI: pci-driver: Introduce pci device delete list
In reply to Greg Kroah-HartmanQuoted 40 lines · Greg Kroah-Hartman
Quoted 7 lines · Jon Derrick
This patch introduces a new kernel command line parameter to mask pci device ids from pci driver id tables. This prevents masked devices from automatically binding to both built-in and module drivers.
Devices can be later attached through the driver's sysfs
new_idinteface.Quoted 6 lines
The use cases for this are primarily for debugging, eg, being able to prevent attachment before probes are set up. It can also be used to mask off faulty built-in hardware or faulty simulated hardware.
Another use case is to prevent attachment of devices which will be passed to VMs, shortcutting the detachment effort.
Is the "shortcut" really that big of a deal? unbind actually causes problems? Is this an attempt to deal with devices being handled by more than one driver and then you want to manually bind it later on?
Quoted 13 lines
The format is similar to the sysfs
new_idformat. Device ids are specified with:VVVV:DDDD[:SVVV:SDDD][:CCCC][:MMMM]
Where:
VVVV = Vendor ID
DDDD = Device ID
SVVV = Subvendor ID
SDDD = Subdevice ID
CCCC = Class
MMMM = Class MaskQuoted 7 lines
IDs can be chained with commas.
Examples:
<driver>.delete_id=1234:5678 <driver>.delete_id=ffffffff:ffffffff <driver>.delete_id=ffffffff:ffffffff:ffffffff:ffffffff:010802 <driver>.delete_id=1234:5678,abcd:ef01,2345:ffffffff
Quoted 3 lines
What about drivers that handle more than one bus type (i.e. USB and PCI?) This format is specific to PCI, yet you are defining it as a "global" for all drivers :(
I assume other buses could define their own "delete_id" format and it's up to those bus_type implementations to check for "delete_id" statements for the drivers attached to it. Somewhat similar to what we have for "new_id" where it appears to be global sysfs attribute, but implemented per-bus.
Quoted 3 lines
This feels hacky, what is the real reason for this? It feels like we have so many different ways to blacklist and unbind and bind devices to drivers already, why add yet-another way?
Unbind after the fact may be too late, and builtin-drivers eliminate modprobe blacklisting. I've missed having this functionality in the past for platform bring up.
RFC 2/3 module: Ignore delete_id parameter
In reply to Jon DerrickThe PCI driver delete_id parameter is handled in each individual driver registration callback.
--- kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/module.c +7 -0
@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,return 0 ; } +/* +* Ignore driver delete list arguments. They are handled by driver +* registration callbacks +*/ +if (strcmp (param,"delete_id" ) ==0 ) +return 0 ; +/* Check for magic 'dyndbg' arg */ ret =ddebug_dyndbg_module_param_cb (param, val, modname);if (ret !=0 )
Signature · 1 line
1.8.3.1
Re: [RFC 2/3] module: Ignore delete_id parameter
Quoted 13 lines · Jon Derrick
The PCI driver
delete_idparameter is handled in each individual driver registration callback.Signed-off-by Jon Derrick <[email protected]>--- kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/module.c +0
-0
kernel/module.c static int unknown_module_param_cb(char *param, char *val, const char *modname, · line 3629
Quoted 10 lines
kernel/module.c +7
-0@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,return 0 ; } +/* +* Ignore driver delete list arguments. They are handled by driver +* registration callbacks +*/ +if (strcmp (param,"delete_id" ) ==0 ) +return 0 ; +
Does this preclude something like:
modprobe ahci delete_id=1234:5678?
Re: [RFC 2/3] module: Ignore delete_id parameter
Quoted 27 lines · Dan Williams
Quoted 13 lines · Jon Derrick
The PCI driver
delete_idparameter is handled in each individual driver registration callback.Signed-off-by Jon Derrick <[email protected]>--- kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/module.c +0
-0kernel/module.c static int unknown_module_param_cb(char *param, char *val, const char *modname, · line 3629
Quoted 10 lines
kernel/module.c +7
-0@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,return 0 ; } +/* +* Ignore driver delete list arguments. They are handled by driver +* registration callbacks +*/ +if (strcmp (param,"delete_id" ) ==0 ) +return 0 ; +Does this preclude something like:
Quoted 2 lines
modprobe ahci delete_id=1234:5678?
It does seem like it would. I can look into calling into the pci callback for this, but val is a struct module here and I haven't figured out the plumbing to get the [correct] driver from that.
Maybe if I enforce the format of 'modprobe ahci ahci.delete_id=xxxx' to ensure the driver is specified (and would be required in cases with multi-driver modules).
Re: [RFC 2/3] module: Ignore delete_id parameter
In reply to Jon DerrickQuoted 13 lines · Jon Derrick
The PCI driver
delete_idparameter is handled in each individual driver registration callback.Signed-off-by Jon Derrick <[email protected]>--- kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/module.c +0
-0
kernel/module.c static int unknown_module_param_cb(char *param, char *val, const char *modname, · line 3628
Quoted 9 lines
kernel/module.c +6
-0@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,return 0 ; } +/* +* Ignore driver delete list arguments. They are handled by driver +* registration callbacks +*/ +if (strcmp (param,"delete_id" ) ==0 ) +return 0 ;
Why? This is only for the PCI core as you have defined it in this patchset, but you just broke this module id for all other kernel modules in the system :(
If you want to do this, you need to provide this feature for _all_ kernel drivers...
thanks,
greg k-h
Re: [RFC 2/3] module: Ignore delete_id parameter
Quoted 28 lines · Greg Kroah-Hartman
Quoted 13 lines · Jon Derrick
The PCI driver
delete_idparameter is handled in each individual driver registration callback.Signed-off-by Jon Derrick <[email protected]>--- kernel/module.c | 7 +++++++ 1 file changed, 7 insertions(+)kernel/module.c +0
-0kernel/module.c static int unknown_module_param_cb(char *param, char *val, const char *modname, · line 3628
Quoted 9 lines
kernel/module.c +6
-0@@ -3620,6 +3620,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,return 0 ; } +/* +* Ignore driver delete list arguments. They are handled by driver +* registration callbacks +*/ +if (strcmp (param,"delete_id" ) ==0 ) +return 0 ;Why? This is only for the PCI core as you have defined it in this patchset, but you just broke this module id for all other kernel modules in the system :(
Quoted 7 lines
If you want to do this, you need to provide this feature for _all_ kernel drivers...
thanks,
greg k-h
Yes I'm not particularly happy about this one either. I will make this more robust if the blacklisting idea is sound.
RFC 3/3 Documentation: Add pci device delete_id interface
In reply to Jon DerrickDocument the <driver>.delete_id= cmdline interface for masking built-in and module device id tables.
--- Documentation/admin-guide/kernel-parameters.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +13 -0
@@ -789,6 +789,19 @@ Defaults to the default architecture's huge page size if not specified. + driver.delete_id= + [PCI] + Specified pci device format(s) will mask built-in + driver id tables, preventing devices from + automicatically attaching to built-in or module + drivers. Later attachment should be done by adding the + deleted id back through the "new_id" sysfs interface. + Format: <vendor>:<device>[:<subvendor>:<subdevice>]\ + [:class][:class_mask][, ...] + PCI_ANY_ID masks should use the full 32-bit format. + Examples: virtio-pci.delete_id=ffffffff:ffffffff + nvme.delete_id=1234:5678,1111:ffffffff + dhash_entries= [KNL] Set number of hash buckets for dentry cache.
Signature · 1 line
1.8.3.1