* [PATCH 2/2] iov: Use pci_walk_vbus for tasks where we must search for VFs
2014-05-28 22:29 [PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV Alexander Duyck
@ 2014-05-28 22:29 ` Alexander Duyck
2014-05-29 23:00 ` [PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Duyck @ 2014-05-28 22:29 UTC (permalink / raw)
To: bhelgaas, ddutile; +Cc: linux-pci, alex.williamson, linux-kernel
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);
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV
2014-05-28 22:29 [PATCH 1/2] pci: Add pci_walk_vbus for walking virtual busses associated with SR-IOV Alexander Duyck
2014-05-28 22:29 ` [PATCH 2/2] iov: Use pci_walk_vbus for tasks where we must search for VFs Alexander Duyck
@ 2014-05-29 23:00 ` Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2014-05-29 23:00 UTC (permalink / raw)
To: Alexander Duyck; +Cc: ddutile, linux-pci, alex.williamson, linux-kernel
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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread