Patchwork [RFC,V3,08/17] PCI: Add weak pcibios_sriov_resource_size() interface

login
register
mail settings
Submitter Wei Yang
Date June 10, 2014, 1:56 a.m.
Message ID <1402365399-5121-9-git-send-email-weiyang@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/357692/
State Superseded
Headers show

Comments

Wei Yang - June 10, 2014, 1:56 a.m.
When retrieving sriov resource size in pci_sriov_resource_size(), it will
divide the total IOV resource size with the totalVF number. This is true for
most cases, while may not be correct on some specific platform.

For example on powernv platform, in order to fix the IOV BAR into a hardware
alignment, the IOV resource size would be expended. This means the original
method couldn't work.

This patch introduces a weak pcibios_sriov_resource_size() interface, which
gives platform a chance to implement specific method to calculate the sriov
resource size.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/iov.c   |   27 +++++++++++++++++++++++++--
 include/linux/pci.h |    3 +++
 2 files changed, 28 insertions(+), 2 deletions(-)
Gavin Shan - June 23, 2014, 5:41 a.m.
On Tue, Jun 10, 2014 at 09:56:30AM +0800, Wei Yang wrote:
>When retrieving sriov resource size in pci_sriov_resource_size(), it will
>divide the total IOV resource size with the totalVF number. This is true for
>most cases, while may not be correct on some specific platform.
>
>For example on powernv platform, in order to fix the IOV BAR into a hardware
>alignment, the IOV resource size would be expended. This means the original
>method couldn't work.
>
>This patch introduces a weak pcibios_sriov_resource_size() interface, which
>gives platform a chance to implement specific method to calculate the sriov
>resource size.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> drivers/pci/iov.c   |   27 +++++++++++++++++++++++++--
> include/linux/pci.h |    3 +++
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>index cc87773..9fd4648 100644
>--- a/drivers/pci/iov.c
>+++ b/drivers/pci/iov.c
>@@ -45,6 +45,30 @@ static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
> 		pci_remove_bus(virtbus);
> }
>
>+resource_size_t __weak pcibios_sriov_resource_size(struct pci_dev *dev, int resno)
>+{
>+	return 0;
>+}
>+

Please define the prototype of weak function in header files (e.g.
linux/include/pci.h) :-)

If you missed doing same thing for the weak functions added in the
previous patches, you need fix it as well.

>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>+{
>+	u64 size;

I guess it'd better to be "resource_size_t". 

>+	struct pci_sriov *iov;
>+
>+	if (!dev->is_physfn)
>+		return 0;
>+
>+	size = pcibios_sriov_resource_size(dev, resno);
>+	if (size != 0)
>+		return size;
>+
>+	iov = dev->sriov;
>+	size = resource_size(dev->resource + resno);
>+	do_div(size, iov->total_VFs);
>+
>+	return size;
>+}
>+
> static int virtfn_add(struct pci_dev *dev, int id, int reset)
> {
> 	int i;
>@@ -81,8 +105,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
> 			continue;
> 		virtfn->resource[i].name = pci_name(virtfn);
> 		virtfn->resource[i].flags = res->flags;
>-		size = resource_size(res);
>-		do_div(size, iov->total_VFs);
>+		size = pci_sriov_resource_size(dev, i + PCI_IOV_RESOURCES);
> 		virtfn->resource[i].start = res->start + size * id;
> 		virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
> 		rc = request_resource(res, &virtfn->resource[i]);
>diff --git a/include/linux/pci.h b/include/linux/pci.h
>index ddb1ca0..315c150 100644
>--- a/include/linux/pci.h
>+++ b/include/linux/pci.h
>@@ -1637,6 +1637,7 @@ int pci_num_vf(struct pci_dev *dev);
> int pci_vfs_assigned(struct pci_dev *dev);
> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
> int pci_sriov_get_totalvfs(struct pci_dev *dev);
>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno);
> #else
> static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
> {
>@@ -1658,6 +1659,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
> { return 0; }
> static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
> { return 0; }
>+static inline resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>+{ return -1; }
> #endif
>
> #if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)

Thanks,
Gavin
Wei Yang - June 23, 2014, 7:56 a.m.
On Mon, Jun 23, 2014 at 03:41:28PM +1000, Gavin Shan wrote:
>On Tue, Jun 10, 2014 at 09:56:30AM +0800, Wei Yang wrote:
>>When retrieving sriov resource size in pci_sriov_resource_size(), it will
>>divide the total IOV resource size with the totalVF number. This is true for
>>most cases, while may not be correct on some specific platform.
>>
>>For example on powernv platform, in order to fix the IOV BAR into a hardware
>>alignment, the IOV resource size would be expended. This means the original
>>method couldn't work.
>>
>>This patch introduces a weak pcibios_sriov_resource_size() interface, which
>>gives platform a chance to implement specific method to calculate the sriov
>>resource size.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> drivers/pci/iov.c   |   27 +++++++++++++++++++++++++--
>> include/linux/pci.h |    3 +++
>> 2 files changed, 28 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>>index cc87773..9fd4648 100644
>>--- a/drivers/pci/iov.c
>>+++ b/drivers/pci/iov.c
>>@@ -45,6 +45,30 @@ static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
>> 		pci_remove_bus(virtbus);
>> }
>>
>>+resource_size_t __weak pcibios_sriov_resource_size(struct pci_dev *dev, int resno)
>>+{
>>+	return 0;
>>+}
>>+
>
>Please define the prototype of weak function in header files (e.g.
>linux/include/pci.h) :-)

Missed, will add it.

>
>If you missed doing same thing for the weak functions added in the
>previous patches, you need fix it as well.

Yep.

>
>>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>>+{
>>+	u64 size;
>
>I guess it'd better to be "resource_size_t". 
>
>>+	struct pci_sriov *iov;
>>+
>>+	if (!dev->is_physfn)
>>+		return 0;
>>+
>>+	size = pcibios_sriov_resource_size(dev, resno);
>>+	if (size != 0)
>>+		return size;
>>+
>>+	iov = dev->sriov;
>>+	size = resource_size(dev->resource + resno);
>>+	do_div(size, iov->total_VFs);
>>+
>>+	return size;
>>+}
>>+
>> static int virtfn_add(struct pci_dev *dev, int id, int reset)
>> {
>> 	int i;
>>@@ -81,8 +105,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>> 			continue;
>> 		virtfn->resource[i].name = pci_name(virtfn);
>> 		virtfn->resource[i].flags = res->flags;
>>-		size = resource_size(res);
>>-		do_div(size, iov->total_VFs);
>>+		size = pci_sriov_resource_size(dev, i + PCI_IOV_RESOURCES);
>> 		virtfn->resource[i].start = res->start + size * id;
>> 		virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
>> 		rc = request_resource(res, &virtfn->resource[i]);
>>diff --git a/include/linux/pci.h b/include/linux/pci.h
>>index ddb1ca0..315c150 100644
>>--- a/include/linux/pci.h
>>+++ b/include/linux/pci.h
>>@@ -1637,6 +1637,7 @@ int pci_num_vf(struct pci_dev *dev);
>> int pci_vfs_assigned(struct pci_dev *dev);
>> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
>> int pci_sriov_get_totalvfs(struct pci_dev *dev);
>>+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno);
>> #else
>> static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
>> {
>>@@ -1658,6 +1659,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
>> { return 0; }
>> static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
>> { return 0; }
>>+static inline resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
>>+{ return -1; }
>> #endif
>>
>> #if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)
>
>Thanks,
>Gavin

Patch

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index cc87773..9fd4648 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -45,6 +45,30 @@  static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
 		pci_remove_bus(virtbus);
 }
 
+resource_size_t __weak pcibios_sriov_resource_size(struct pci_dev *dev, int resno)
+{
+	return 0;
+}
+
+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
+{
+	u64 size;
+	struct pci_sriov *iov;
+
+	if (!dev->is_physfn)
+		return 0;
+
+	size = pcibios_sriov_resource_size(dev, resno);
+	if (size != 0)
+		return size;
+
+	iov = dev->sriov;
+	size = resource_size(dev->resource + resno);
+	do_div(size, iov->total_VFs);
+
+	return size;
+}
+
 static int virtfn_add(struct pci_dev *dev, int id, int reset)
 {
 	int i;
@@ -81,8 +105,7 @@  static int virtfn_add(struct pci_dev *dev, int id, int reset)
 			continue;
 		virtfn->resource[i].name = pci_name(virtfn);
 		virtfn->resource[i].flags = res->flags;
-		size = resource_size(res);
-		do_div(size, iov->total_VFs);
+		size = pci_sriov_resource_size(dev, i + PCI_IOV_RESOURCES);
 		virtfn->resource[i].start = res->start + size * id;
 		virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
 		rc = request_resource(res, &virtfn->resource[i]);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ddb1ca0..315c150 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1637,6 +1637,7 @@  int pci_num_vf(struct pci_dev *dev);
 int pci_vfs_assigned(struct pci_dev *dev);
 int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
 int pci_sriov_get_totalvfs(struct pci_dev *dev);
+resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno);
 #else
 static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
 {
@@ -1658,6 +1659,8 @@  static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
 { return 0; }
 static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
 { return 0; }
+static inline resource_size_t pci_sriov_resource_size(struct pci_dev *dev, int resno)
+{ return -1; }
 #endif
 
 #if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)