[PATCH 0/2] PCI/IOV: fix on offset/stride based on NumVFs change

[PATCH 0/2] PCI/IOV: fix on offset/stride based on NumVFs change

[Wei Yang]

Per SRIOV SPEC sec 3.3.9, 3.3.10, offset/stride may change when NumVFs is
written. While current kernel doesn't update offset/stride with the change of
NumVFs.

This property will also affect the iov bus range in enumeration.

This patch set consists of two patches:
1. Update the offset/stride with NumVFs change
2. Calculate VF bus range on each possilbe NumVFs

Code based on 3.19-rc1 and tested on x86 and powernv platform.
Bus range doesn't chage on current hardware.

Wei Yang (2):
  PCI: Refresh offset/stride after NumVFs is written
  PCI: Calculate the VF bus range on each possible NumVFs

 drivers/pci/iov.c |   54 +++++++++++++++++++++++++++++++++++++++++++++--------
 drivers/pci/pci.h |    1 +
 2 files changed, 47 insertions(+), 8 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] PCI: Refresh offset/stride after NumVFs is written

[Wei Yang]

According to SR-IOV SPEC sec 3.3.9, 3.3.10, the NumVFs setting change will
affect the offset and stride. Current implementation doesn't refresh the
offset/stride cached in pci_sriov structure.

This patch introduces a wrapper pci_iov_set_numvfs(), which refresh these two
value after NumVFs is written.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/iov.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 4b3a4ea..ef0ceaa 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -31,6 +31,21 @@ static inline u8 virtfn_devfn(struct pci_dev *dev, int id)
 		dev->sriov->stride * id) & 0xff;
 }
 
+/**
+ * Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF offset may
+ * change when NumVFs changes.
+ *
+ * This function update the iov->offset and iov->stride when NumVFs is written.
+ */
+static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
+{
+	struct pci_sriov *iov = dev->sriov;
+
+	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
+	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_OFFSET, &iov->offset);
+	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
+}
+
 static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
 {
 	struct pci_bus *child;
@@ -243,7 +258,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 			return rc;
 	}
 
-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
+	pci_iov_set_numvfs(dev, nr_virtfn);
 	iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
@@ -272,7 +287,7 @@ failed:
 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, 0);
+	pci_iov_set_numvfs(dev, 0);
 	ssleep(1);
 	pci_cfg_access_unlock(dev);
 
@@ -303,7 +318,7 @@ static void sriov_disable(struct pci_dev *dev)
 		sysfs_remove_link(&dev->dev.kobj, "dep_link");
 
 	iov->num_VFs = 0;
-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, 0);
+	pci_iov_set_numvfs(dev, 0);
 }
 
 static int sriov_init(struct pci_dev *dev, int pos)
@@ -439,7 +454,7 @@ static void sriov_restore_state(struct pci_dev *dev)
 		pci_update_resource(dev, i);
 
 	pci_write_config_dword(dev, iov->pos + PCI_SRIOV_SYS_PGSIZE, iov->pgsz);
-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, iov->num_VFs);
+	pci_iov_set_numvfs(dev, iov->num_VFs);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
 	if (iov->ctrl & PCI_SRIOV_CTRL_VFE)
 		msleep(100);
-- 
1.7.9.5

[PATCH 2/2] PCI: Calculate the VF bus range on each possible NumVFs

[Wei Yang]

Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF Offset may change
when NumVFs changes. This will affect the bus range for VFs.

This patch iterates on each possible NumVFs and calculate the maximum bus
range for VFs.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/iov.c |   31 +++++++++++++++++++++++++++----
 drivers/pci/pci.h |    1 +
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index ef0ceaa..ea3a82c 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -46,6 +46,30 @@ static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
 }
 
+/**
+ * Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF offset may
+ * change when NumVFs changes. Which will affect the bus range for VFs.
+ *
+ * This function iterate on all valide NumVFs and calculate the maximum bus
+ * range for VFs.
+ */
+static inline void pci_iov_max_bus_range(struct pci_dev *dev)
+{
+	struct pci_sriov *iov = dev->sriov;
+	int total = iov->total_VFs;
+	u8 max = 0;
+	u8 busnr;
+
+	for ( ; total >= 0; total--) {
+		pci_iov_set_numvfs(dev, total);
+		busnr = virtfn_bus(dev, iov->total_VFs - 1);
+		if (busnr > max)
+			max = busnr;
+	}
+
+	iov->max_bus_range = max;
+}
+
 static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
 {
 	struct pci_bus *child;
@@ -415,6 +439,7 @@ found:
 
 	dev->sriov = iov;
 	dev->is_physfn = 1;
+	pci_iov_max_bus_range(dev);
 
 	return 0;
 
@@ -550,15 +575,13 @@ void pci_restore_iov_state(struct pci_dev *dev)
 int pci_iov_bus_range(struct pci_bus *bus)
 {
 	int max = 0;
-	u8 busnr;
 	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (!dev->is_physfn)
 			continue;
-		busnr = virtfn_bus(dev, dev->sriov->total_VFs - 1);
-		if (busnr > max)
-			max = busnr;
+		if (dev->sriov->max_bus_range > max)
+			max = dev->sriov->max_bus_range;
 	}
 
 	return max ? max - bus->number : 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 8aff29a..94faf97 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -236,6 +236,7 @@ struct pci_sriov {
 	u16 stride;		/* following VF stride */
 	u32 pgsz;		/* page size for BAR alignment */
 	u8 link;		/* Function Dependency Link */
+	u8 max_bus_range;	/* Maximum bus range a VF could sit*/
 	u16 driver_max_VFs;	/* max num VFs driver supports */
 	struct pci_dev *dev;	/* lowest numbered PF */
 	struct pci_dev *self;	/* this PF */
-- 
1.7.9.5

Re: [PATCH 2/2] PCI: Calculate the VF bus range on each possible NumVFs

[Bjorn Helgaas]

On Mon, Dec 22, 2014 at 01:48:46PM +0800, Wei Yang wrote:
> Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF Offset may change
> when NumVFs changes. This will affect the bus range for VFs.
> 
> This patch iterates on each possible NumVFs and calculate the maximum bus
> range for VFs.
> 
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/pci/iov.c |   31 +++++++++++++++++++++++++++----
>  drivers/pci/pci.h |    1 +
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index ef0ceaa..ea3a82c 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -46,6 +46,30 @@ static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
>  	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
>  }
>  
> +/**
> + * Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF offset may
> + * change when NumVFs changes. Which will affect the bus range for VFs.
> + *
> + * This function iterate on all valide NumVFs and calculate the maximum bus
> + * range for VFs.
> + */
> +static inline void pci_iov_max_bus_range(struct pci_dev *dev)
> +{
> +	struct pci_sriov *iov = dev->sriov;
> +	int total = iov->total_VFs;
> +	u8 max = 0;
> +	u8 busnr;
> +
> +	for ( ; total >= 0; total--) {
> +		pci_iov_set_numvfs(dev, total);
> +		busnr = virtfn_bus(dev, iov->total_VFs - 1);

We don't need TotalVFs here, do we?  I think NumVFs should be enough.
Proposed updated patch attached below.

> +		if (busnr > max)
> +			max = busnr;
> +	}
> +
> +	iov->max_bus_range = max;
> +}


commit aecf80117ca5be299f85ead9125434f58f2ae3cb
Author: Wei Yang <weiyang@linux.vnet.ibm.com>
Date:   Mon Dec 22 13:48:46 2014 +0800

    PCI: Calculate maximum number of buses required for VFs
    
    An SR-IOV device can change its First VF OFfset and VF Stride based on the
    values of ARI Capable Hierarchy and NumVFs.  The number of buses required
    for all VFs is determined by NumVFs, First VF Offset, and VF Stride (see
    SR-IOV spec r1.1, sec 2.1.2).
    
    Previously pci_iov_bus_range() computed how many buses would be required by
    TotalVFs, but this was based on a single NumVFs value and may not have been
    the maximum for all NumVFs configurations.
    
    Iterate over all valid NumVFs and calculate the maximum number of bus
    numbers that could ever be required for VFs of this device.
    
    [bhelgaas: compute busnr of NumVFs, not TotalVFs, changelog]
    Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index dd4bc361f24a..62f9c8f6871f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -46,6 +46,30 @@ static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
 }
 
+/**
+ * The PF consumes one bus number.  NumVFs, First VF Offset, and VF Stride
+ * determine how many additional bus numbers will be consumed by VFs.
+ *
+ * Iterate over all valid NumVFs and calculate the maximum number of bus
+ * numbers that could ever be required.
+ */
+static inline u8 virtfn_max_buses(struct pci_dev *dev)
+{
+	struct pci_sriov *iov = dev->sriov;
+	int nr_virtfn;
+	u8 max = 0;
+	u8 busnr;
+
+	for (nr_virtfn = 1; nr_virtfn <= iov->total_VFs; nr_virtfn++) {
+		pci_iov_set_numvfs(dev, nr_virtfn);
+		busnr = virtfn_bus(dev, nr_virtfn - 1);
+		if (busnr > max)
+			max = busnr;
+	}
+
+	return max;
+}
+
 static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
 {
 	struct pci_bus *child;
@@ -413,6 +437,7 @@ found:
 
 	mutex_init(&iov->lock);
 
+	iov->max_VF_buses = virtfn_max_buses(dev);
 	dev->sriov = iov;
 	dev->is_physfn = 1;
 
@@ -550,15 +575,13 @@ void pci_restore_iov_state(struct pci_dev *dev)
 int pci_iov_bus_range(struct pci_bus *bus)
 {
 	int max = 0;
-	u8 busnr;
 	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (!dev->is_physfn)
 			continue;
-		busnr = virtfn_bus(dev, dev->sriov->total_VFs - 1);
-		if (busnr > max)
-			max = busnr;
+		if (dev->sriov->max_VF_buses > max)
+			max = dev->sriov->max_VF_buses;
 	}
 
 	return max ? max - bus->number : 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 8aff29a804ff..f0a19dc7acdd 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -236,6 +236,7 @@ struct pci_sriov {
 	u16 stride;		/* following VF stride */
 	u32 pgsz;		/* page size for BAR alignment */
 	u8 link;		/* Function Dependency Link */
+	u8 max_VF_buses;	/* max buses consumed by VFs */
 	u16 driver_max_VFs;	/* max num VFs driver supports */
 	struct pci_dev *dev;	/* lowest numbered PF */
 	struct pci_dev *self;	/* this PF */

Re: [PATCH 2/2] PCI: Calculate the VF bus range on each possible NumVFs

[Wei Yang]

On Wed, Jan 21, 2015 at 04:54:05PM -0600, Bjorn Helgaas wrote:
>On Mon, Dec 22, 2014 at 01:48:46PM +0800, Wei Yang wrote:
>> Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF Offset may change
>> when NumVFs changes. This will affect the bus range for VFs.
>> 
>> This patch iterates on each possible NumVFs and calculate the maximum bus
>> range for VFs.
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/iov.c |   31 +++++++++++++++++++++++++++----
>>  drivers/pci/pci.h |    1 +
>>  2 files changed, 28 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index ef0ceaa..ea3a82c 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -46,6 +46,30 @@ static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
>>  	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
>>  }
>>  
>> +/**
>> + * Per SRIOV SPEC section 3.3.10 and 3.3.11, VF Stride and VF offset may
>> + * change when NumVFs changes. Which will affect the bus range for VFs.
>> + *
>> + * This function iterate on all valide NumVFs and calculate the maximum bus
>> + * range for VFs.
>> + */
>> +static inline void pci_iov_max_bus_range(struct pci_dev *dev)
>> +{
>> +	struct pci_sriov *iov = dev->sriov;
>> +	int total = iov->total_VFs;
>> +	u8 max = 0;
>> +	u8 busnr;
>> +
>> +	for ( ; total >= 0; total--) {
>> +		pci_iov_set_numvfs(dev, total);
>> +		busnr = virtfn_bus(dev, iov->total_VFs - 1);
>
>We don't need TotalVFs here, do we?  I think NumVFs should be enough.
>Proposed updated patch attached below.
>

Agree use NumVFs is reasonable. One small comment in below patch.

>> +		if (busnr > max)
>> +			max = busnr;
>> +	}
>> +
>> +	iov->max_bus_range = max;
>> +}
>
>
>commit aecf80117ca5be299f85ead9125434f58f2ae3cb
>Author: Wei Yang <weiyang@linux.vnet.ibm.com>
>Date:   Mon Dec 22 13:48:46 2014 +0800
>
>    PCI: Calculate maximum number of buses required for VFs
>    
>    An SR-IOV device can change its First VF OFfset and VF Stride based on the
>    values of ARI Capable Hierarchy and NumVFs.  The number of buses required
>    for all VFs is determined by NumVFs, First VF Offset, and VF Stride (see
>    SR-IOV spec r1.1, sec 2.1.2).
>    
>    Previously pci_iov_bus_range() computed how many buses would be required by
>    TotalVFs, but this was based on a single NumVFs value and may not have been
>    the maximum for all NumVFs configurations.
>    
>    Iterate over all valid NumVFs and calculate the maximum number of bus
>    numbers that could ever be required for VFs of this device.
>    
>    [bhelgaas: compute busnr of NumVFs, not TotalVFs, changelog]
>    Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>index dd4bc361f24a..62f9c8f6871f 100644
>--- a/drivers/pci/iov.c
>+++ b/drivers/pci/iov.c
>@@ -46,6 +46,30 @@ static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
> }
>
>+/**
>+ * The PF consumes one bus number.  NumVFs, First VF Offset, and VF Stride
>+ * determine how many additional bus numbers will be consumed by VFs.
>+ *
>+ * Iterate over all valid NumVFs and calculate the maximum number of bus
>+ * numbers that could ever be required.
>+ */
>+static inline u8 virtfn_max_buses(struct pci_dev *dev)
>+{
>+	struct pci_sriov *iov = dev->sriov;
>+	int nr_virtfn;
>+	u8 max = 0;
>+	u8 busnr;
>+
>+	for (nr_virtfn = 1; nr_virtfn <= iov->total_VFs; nr_virtfn++) {
>+		pci_iov_set_numvfs(dev, nr_virtfn);
>+		busnr = virtfn_bus(dev, nr_virtfn - 1);
>+		if (busnr > max)
>+			max = busnr;
>+	}
>+
>+	return max;
>+}
>+
> static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
> {
> 	struct pci_bus *child;
>@@ -413,6 +437,7 @@ found:
>
> 	mutex_init(&iov->lock);
>
>+	iov->max_VF_buses = virtfn_max_buses(dev);
> 	dev->sriov = iov;
> 	dev->is_physfn = 1;

We need to move the function after dev->sriov is set, otherwise in
virtfn_max_buses() the dev->sriov is not valid.

>
>@@ -550,15 +575,13 @@ void pci_restore_iov_state(struct pci_dev *dev)
> int pci_iov_bus_range(struct pci_bus *bus)
> {
> 	int max = 0;
>-	u8 busnr;
> 	struct pci_dev *dev;
>
> 	list_for_each_entry(dev, &bus->devices, bus_list) {
> 		if (!dev->is_physfn)
> 			continue;
>-		busnr = virtfn_bus(dev, dev->sriov->total_VFs - 1);
>-		if (busnr > max)
>-			max = busnr;
>+		if (dev->sriov->max_VF_buses > max)
>+			max = dev->sriov->max_VF_buses;
> 	}
>
> 	return max ? max - bus->number : 0;
>diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>index 8aff29a804ff..f0a19dc7acdd 100644
>--- a/drivers/pci/pci.h
>+++ b/drivers/pci/pci.h
>@@ -236,6 +236,7 @@ struct pci_sriov {
> 	u16 stride;		/* following VF stride */
> 	u32 pgsz;		/* page size for BAR alignment */
> 	u8 link;		/* Function Dependency Link */
>+	u8 max_VF_buses;	/* max buses consumed by VFs */
> 	u16 driver_max_VFs;	/* max num VFs driver supports */
> 	struct pci_dev *dev;	/* lowest numbered PF */
> 	struct pci_dev *self;	/* this PF */

-- 
Richard Yang
Help you, Help me