diff mbox

[v10,3/7] PCI: reserve bus range for SR-IOV device

Message ID 1235112888-9524-4-git-send-email-yu.zhao@intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Yu Zhao Feb. 20, 2009, 6:54 a.m. UTC
Signed-off-by: Yu Zhao <yu.zhao@intel.com>
---
 drivers/pci/iov.c   |   34 ++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h   |    5 +++++
 drivers/pci/probe.c |    3 +++
 3 files changed, 42 insertions(+), 0 deletions(-)

Comments

Matthew Wilcox March 6, 2009, 8:20 p.m. UTC | #1
On Fri, Feb 20, 2009 at 02:54:44PM +0800, Yu Zhao wrote:
> +static inline void virtfn_bdf(struct pci_dev *dev, int id, u8 *busnr, u8 *devfn)
> +{
> +	u16 bdf;
> +
> +	bdf = (dev->bus->number << 8) + dev->devfn +
> +	      dev->sriov->offset + dev->sriov->stride * id;
> +	*busnr = bdf >> 8;
> +	*devfn = bdf & 0xff;
> +}

I find the interface here a bit clunky -- a function returning void
while having two OUT parameters.  How about this variation on the theme
(viewers are encouraged to come up with their own preferred
implementations and interfaces):

static inline __pure u16 virtfn_bdf(struct pci_dev *dev, int id)
{
	return (dev->bus->number << 8) + dev->devfn + dev->sriov->offset +
		dev->sriov->stride * id;
}

#define VIRT_BUS(dev, id)	(virtfn_bdf(dev, id) >> 8)
#define VIRT_DEVFN(dev, id)	(virtfn_bdf(dev, id) & 0xff)

We rely on GCC to do CSE and not actually invoke virtfn_bdf more than
once.
Yu Zhao March 9, 2009, 8:13 a.m. UTC | #2
On Sat, Mar 07, 2009 at 04:20:24AM +0800, Matthew Wilcox wrote:
> On Fri, Feb 20, 2009 at 02:54:44PM +0800, Yu Zhao wrote:
> > +static inline void virtfn_bdf(struct pci_dev *dev, int id, u8 *busnr, u8 *devfn)
> > +{
> > +	u16 bdf;
> > +
> > +	bdf = (dev->bus->number << 8) + dev->devfn +
> > +	      dev->sriov->offset + dev->sriov->stride * id;
> > +	*busnr = bdf >> 8;
> > +	*devfn = bdf & 0xff;
> > +}
> 
> I find the interface here a bit clunky -- a function returning void
> while having two OUT parameters.  How about this variation on the theme
> (viewers are encouraged to come up with their own preferred
> implementations and interfaces):
> 
> static inline __pure u16 virtfn_bdf(struct pci_dev *dev, int id)
> {
> 	return (dev->bus->number << 8) + dev->devfn + dev->sriov->offset +
> 		dev->sriov->stride * id;
> }
> 
> #define VIRT_BUS(dev, id)	(virtfn_bdf(dev, id) >> 8)
> #define VIRT_DEVFN(dev, id)	(virtfn_bdf(dev, id) & 0xff)
> 
> We rely on GCC to do CSE and not actually invoke virtfn_bdf more than
> once.

Yes, that's a good idea. Will replace that function with macros.
--
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
Randy Dunlap March 9, 2009, 6:09 p.m. UTC | #3
Yu Zhao wrote:
> On Sat, Mar 07, 2009 at 04:20:24AM +0800, Matthew Wilcox wrote:
>> On Fri, Feb 20, 2009 at 02:54:44PM +0800, Yu Zhao wrote:
>>> +static inline void virtfn_bdf(struct pci_dev *dev, int id, u8 *busnr, u8 *devfn)
>>> +{
>>> +	u16 bdf;
>>> +
>>> +	bdf = (dev->bus->number << 8) + dev->devfn +
>>> +	      dev->sriov->offset + dev->sriov->stride * id;
>>> +	*busnr = bdf >> 8;
>>> +	*devfn = bdf & 0xff;
>>> +}
>> I find the interface here a bit clunky -- a function returning void
>> while having two OUT parameters.  How about this variation on the theme
>> (viewers are encouraged to come up with their own preferred
>> implementations and interfaces):
>>
>> static inline __pure u16 virtfn_bdf(struct pci_dev *dev, int id)
>> {
>> 	return (dev->bus->number << 8) + dev->devfn + dev->sriov->offset +
>> 		dev->sriov->stride * id;
>> }
>>
>> #define VIRT_BUS(dev, id)	(virtfn_bdf(dev, id) >> 8)
>> #define VIRT_DEVFN(dev, id)	(virtfn_bdf(dev, id) & 0xff)
>>
>> We rely on GCC to do CSE and not actually invoke virtfn_bdf more than
>> once.
> 
> Yes, that's a good idea. Will replace that function with macros.

That's the opposite of most changes lately.  I.e., functions (with
typechecking) are preferred.
Matthew Wilcox March 9, 2009, 6:11 p.m. UTC | #4
On Mon, Mar 09, 2009 at 11:09:02AM -0700, Randy Dunlap wrote:
> >> static inline __pure u16 virtfn_bdf(struct pci_dev *dev, int id)
> >> {
> >> 	return (dev->bus->number << 8) + dev->devfn + dev->sriov->offset +
> >> 		dev->sriov->stride * id;
> >> }
> >>
> >> #define VIRT_BUS(dev, id)	(virtfn_bdf(dev, id) >> 8)
> >> #define VIRT_DEVFN(dev, id)	(virtfn_bdf(dev, id) & 0xff)
> >>
> >> We rely on GCC to do CSE and not actually invoke virtfn_bdf more than
> >> once.
> > 
> > Yes, that's a good idea. Will replace that function with macros.
> 
> That's the opposite of most changes lately.  I.e., functions (with
> typechecking) are preferred.

There's every bit as much typechecking with the approach I outlined as
there was with the original.
diff mbox

Patch

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 3bca8f8..0b80437 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -14,6 +14,16 @@ 
 #include "pci.h"
 
 
+static inline void virtfn_bdf(struct pci_dev *dev, int id, u8 *busnr, u8 *devfn)
+{
+	u16 bdf;
+
+	bdf = (dev->bus->number << 8) + dev->devfn +
+	      dev->sriov->offset + dev->sriov->stride * id;
+	*busnr = bdf >> 8;
+	*devfn = bdf & 0xff;
+}
+
 static int sriov_init(struct pci_dev *dev, int pos)
 {
 	int i;
@@ -208,3 +218,27 @@  void pci_restore_iov_state(struct pci_dev *dev)
 	if (dev->sriov)
 		sriov_restore_state(dev);
 }
+
+/**
+ * pci_iov_bus_range - find bus range used by Virtual Function
+ * @bus: the PCI bus
+ *
+ * Returns max number of buses (exclude current one) used by Virtual
+ * Functions.
+ */
+int pci_iov_bus_range(struct pci_bus *bus)
+{
+	int max = 0;
+	u8 busnr, devfn;
+	struct pci_dev *dev;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (!dev->sriov)
+			continue;
+		virtfn_bdf(dev, dev->sriov->total - 1, &busnr, &devfn);
+		if (busnr > max)
+			max = busnr;
+	}
+
+	return max ? max - bus->number : 0;
+}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b24c9e2..2cf32f5 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -217,6 +217,7 @@  extern void pci_iov_release(struct pci_dev *dev);
 extern int pci_iov_resource_bar(struct pci_dev *dev, int resno,
 				enum pci_bar_type *type);
 extern void pci_restore_iov_state(struct pci_dev *dev);
+extern int pci_iov_bus_range(struct pci_bus *bus);
 #else
 static inline int pci_iov_init(struct pci_dev *dev)
 {
@@ -234,6 +235,10 @@  static inline int pci_iov_resource_bar(struct pci_dev *dev, int resno,
 static inline void pci_restore_iov_state(struct pci_dev *dev)
 {
 }
+static inline int pci_iov_bus_range(struct pci_bus *bus)
+{
+	return 0;
+}
 #endif /* CONFIG_PCI_IOV */
 
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 03b6f29..4c8abd0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1078,6 +1078,9 @@  unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	for (devfn = 0; devfn < 0x100; devfn += 8)
 		pci_scan_slot(bus, devfn);
 
+	/* Reserve buses for SR-IOV capability. */
+	max += pci_iov_bus_range(bus);
+
 	/*
 	 * After performing arch-dependent fixup of the bus, look behind
 	 * all PCI-to-PCI bridges on this bus.