[v7] PCI: VF resizable BAR

[PATCH v7 2/6] PCI: Add a helper to convert between VF BAR number and IOV resource

Posted by Michał Winiarski 10 months, 1 week ago

There are multiple places where conversions between IOV resources and
corresponding VF BAR numbers are done.

Extract the logic to pci_resource_num_from_vf_bar() and
pci_resource_num_to_vf_bar() helpers.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/iov.c       | 26 ++++++++++++++++----------
 drivers/pci/pci.h       | 19 +++++++++++++++++++
 drivers/pci/setup-bus.c |  3 ++-
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 8bdc0829f847b..3d5da055c3dc1 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -151,7 +151,7 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
 	if (!dev->is_physfn)
 		return 0;
 
-	return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
+	return dev->sriov->barsz[pci_resource_num_to_vf_bar(resno)];
 }
 
 static void pci_read_vf_config_common(struct pci_dev *virtfn)
@@ -342,12 +342,14 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 	virtfn->multifunction = 0;
 
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
-		res = &dev->resource[i + PCI_IOV_RESOURCES];
+		int idx = pci_resource_num_from_vf_bar(i);
+
+		res = &dev->resource[idx];
 		if (!res->parent)
 			continue;
 		virtfn->resource[i].name = pci_name(virtfn);
 		virtfn->resource[i].flags = res->flags;
-		size = pci_iov_resource_size(dev, i + PCI_IOV_RESOURCES);
+		size = pci_iov_resource_size(dev, idx);
 		resource_set_range(&virtfn->resource[i],
 				   res->start + size * id, size);
 		rc = request_resource(res, &virtfn->resource[i]);
@@ -644,8 +646,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 
 	nres = 0;
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
-		bars |= (1 << (i + PCI_IOV_RESOURCES));
-		res = &dev->resource[i + PCI_IOV_RESOURCES];
+		int idx = pci_resource_num_from_vf_bar(i);
+
+		bars |= (1 << idx);
+		res = &dev->resource[idx];
 		if (res->parent)
 			nres++;
 	}
@@ -811,8 +815,10 @@ static int sriov_init(struct pci_dev *dev, int pos)
 
 	nres = 0;
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
-		res = &dev->resource[i + PCI_IOV_RESOURCES];
-		res_name = pci_resource_name(dev, i + PCI_IOV_RESOURCES);
+		int idx = pci_resource_num_from_vf_bar(i);
+
+		res = &dev->resource[idx];
+		res_name = pci_resource_name(dev, idx);
 
 		/*
 		 * If it is already FIXED, don't change it, something
@@ -871,7 +877,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	dev->is_physfn = 0;
 failed:
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
-		res = &dev->resource[i + PCI_IOV_RESOURCES];
+		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
 		res->flags = 0;
 	}
 
@@ -933,7 +939,7 @@ static void sriov_restore_state(struct pci_dev *dev)
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, ctrl);
 
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++)
-		pci_update_resource(dev, i + PCI_IOV_RESOURCES);
+		pci_update_resource(dev, pci_resource_num_from_vf_bar(i));
 
 	pci_write_config_dword(dev, iov->pos + PCI_SRIOV_SYS_PGSIZE, iov->pgsz);
 	pci_iov_set_numvfs(dev, iov->num_VFs);
@@ -999,7 +1005,7 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno)
 {
 	struct pci_sriov *iov = dev->is_physfn ? dev->sriov : NULL;
 	struct resource *res = pci_resource_n(dev, resno);
-	int vf_bar = resno - PCI_IOV_RESOURCES;
+	int vf_bar = pci_resource_num_to_vf_bar(resno);
 	struct pci_bus_region region;
 	u16 cmd;
 	u32 new;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index adc54bb2c8b34..f44840ee3c327 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -693,6 +693,15 @@ static inline bool pci_resource_is_iov(int resno)
 {
 	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
 }
+static inline int pci_resource_num_from_vf_bar(int resno)
+{
+	return resno + PCI_IOV_RESOURCES;
+}
+
+static inline int pci_resource_num_to_vf_bar(int resno)
+{
+	return resno - PCI_IOV_RESOURCES;
+}
 extern const struct attribute_group sriov_pf_dev_attr_group;
 extern const struct attribute_group sriov_vf_dev_attr_group;
 #else
@@ -717,6 +726,16 @@ static inline bool pci_resource_is_iov(int resno)
 {
 	return false;
 }
+static inline int pci_resource_num_from_vf_bar(int resno)
+{
+	WARN_ON_ONCE(1);
+	return -ENODEV;
+}
+static inline int pci_resource_num_to_vf_bar(int resno)
+{
+	WARN_ON_ONCE(1);
+	return -ENODEV;
+}
 #endif /* CONFIG_PCI_IOV */
 
 #ifdef CONFIG_PCIE_TPH
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 54d6f4fa3ce16..281121449fc0b 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1885,7 +1885,8 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
 	bool *unassigned = data;
 
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
-		struct resource *r = &dev->resource[i + PCI_IOV_RESOURCES];
+		int idx = pci_resource_num_from_vf_bar(i);
+		struct resource *r = &dev->resource[idx];
 		struct pci_bus_region region;
 
 		/* Not assigned or rejected by kernel? */
-- 
2.49.0

Re: [PATCH v7 2/6] PCI: Add a helper to convert between VF BAR number and IOV resource

Posted by Ilpo Järvinen 10 months, 1 week ago

On Wed, 2 Apr 2025, Michał Winiarski wrote:

> There are multiple places where conversions between IOV resources and
> corresponding VF BAR numbers are done.
> 
> Extract the logic to pci_resource_num_from_vf_bar() and
> pci_resource_num_to_vf_bar() helpers.
> 
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/pci/iov.c       | 26 ++++++++++++++++----------
>  drivers/pci/pci.h       | 19 +++++++++++++++++++
>  drivers/pci/setup-bus.c |  3 ++-
>  3 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 8bdc0829f847b..3d5da055c3dc1 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -151,7 +151,7 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
>  	if (!dev->is_physfn)
>  		return 0;
>  
> -	return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
> +	return dev->sriov->barsz[pci_resource_num_to_vf_bar(resno)];
>  }
>  
>  static void pci_read_vf_config_common(struct pci_dev *virtfn)
> @@ -342,12 +342,14 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
>  	virtfn->multifunction = 0;
>  
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> -		res = &dev->resource[i + PCI_IOV_RESOURCES];
> +		int idx = pci_resource_num_from_vf_bar(i);
> +
> +		res = &dev->resource[idx];
>  		if (!res->parent)
>  			continue;
>  		virtfn->resource[i].name = pci_name(virtfn);
>  		virtfn->resource[i].flags = res->flags;
> -		size = pci_iov_resource_size(dev, i + PCI_IOV_RESOURCES);
> +		size = pci_iov_resource_size(dev, idx);
>  		resource_set_range(&virtfn->resource[i],
>  				   res->start + size * id, size);
>  		rc = request_resource(res, &virtfn->resource[i]);
> @@ -644,8 +646,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>  
>  	nres = 0;
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> -		bars |= (1 << (i + PCI_IOV_RESOURCES));
> -		res = &dev->resource[i + PCI_IOV_RESOURCES];
> +		int idx = pci_resource_num_from_vf_bar(i);
> +
> +		bars |= (1 << idx);
> +		res = &dev->resource[idx];
>  		if (res->parent)
>  			nres++;
>  	}
> @@ -811,8 +815,10 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  
>  	nres = 0;
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> -		res = &dev->resource[i + PCI_IOV_RESOURCES];
> -		res_name = pci_resource_name(dev, i + PCI_IOV_RESOURCES);
> +		int idx = pci_resource_num_from_vf_bar(i);
> +
> +		res = &dev->resource[idx];
> +		res_name = pci_resource_name(dev, idx);
>  
>  		/*
>  		 * If it is already FIXED, don't change it, something
> @@ -871,7 +877,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  	dev->is_physfn = 0;
>  failed:
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> -		res = &dev->resource[i + PCI_IOV_RESOURCES];
> +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
>  		res->flags = 0;
>  	}
>  
> @@ -933,7 +939,7 @@ static void sriov_restore_state(struct pci_dev *dev)
>  	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, ctrl);
>  
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++)
> -		pci_update_resource(dev, i + PCI_IOV_RESOURCES);
> +		pci_update_resource(dev, pci_resource_num_from_vf_bar(i));
>  
>  	pci_write_config_dword(dev, iov->pos + PCI_SRIOV_SYS_PGSIZE, iov->pgsz);
>  	pci_iov_set_numvfs(dev, iov->num_VFs);
> @@ -999,7 +1005,7 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno)
>  {
>  	struct pci_sriov *iov = dev->is_physfn ? dev->sriov : NULL;
>  	struct resource *res = pci_resource_n(dev, resno);
> -	int vf_bar = resno - PCI_IOV_RESOURCES;
> +	int vf_bar = pci_resource_num_to_vf_bar(resno);
>  	struct pci_bus_region region;
>  	u16 cmd;
>  	u32 new;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index adc54bb2c8b34..f44840ee3c327 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -693,6 +693,15 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
>  }
> +static inline int pci_resource_num_from_vf_bar(int resno)
> +{
> +	return resno + PCI_IOV_RESOURCES;
> +}
> +
> +static inline int pci_resource_num_to_vf_bar(int resno)
> +{
> +	return resno - PCI_IOV_RESOURCES;
> +}
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -717,6 +726,16 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return false;
>  }
> +static inline int pci_resource_num_from_vf_bar(int resno)
> +{
> +	WARN_ON_ONCE(1);
> +	return -ENODEV;
> +}
> +static inline int pci_resource_num_to_vf_bar(int resno)
> +{
> +	WARN_ON_ONCE(1);
> +	return -ENODEV;
> +}
>  #endif /* CONFIG_PCI_IOV */
>  
>  #ifdef CONFIG_PCIE_TPH
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 54d6f4fa3ce16..281121449fc0b 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1885,7 +1885,8 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
>  	bool *unassigned = data;
>  
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> -		struct resource *r = &dev->resource[i + PCI_IOV_RESOURCES];
> +		int idx = pci_resource_num_from_vf_bar(i);
> +		struct resource *r = &dev->resource[idx];
>  		struct pci_bus_region region;
>  
>  		/* Not assigned or rejected by kernel? */
> 

Thanks, looks more readable now.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

[PATCH v7 1/6] PCI/IOV: Restore VF resizable BAR state after reset
[PATCH v7 2/6] PCI: Add a helper to convert between VF BAR number and IOV resource
[PATCH v7 3/6] PCI: Allow IOV resources to be resized in pci_resize_resource()
[PATCH v7 4/6] PCI/IOV: Check that VF BAR fits within the reservation
[PATCH v7 5/6] PCI: Allow drivers to control VF BAR size
[PATCH v7 6/6] drm/xe/pf: Set VF LMEM BAR size