diff mbox

[2/2,v2] PCI: add remove_id sysfs entry

Message ID 20090224055223.GG7436@sequoia.sous-sol.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Chris Wright Feb. 24, 2009, 5:52 a.m. UTC 
This adds a remove_id sysfs entry to allow users of new_id to later
remove the added dynid.  One use case is management tools that want to
dynamically bind/unbind devices to pci-stub driver while devices are
assigned to KVM guests.  Rather than having to track which driver was
originally bound to the driver, a mangement tool can simply:

# echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
# echo 0000:00:19.0 > /sys/bus/pci/devices/0000:00:19.0/driver/unbind
# echo 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind

Guest uses device

# echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/remove_id
# echo 0000:00:19.0 > /sys/bus/pci/drivers_probe

Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---
v2
 - update changelog to match ABI docs (remove echo -n)

 Documentation/ABI/testing/sysfs-bus-pci |   16 ++++++
 drivers/pci/pci-driver.c                |   80 +++++++++++++++++++++++++++++++-
 2 files changed, 94 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Han, Weidong Feb. 26, 2009, 5:37 a.m. UTC | #1 
Chris Wright wrote:
> This adds a remove_id sysfs entry to allow users of new_id to later
> remove the added dynid.  One use case is management tools that want to
> dynamically bind/unbind devices to pci-stub driver while devices are
> assigned to KVM guests.  Rather than having to track which driver was
> originally bound to the driver, a mangement tool can simply:
> 
> # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
> # echo 0000:00:19.0 > /sys/bus/pci/devices/0000:00:19.0/driver/unbind
> # echo 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
> 
> Guest uses device
> 
> # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/remove_id
> # echo 0000:00:19.0 > /sys/bus/pci/drivers_probe
> 

After above two commands, I found device 00:19.0 was still bound to pci-stub, and doesn't work. I found it needs following unbind command between remove_id and drivers_probe to make it work with original driver:
  # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/unbind

Chris, did it also happen on your side? or did I miss something? 

Regards,
Weidong

> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> ---
> v2
>  - update changelog to match ABI docs (remove echo -n)
> 
>  Documentation/ABI/testing/sysfs-bus-pci |   16 ++++++
>  drivers/pci/pci-driver.c                |   80
>  +++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+),
> 2 deletions(-) 
> 
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -99,6 +99,52 @@ store_new_id(struct device_driver *drive
>  }
>  static DRIVER_ATTR(new_id, S_IWUSR, NULL, store_new_id);
> 
> +/**
> + * store_remove_id - remove a PCI device ID from this driver
> + * @driver: target device driver
> + * @buf: buffer for scanning device ID data
> + * @count: input size
> + *
> + * Removes a dynamic pci device ID to this driver.
> + */
> +static ssize_t
> +store_remove_id(struct device_driver *driver, const char *buf,
> size_t count) +{
> +	struct pci_dynid *dynid, *n;
> +	struct pci_driver *pdrv = to_pci_driver(driver);
> +	__u32 vendor, device, subvendor = PCI_ANY_ID,
> +		subdevice = PCI_ANY_ID, class = 0, class_mask = 0;
> +	int fields = 0;
> +	int retval = -ENODEV;
> +
> +	fields = sscanf(buf, "%x %x %x %x %x %x",
> +			&vendor, &device, &subvendor, &subdevice,
> +			&class, &class_mask);
> +	if (fields < 2)
> +		return -EINVAL;
> +
> +	spin_lock(&pdrv->dynids.lock);
> +	list_for_each_entry_safe(dynid, n, &pdrv->dynids.list, node) {
> +		struct pci_device_id *id = &dynid->id;
> +		if ((id->vendor == vendor) &&
> +		    (id->device == device) &&
> +		    (subvendor == PCI_ANY_ID || id->subvendor == subvendor) &&
> +		    (subdevice == PCI_ANY_ID || id->subdevice == subdevice) &&
> +		    !((id->class ^ class) & class_mask)) {
> +			list_del(&dynid->node);
> +			kfree(dynid);
> +			retval = 0;
> +			break;
> +		}
> +	}
> +	spin_unlock(&pdrv->dynids.lock);
> +
> +	if (retval)
> +		return retval;
> +	return count;
> +}
> +static DRIVER_ATTR(remove_id, S_IWUSR, NULL, store_remove_id);
> +
>  static void
>  pci_free_dynids(struct pci_driver *drv)
>  {
> @@ -125,6 +171,20 @@ static void pci_remove_newid_file(struct
>  {
>  	driver_remove_file(&drv->driver, &driver_attr_new_id);
>  }
> +
> +static int
> +pci_create_removeid_file(struct pci_driver *drv)
> +{
> +	int error = 0;
> +	if (drv->probe != NULL)
> +		error = driver_create_file(&drv->driver,&driver_attr_remove_id);
> +	return error;
> +}
> +
> +static void pci_remove_removeid_file(struct pci_driver *drv)
> +{
> +	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)
> @@ -132,6 +192,11 @@ static inline int pci_create_newid_file(
>  	return 0;
>  }
>  static inline void pci_remove_newid_file(struct pci_driver *drv) {}
> +static inline int pci_create_removeid_file(struct pci_driver *drv)
> +{
> +	return 0;
> +}
> +static inline void pci_remove_removeid_file(struct pci_driver *drv)
>  {} #endif
> 
>  /**
> @@ -852,13 +917,23 @@ int __pci_register_driver(struct pci_dri
>  	/* register with core */
>  	error = driver_register(&drv->driver);
>  	if (error)
> -		return error;
> +		goto out;
> 
>  	error = pci_create_newid_file(drv);
>  	if (error)
> -		driver_unregister(&drv->driver);
> +		goto out_newid;
> 
> +	error = pci_create_removeid_file(drv);
> +	if (error)
> +		goto out_removeid;
> +out:
>  	return error;
> +
> +out_removeid:
> +	pci_remove_newid_file(drv);
> +out_newid:
> +	driver_unregister(&drv->driver);
> +	goto out;
>  }
> 
>  /**
> @@ -874,6 +949,7 @@ int __pci_register_driver(struct pci_dri
>  void
>  pci_unregister_driver(struct pci_driver *drv)
>  {
> +	pci_remove_removeid_file(drv);
>  	pci_remove_newid_file(drv);
>  	driver_unregister(&drv->driver);
>  	pci_free_dynids(drv);
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -41,6 +41,22 @@ Description:
>  		for the device and attempt to bind to it.  For example:
>  		# echo "8086 10f5" > /sys/bus/pci/drivers/foo/new_id
> 
> +What:		/sys/bus/pci/drivers/.../remove_id
> +Date:		February 2009
> +Contact:	Chris Wright <chrisw@sous-sol.org>
> +Description:
> +		Writing a device ID to this file will remove an ID
> +		that was dynamically added via the new_id sysfs entry.
> +		The format for the device ID is:
> +		VVVV DDDD SVVV SDDD CCCC MMMM.	That is Vendor ID, Device
> +		ID, Subsystem Vendor ID, Subsystem Device ID, Class,
> +		and Class Mask.  The Vendor ID and Device ID fields are
> +		required, the rest are optional.  After successfully
> +		removing an ID, the driver will no longer support the
> +		device.  This is useful to ensure auto probing won't
> +		match the driver to the device.  For example:
> +		# echo "8086 10f5" > /sys/bus/pci/drivers/foo/remove_id
> +
>  What:		/sys/bus/pci/devices/.../vpd
>  Date:		February 2008
>  Contact:	Ben Hutchings <bhutchings@solarflare.com>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Wright Feb. 27, 2009, 12:27 a.m. UTC | #2 
* Han, Weidong (weidong.han@intel.com) wrote:
> Chris Wright wrote:
> > This adds a remove_id sysfs entry to allow users of new_id to later
> > remove the added dynid.  One use case is management tools that want to
> > dynamically bind/unbind devices to pci-stub driver while devices are
> > assigned to KVM guests.  Rather than having to track which driver was
> > originally bound to the driver, a mangement tool can simply:
> > 
> > # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
> > # echo 0000:00:19.0 > /sys/bus/pci/devices/0000:00:19.0/driver/unbind
> > # echo 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
> > 
> > Guest uses device
> > 
> > # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/remove_id
> > # echo 0000:00:19.0 > /sys/bus/pci/drivers_probe
> 
> After above two commands, I found device 00:19.0 was still bound to pci-stub, and doesn't work. I found it needs following unbind command between remove_id and drivers_probe to make it work with original driver:
>   # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/unbind
> 
> Chris, did it also happen on your side? or did I miss something? 

You're right.  I just forgot to put that in the changelog.

thanks,
-chris
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesse Barnes March 20, 2009, 12:35 a.m. UTC | #3 
On Mon, 23 Feb 2009 21:52:23 -0800
Chris Wright <chrisw@sous-sol.org> wrote:

> This adds a remove_id sysfs entry to allow users of new_id to later
> remove the added dynid.  One use case is management tools that want to
> dynamically bind/unbind devices to pci-stub driver while devices are
> assigned to KVM guests.  Rather than having to track which driver was
> originally bound to the driver, a mangement tool can simply:
> 
> # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
> # echo 0000:00:19.0 > /sys/bus/pci/devices/0000:00:19.0/driver/unbind
> # echo 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
> 
> Guest uses device
> 
> # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/remove_id
> # echo 0000:00:19.0 > /sys/bus/pci/drivers_probe
> 
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>

Applied to linux-next, thanks Chris.
diff mbox

Patch

--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -99,6 +99,52 @@  store_new_id(struct device_driver *drive
 }
 static DRIVER_ATTR(new_id, S_IWUSR, NULL, store_new_id);
 
+/**
+ * store_remove_id - remove a PCI device ID from this driver
+ * @driver: target device driver
+ * @buf: buffer for scanning device ID data
+ * @count: input size
+ *
+ * Removes a dynamic pci device ID to this driver.
+ */
+static ssize_t
+store_remove_id(struct device_driver *driver, const char *buf, size_t count)
+{
+	struct pci_dynid *dynid, *n;
+	struct pci_driver *pdrv = to_pci_driver(driver);
+	__u32 vendor, device, subvendor = PCI_ANY_ID,
+		subdevice = PCI_ANY_ID, class = 0, class_mask = 0;
+	int fields = 0;
+	int retval = -ENODEV;
+
+	fields = sscanf(buf, "%x %x %x %x %x %x",
+			&vendor, &device, &subvendor, &subdevice,
+			&class, &class_mask);
+	if (fields < 2)
+		return -EINVAL;
+
+	spin_lock(&pdrv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &pdrv->dynids.list, node) {
+		struct pci_device_id *id = &dynid->id;
+		if ((id->vendor == vendor) &&
+		    (id->device == device) &&
+		    (subvendor == PCI_ANY_ID || id->subvendor == subvendor) &&
+		    (subdevice == PCI_ANY_ID || id->subdevice == subdevice) &&
+		    !((id->class ^ class) & class_mask)) {
+			list_del(&dynid->node);
+			kfree(dynid);
+			retval = 0;
+			break;
+		}
+	}
+	spin_unlock(&pdrv->dynids.lock);
+
+	if (retval)
+		return retval;
+	return count;
+}
+static DRIVER_ATTR(remove_id, S_IWUSR, NULL, store_remove_id);
+
 static void
 pci_free_dynids(struct pci_driver *drv)
 {
@@ -125,6 +171,20 @@  static void pci_remove_newid_file(struct
 {
 	driver_remove_file(&drv->driver, &driver_attr_new_id);
 }
+
+static int
+pci_create_removeid_file(struct pci_driver *drv)
+{
+	int error = 0;
+	if (drv->probe != NULL)
+		error = driver_create_file(&drv->driver,&driver_attr_remove_id);
+	return error;
+}
+
+static void pci_remove_removeid_file(struct pci_driver *drv)
+{
+	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)
@@ -132,6 +192,11 @@  static inline int pci_create_newid_file(
 	return 0;
 }
 static inline void pci_remove_newid_file(struct pci_driver *drv) {}
+static inline int pci_create_removeid_file(struct pci_driver *drv)
+{
+	return 0;
+}
+static inline void pci_remove_removeid_file(struct pci_driver *drv) {}
 #endif
 
 /**
@@ -852,13 +917,23 @@  int __pci_register_driver(struct pci_dri
 	/* register with core */
 	error = driver_register(&drv->driver);
 	if (error)
-		return error;
+		goto out;
 
 	error = pci_create_newid_file(drv);
 	if (error)
-		driver_unregister(&drv->driver);
+		goto out_newid;
 
+	error = pci_create_removeid_file(drv);
+	if (error)
+		goto out_removeid;
+out:
 	return error;
+
+out_removeid:
+	pci_remove_newid_file(drv);
+out_newid:
+	driver_unregister(&drv->driver);
+	goto out;
 }
 
 /**
@@ -874,6 +949,7 @@  int __pci_register_driver(struct pci_dri
 void
 pci_unregister_driver(struct pci_driver *drv)
 {
+	pci_remove_removeid_file(drv);
 	pci_remove_newid_file(drv);
 	driver_unregister(&drv->driver);
 	pci_free_dynids(drv);
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -41,6 +41,22 @@  Description:
 		for the device and attempt to bind to it.  For example:
 		# echo "8086 10f5" > /sys/bus/pci/drivers/foo/new_id
 
+What:		/sys/bus/pci/drivers/.../remove_id
+Date:		February 2009
+Contact:	Chris Wright <chrisw@sous-sol.org>
+Description:
+		Writing a device ID to this file will remove an ID
+		that was dynamically added via the new_id sysfs entry.
+		The format for the device ID is:
+		VVVV DDDD SVVV SDDD CCCC MMMM.	That is Vendor ID, Device
+		ID, Subsystem Vendor ID, Subsystem Device ID, Class,
+		and Class Mask.  The Vendor ID and Device ID fields are
+		required, the rest are optional.  After successfully
+		removing an ID, the driver will no longer support the
+		device.  This is useful to ensure auto probing won't
+		match the driver to the device.  For example:
+		# echo "8086 10f5" > /sys/bus/pci/drivers/foo/remove_id
+
 What:		/sys/bus/pci/devices/.../vpd
 Date:		February 2008
 Contact:	Ben Hutchings <bhutchings@solarflare.com>