2009-09-03 06:26:51

by Tejun Heo

[permalink] [raw]
Subject: [PATCH updated 1/2] pci: separate out pci_add_dynid()

Separate out pci_add_dynid() from store_new_id() and export it so that
in-kernel code can add PCI IDs dynamically. As the function will be
available regardless of HOTPLUG, put it and pull pci_free_dynids()
outside of CONFIG_HOTPLUG.

This will be used by pci-stub to initialize initial IDs via module
param.

While at it, remove bogus get_driver() failure check.

Signed-off-by: Tejun Heo <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Grant Grundler <[email protected]>
---
These two patches allow preventing built-in drivers from attaching to
devices with certain IDs. EXPORT_SYMBOL() changed to
EXPORT_SYMBOL_GPL() as suggested by Greg and patch description updated
to mention drop of the bogus failure condition.

Thanks.

drivers/pci/pci-driver.c | 119 +++++++++++++++++++++++++++++------------------
include/linux/pci.h | 5 +
2 files changed, 79 insertions(+), 45 deletions(-)

Index: ata/drivers/pci/pci-driver.c
===================================================================
--- ata.orig/drivers/pci/pci-driver.c
+++ ata/drivers/pci/pci-driver.c
@@ -19,37 +19,98 @@
#include <linux/cpu.h>
#include "pci.h"

-/*
- * Dynamic device IDs are disabled for !CONFIG_HOTPLUG
- */
-
struct pci_dynid {
struct list_head node;
struct pci_device_id id;
};

-#ifdef CONFIG_HOTPLUG
+/**
+ * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
+ * @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
+ * @driver_data: private driver data
+ *
+ * Adds a new dynamic pci device ID to this driver and causes the
+ * driver to probe for all devices again. @drv must have been
+ * registered prior to calling this function.
+ *
+ * CONTEXT:
+ * Does GFP_KERNEL allocation.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int pci_add_dynid(struct pci_driver *drv,
+ unsigned int vendor, unsigned int device,
+ unsigned int subvendor, unsigned int subdevice,
+ unsigned int class, unsigned int class_mask,
+ unsigned long driver_data)
+{
+ struct pci_dynid *dynid;
+ int retval;
+
+ dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
+ if (!dynid)
+ return -ENOMEM;
+
+ dynid->id.vendor = vendor;
+ dynid->id.device = device;
+ dynid->id.subvendor = subvendor;
+ dynid->id.subdevice = subdevice;
+ dynid->id.class = class;
+ dynid->id.class_mask = class_mask;
+ dynid->id.driver_data = driver_data;

+ spin_lock(&drv->dynids.lock);
+ list_add_tail(&dynid->node, &drv->dynids.list);
+ spin_unlock(&drv->dynids.lock);
+
+ get_driver(&drv->driver);
+ retval = driver_attach(&drv->driver);
+ put_driver(&drv->driver);
+
+ return retval;
+}
+
+static void pci_free_dynids(struct pci_driver *drv)
+{
+ struct pci_dynid *dynid, *n;
+
+ spin_lock(&drv->dynids.lock);
+ list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
+ list_del(&dynid->node);
+ kfree(dynid);
+ }
+ spin_unlock(&drv->dynids.lock);
+}
+
+/*
+ * Dynamic device ID manipulation via sysfs is disabled for !CONFIG_HOTPLUG
+ */
+#ifdef CONFIG_HOTPLUG
/**
- * store_new_id - add a new PCI device ID to this driver and re-probe devices
+ * store_new_id - sysfs frontend to pci_add_dynid()
* @driver: target device driver
* @buf: buffer for scanning device ID data
* @count: input size
*
- * Adds a new dynamic pci device ID to this driver,
- * and causes the driver to probe for all devices again.
+ * Allow PCI IDs to be added to an existing driver via sysfs.
*/
static ssize_t
store_new_id(struct device_driver *driver, const char *buf, size_t count)
{
- struct pci_dynid *dynid;
struct pci_driver *pdrv = to_pci_driver(driver);
const struct pci_device_id *ids = pdrv->id_table;
__u32 vendor, device, subvendor=PCI_ANY_ID,
subdevice=PCI_ANY_ID, class=0, class_mask=0;
unsigned long driver_data=0;
int fields=0;
- int retval=0;
+ int retval;

fields = sscanf(buf, "%x %x %x %x %x %x %lx",
&vendor, &device, &subvendor, &subdevice,
@@ -72,27 +133,8 @@ store_new_id(struct device_driver *drive
return retval;
}

- dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
- if (!dynid)
- return -ENOMEM;
-
- dynid->id.vendor = vendor;
- dynid->id.device = device;
- dynid->id.subvendor = subvendor;
- dynid->id.subdevice = subdevice;
- dynid->id.class = class;
- dynid->id.class_mask = class_mask;
- dynid->id.driver_data = driver_data;
-
- spin_lock(&pdrv->dynids.lock);
- list_add_tail(&dynid->node, &pdrv->dynids.list);
- spin_unlock(&pdrv->dynids.lock);
-
- if (get_driver(&pdrv->driver)) {
- retval = driver_attach(&pdrv->driver);
- put_driver(&pdrv->driver);
- }
-
+ retval = pci_add_dynid(pdrv, vendor, device, subvendor, subdevice,
+ class, class_mask, driver_data);
if (retval)
return retval;
return count;
@@ -145,19 +187,6 @@ store_remove_id(struct device_driver *dr
}
static DRIVER_ATTR(remove_id, S_IWUSR, NULL, store_remove_id);

-static void
-pci_free_dynids(struct pci_driver *drv)
-{
- struct pci_dynid *dynid, *n;
-
- spin_lock(&drv->dynids.lock);
- list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
- list_del(&dynid->node);
- kfree(dynid);
- }
- spin_unlock(&drv->dynids.lock);
-}
-
static int
pci_create_newid_file(struct pci_driver *drv)
{
@@ -186,7 +215,6 @@ static void pci_remove_removeid_file(str
driver_remove_file(&drv->driver, &driver_attr_remove_id);
}
#else /* !CONFIG_HOTPLUG */
-static inline void pci_free_dynids(struct pci_driver *drv) {}
static inline int pci_create_newid_file(struct pci_driver *drv)
{
return 0;
@@ -1106,6 +1134,7 @@ static int __init pci_driver_init(void)

postcore_initcall(pci_driver_init);

+EXPORT_SYMBOL_GPL(pci_add_dynid);
EXPORT_SYMBOL(pci_match_id);
EXPORT_SYMBOL(__pci_register_driver);
EXPORT_SYMBOL(pci_unregister_driver);
Index: ata/include/linux/pci.h
===================================================================
--- ata.orig/include/linux/pci.h
+++ ata/include/linux/pci.h
@@ -794,6 +794,11 @@ int __must_check __pci_register_driver(s
void pci_unregister_driver(struct pci_driver *dev);
void pci_remove_behind_bridge(struct pci_dev *dev);
struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+int pci_add_dynid(struct pci_driver *drv,
+ unsigned int vendor, unsigned int device,
+ unsigned int subvendor, unsigned int subdevice,
+ unsigned int class, unsigned int class_mask,
+ unsigned long driver_data);
const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
struct pci_dev *dev);
int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,


2009-09-03 06:27:39

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/2] pci-stub: add pci_stub.ids parameter

Add ids module parameter which allows specifying initial IDs for the
pci-stub driver. When built into the kernel, pci-stub is linked
before any real pci drivers and by setting up IDs from initialization
it can prevent built-in drivers from attaching to specific devices.

While at it, make pci_stub_probe() print out about devices it grabbed
to weed out "but my controller isn't being probed" bug reports.

Signed-off-by: Tejun Heo <[email protected]>
---
Unchanged from the first posting.

Thanks.

drivers/pci/pci-stub.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)

Index: ata/drivers/pci/pci-stub.c
===================================================================
--- ata.orig/drivers/pci/pci-stub.c
+++ ata/drivers/pci/pci-stub.c
@@ -19,8 +19,16 @@
#include <linux/module.h>
#include <linux/pci.h>

+static char ids[1024] __initdata;
+
+module_param_string(ids, ids, sizeof(ids), 0);
+MODULE_PARM_DESC(ids, "Initial PCI IDs to add to the stub driver, format is "
+ "\"vendor:device[:subvendor[:subdevice[:class[:class_mask]]]]\""
+ " and multiple comma separated entries can be specified");
+
static int pci_stub_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
+ dev_printk(KERN_INFO, &dev->dev, "claimed by stub\n");
return 0;
}

@@ -32,7 +40,42 @@ static struct pci_driver stub_driver = {

static int __init pci_stub_init(void)
{
- return pci_register_driver(&stub_driver);
+ char *p, *id;
+ int rc;
+
+ rc = pci_register_driver(&stub_driver);
+ if (rc)
+ return rc;
+
+ /* add ids specified in the module parameter */
+ p = ids;
+ while ((id = strsep(&p, ","))) {
+ unsigned int vendor, device, subvendor = PCI_ANY_ID,
+ subdevice = PCI_ANY_ID, class=0, class_mask=0;
+ int fields;
+
+ fields = sscanf(id, "%x:%x:%x:%x:%x:%x",
+ &vendor, &device, &subvendor, &subdevice,
+ &class, &class_mask);
+
+ if (fields < 2) {
+ printk(KERN_WARNING
+ "pci-stub: invalid id string \"%s\"\n", id);
+ continue;
+ }
+
+ printk(KERN_INFO
+ "pci-stub: add %04X:%04X sub=%04X:%04X cls=%08X/%08X\n",
+ vendor, device, subvendor, subdevice, class, class_mask);
+
+ rc = pci_add_dynid(&stub_driver, vendor, device,
+ subvendor, subdevice, class, class_mask, 0);
+ if (rc)
+ printk(KERN_WARNING
+ "pci-stub: failed to add dynamic id (%d)\n", rc);
+ }
+
+ return 0;
}

static void __exit pci_stub_exit(void)

2009-09-09 20:45:09

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci-stub: add pci_stub.ids parameter

On Thu, 03 Sep 2009 15:27:27 +0900
Tejun Heo <[email protected]> wrote:

> Add ids module parameter which allows specifying initial IDs for the
> pci-stub driver. When built into the kernel, pci-stub is linked
> before any real pci drivers and by setting up IDs from initialization
> it can prevent built-in drivers from attaching to specific devices.
>
> While at it, make pci_stub_probe() print out about devices it grabbed
> to weed out "but my controller isn't being probed" bug reports.
>
> Signed-off-by: Tejun Heo <[email protected]>
> ---
> Unchanged from the first posting.

Applied these two to linux-next, thanks.

--
Jesse Barnes, Intel Open Source Technology Center