[PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV

[PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV

[Alexander Duyck]

This function provides a simple means for applying a given function to all
devices on bus and all of it's children, including virtual busses.  To do
this the function begins by processing all devices on the bus, then it will
proceed through bus->children and process each of the child busses.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/pci/bus.c   |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    2 ++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index fb8aed3..769bbcb 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -336,6 +336,50 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
 }
 EXPORT_SYMBOL_GPL(pci_walk_bus);
 
+/** pci_walk_vbus - walk devices on/under bus, calling callback.
+ *  @top      bus whose devices should be walked
+ *  @cb       callback to be called for each device found
+ *  @userdata arbitrary pointer to be passed to callback.
+ *
+ *  Walk the given bus, including any child buses under this bus.
+ *  Call the provided callback on each device found.
+ *
+ *  We check the return of @cb each time. If it returns anything
+ *  other than 0, we break out.
+ *
+ */
+void pci_walk_vbus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
+		   void *userdata)
+{
+	struct list_head *bus_next;
+	struct pci_bus *bus;
+	struct pci_dev *dev;
+
+	down_read(&pci_bus_sem);
+	bus_next = &top->node;
+	for (;;) {
+		/* prep bus for next iteration */
+		bus = list_entry(bus_next, struct pci_bus, node);
+
+		/* process each device on this bus */
+		list_for_each_entry(dev, &bus->devices, bus_list) {
+			if (cb(dev, userdata))
+				goto release_semaphore;
+		}
+
+		/* end of this bus, go to child, next bus, or finish */
+		for (bus_next = bus->children.next;
+		     bus_next == &bus->children;
+		     bus_next = bus->node.next, bus = bus->parent) {
+			if (bus == top)
+				goto release_semaphore;
+		}
+	}
+release_semaphore:
+	up_read(&pci_bus_sem);
+}
+EXPORT_SYMBOL_GPL(pci_walk_vbus);
+
 struct pci_bus *pci_bus_get(struct pci_bus *bus)
 {
 	if (bus)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..1fb18a8 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1118,6 +1118,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
 		  void *userdata);
+void pci_walk_vbus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
+		   void *userdata);
 int pci_cfg_space_size(struct pci_dev *dev);
 unsigned char pci_bus_max_busnr(struct pci_bus *bus);
 void pci_setup_bridge(struct pci_bus *bus);

[PATCH 2/2] iov: Use pci_walk_vbus for tasks where we must search for VFs

[Alexander Duyck]

This change makes it so that we use pci_walk_vbus to go though and search
for all devices that are contained within the local bus of our device.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/pci/iov.c |   50 +++++++++++++++++++++++++++-----------------------
 1 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..717f202 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -602,6 +602,30 @@ int pci_num_vf(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_num_vf);
 
+struct vfs_assigned {
+	struct pci_dev *physfn;
+	unsigned int count;
+};
+
+/**
+ * pci_vfs_assigned_cb - callback used by pci_vfs_assigned
+ * @dev: the PCI device
+ * @data: Pointer to structure used by pci_vfs_assigned
+ *
+ * If the device is a virtual function and belongs to the provided PF
+ * and is assigned this will increment the count by 1.
+ */
+static int pci_vfs_assigned_cb(struct pci_dev *dev, void *data)
+{
+	struct vfs_assigned *vfa = data;
+
+	if (dev->is_virtfn && (dev->physfn == vfa->physfn) &&
+	    (dev->dev_flags & PCI_DEV_FLAGS_ASSIGNED))
+		vfa->count++;
+
+	return 0;
+}
+
 /**
  * pci_vfs_assigned - returns number of VFs are assigned to a guest
  * @dev: the PCI device
@@ -611,35 +635,15 @@ EXPORT_SYMBOL_GPL(pci_num_vf);
  */
 int pci_vfs_assigned(struct pci_dev *dev)
 {
-	struct pci_dev *vfdev;
-	unsigned int vfs_assigned = 0;
-	unsigned short dev_id;
+	struct vfs_assigned vfa = { .physfn = dev, .count = 0 };
 
 	/* only search if we are a PF */
 	if (!dev->is_physfn)
 		return 0;
 
-	/*
-	 * determine the device ID for the VFs, the vendor ID will be the
-	 * same as the PF so there is no need to check for that one
-	 */
-	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_VF_DID, &dev_id);
-
-	/* loop through all the VFs to see if we own any that are assigned */
-	vfdev = pci_get_device(dev->vendor, dev_id, NULL);
-	while (vfdev) {
-		/*
-		 * It is considered assigned if it is a virtual function with
-		 * our dev as the physical function and the assigned bit is set
-		 */
-		if (vfdev->is_virtfn && (vfdev->physfn == dev) &&
-		    (vfdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED))
-			vfs_assigned++;
-
-		vfdev = pci_get_device(dev->vendor, dev_id, vfdev);
-	}
+	pci_walk_vbus(dev->bus, pci_vfs_assigned_cb, &vfa);
 
-	return vfs_assigned;
+	return vfa.count;
 }
 EXPORT_SYMBOL_GPL(pci_vfs_assigned);
 

Re: [PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV

[Bjorn Helgaas]

On Wed, May 28, 2014 at 03:29:09PM -0700, Alexander Duyck wrote:
> This function provides a simple means for applying a given function to all
> devices on bus and all of it's children, including virtual busses.  To do
> this the function begins by processing all devices on the bus, then it will
> proceed through bus->children and process each of the child busses.

Yeah, I think this makes sense.  I forgot that the pci_bus has a list of
all child buses, and the virtual buses *are* on that list.  It's just that
there's no bridge pci_dev where dev->subordinate is the virtual bus, so if
we iterate over the bus->devices list as pci_walk_bus() does, we won't find
a bridge leading to the virtual bus.

But it seems like we could use pci_walk_vbus() as you implemented it here
for both purposes, i.e., we could just change pci_walk_bus() to work this
way.  Would that break anything?  It'd be nicer to just have one "walk bus"
interface, especially since these differ in a pretty subtle way.

I don't really want to elevate the "virtual bus" idea, i.e., the idea that
we have a bus that is not the secondary bus of a bridge, to a first-class
PCI citizen.  It seems like a wart that I'd like to get rid of if we had a
way to do it.

I'm curious about what Windows does for this situation.  I have a dim
memory that it creates some kind of virtual bridge device leading to a bus
like this.  But I have no reference, and maybe I just made that up.

Bjorn

> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  drivers/pci/bus.c   |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    2 ++
>  2 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index fb8aed3..769bbcb 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -336,6 +336,50 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
>  }
>  EXPORT_SYMBOL_GPL(pci_walk_bus);
>  
> +/** pci_walk_vbus - walk devices on/under bus, calling callback.
> + *  @top      bus whose devices should be walked
> + *  @cb       callback to be called for each device found
> + *  @userdata arbitrary pointer to be passed to callback.
> + *
> + *  Walk the given bus, including any child buses under this bus.
> + *  Call the provided callback on each device found.
> + *
> + *  We check the return of @cb each time. If it returns anything
> + *  other than 0, we break out.
> + *
> + */
> +void pci_walk_vbus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
> +		   void *userdata)
> +{
> +	struct list_head *bus_next;
> +	struct pci_bus *bus;
> +	struct pci_dev *dev;
> +
> +	down_read(&pci_bus_sem);
> +	bus_next = &top->node;
> +	for (;;) {
> +		/* prep bus for next iteration */
> +		bus = list_entry(bus_next, struct pci_bus, node);
> +
> +		/* process each device on this bus */
> +		list_for_each_entry(dev, &bus->devices, bus_list) {
> +			if (cb(dev, userdata))
> +				goto release_semaphore;
> +		}
> +
> +		/* end of this bus, go to child, next bus, or finish */
> +		for (bus_next = bus->children.next;
> +		     bus_next == &bus->children;
> +		     bus_next = bus->node.next, bus = bus->parent) {
> +			if (bus == top)
> +				goto release_semaphore;
> +		}
> +	}
> +release_semaphore:
> +	up_read(&pci_bus_sem);
> +}
> +EXPORT_SYMBOL_GPL(pci_walk_vbus);
> +
>  struct pci_bus *pci_bus_get(struct pci_bus *bus)
>  {
>  	if (bus)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index aab57b4..1fb18a8 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1118,6 +1118,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>  
>  void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
>  		  void *userdata);
> +void pci_walk_vbus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
> +		   void *userdata);
>  int pci_cfg_space_size(struct pci_dev *dev);
>  unsigned char pci_bus_max_busnr(struct pci_bus *bus);
>  void pci_setup_bridge(struct pci_bus *bus);
>