[PATCH v3 2/5] PCI: Add a helper to identify IOV resources

Michał Winiarski posted 5 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/5] PCI: Add a helper to identify IOV resources
Posted by Michał Winiarski 1 month, 2 weeks ago
There are multiple places where special handling is required for IOV
resources.
Extract it to a helper and drop a few ifdefs.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/pci/pci.h       | 18 ++++++++++++++----
 drivers/pci/setup-bus.c |  5 +----
 drivers/pci/setup-res.c |  4 +---
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 14d00ce45bfa9..c55f2d7a4f37e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -580,6 +580,10 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno);
 resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
 void pci_restore_iov_state(struct pci_dev *dev);
 int pci_iov_bus_range(struct pci_bus *bus);
+static inline bool pci_resource_is_iov(int resno)
+{
+	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
+}
 extern const struct attribute_group sriov_pf_dev_attr_group;
 extern const struct attribute_group sriov_vf_dev_attr_group;
 #else
@@ -589,12 +593,20 @@ static inline int pci_iov_init(struct pci_dev *dev)
 }
 static inline void pci_iov_release(struct pci_dev *dev) { }
 static inline void pci_iov_remove(struct pci_dev *dev) { }
+static inline void pci_iov_update_resource(struct pci_dev *dev, int resno) { }
+static inline resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
+{
+	return 0;
+}
 static inline void pci_restore_iov_state(struct pci_dev *dev) { }
 static inline int pci_iov_bus_range(struct pci_bus *bus)
 {
 	return 0;
 }
-
+static inline bool pci_resource_is_iov(int resno)
+{
+	return false;
+}
 #endif /* CONFIG_PCI_IOV */
 
 #ifdef CONFIG_PCIE_PTM
@@ -616,12 +628,10 @@ unsigned long pci_cardbus_resource_alignment(struct resource *);
 static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 						     struct resource *res)
 {
-#ifdef CONFIG_PCI_IOV
 	int resno = res - dev->resource;
 
-	if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
+	if (pci_resource_is_iov(resno))
 		return pci_sriov_resource_alignment(dev, resno);
-#endif
 	if (dev->class >> 8 == PCI_CLASS_BRIDGE_CARDBUS)
 		return pci_cardbus_resource_alignment(res);
 	return resource_alignment(res);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 23082bc0ca37a..8909948bc9a9f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1093,17 +1093,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			     (r->flags & mask) != type3))
 				continue;
 			r_size = resource_size(r);
-#ifdef CONFIG_PCI_IOV
 			/* Put SRIOV requested res to the optional list */
-			if (realloc_head && i >= PCI_IOV_RESOURCES &&
-					i <= PCI_IOV_RESOURCE_END) {
+			if (realloc_head && pci_resource_is_iov(i)) {
 				add_align = max(pci_resource_alignment(dev, r), add_align);
 				r->end = r->start - 1;
 				add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
 				children_add_size += r_size;
 				continue;
 			}
-#endif
 			/*
 			 * aligns[0] is for 1MB (since bridge memory
 			 * windows are always at least 1MB aligned), so
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index c6d933ddfd464..e2cf79253ebda 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -127,10 +127,8 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 {
 	if (resno <= PCI_ROM_RESOURCE)
 		pci_std_update_resource(dev, resno);
-#ifdef CONFIG_PCI_IOV
-	else if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
+	else if (pci_resource_is_iov(resno))
 		pci_iov_update_resource(dev, resno);
-#endif
 }
 
 int pci_claim_resource(struct pci_dev *dev, int resource)
-- 
2.47.0

Re: [PATCH v3 2/5] PCI: Add a helper to identify IOV resources
Posted by Bjorn Helgaas 1 month, 2 weeks ago
On Thu, Oct 10, 2024 at 12:32:00PM +0200, Michał Winiarski wrote:
> There are multiple places where special handling is required for IOV
> resources.
> Extract it to a helper and drop a few ifdefs.

Add blank line between paragraphs.

Include name of helper in subject line and commit log, i.e.,
pci_resource_is_iov().  No point in guessing or making us extract it
from the patch.  This will also make the commit log say more clearly
what the patch does.

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/pci.h       | 18 ++++++++++++++----
>  drivers/pci/setup-bus.c |  5 +----
>  drivers/pci/setup-res.c |  4 +---
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 14d00ce45bfa9..c55f2d7a4f37e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -580,6 +580,10 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno);
>  resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
>  void pci_restore_iov_state(struct pci_dev *dev);
>  int pci_iov_bus_range(struct pci_bus *bus);
> +static inline bool pci_resource_is_iov(int resno)
> +{
> +	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> +}
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -589,12 +593,20 @@ static inline int pci_iov_init(struct pci_dev *dev)
>  }
>  static inline void pci_iov_release(struct pci_dev *dev) { }
>  static inline void pci_iov_remove(struct pci_dev *dev) { }
> +static inline void pci_iov_update_resource(struct pci_dev *dev, int resno) { }
> +static inline resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
> +{
> +	return 0;
> +}
>  static inline void pci_restore_iov_state(struct pci_dev *dev) { }
>  static inline int pci_iov_bus_range(struct pci_bus *bus)
>  {
>  	return 0;
>  }
> -
> +static inline bool pci_resource_is_iov(int resno)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_PCI_IOV */
>  
>  #ifdef CONFIG_PCIE_PTM
> @@ -616,12 +628,10 @@ unsigned long pci_cardbus_resource_alignment(struct resource *);
>  static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
>  						     struct resource *res)
>  {
> -#ifdef CONFIG_PCI_IOV
>  	int resno = res - dev->resource;
>  
> -	if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
> +	if (pci_resource_is_iov(resno))
>  		return pci_sriov_resource_alignment(dev, resno);
> -#endif
>  	if (dev->class >> 8 == PCI_CLASS_BRIDGE_CARDBUS)
>  		return pci_cardbus_resource_alignment(res);
>  	return resource_alignment(res);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 23082bc0ca37a..8909948bc9a9f 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1093,17 +1093,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  			     (r->flags & mask) != type3))
>  				continue;
>  			r_size = resource_size(r);
> -#ifdef CONFIG_PCI_IOV
>  			/* Put SRIOV requested res to the optional list */
> -			if (realloc_head && i >= PCI_IOV_RESOURCES &&
> -					i <= PCI_IOV_RESOURCE_END) {
> +			if (realloc_head && pci_resource_is_iov(i)) {
>  				add_align = max(pci_resource_alignment(dev, r), add_align);
>  				r->end = r->start - 1;
>  				add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
>  				children_add_size += r_size;
>  				continue;
>  			}
> -#endif
>  			/*
>  			 * aligns[0] is for 1MB (since bridge memory
>  			 * windows are always at least 1MB aligned), so
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index c6d933ddfd464..e2cf79253ebda 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -127,10 +127,8 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>  {
>  	if (resno <= PCI_ROM_RESOURCE)
>  		pci_std_update_resource(dev, resno);
> -#ifdef CONFIG_PCI_IOV
> -	else if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
> +	else if (pci_resource_is_iov(resno))
>  		pci_iov_update_resource(dev, resno);
> -#endif
>  }
>  
>  int pci_claim_resource(struct pci_dev *dev, int resource)
> -- 
> 2.47.0
> 
Re: [PATCH v3 2/5] PCI: Add a helper to identify IOV resources
Posted by Ilpo Järvinen 1 month, 2 weeks ago
On Thu, 10 Oct 2024, Michał Winiarski wrote:

> There are multiple places where special handling is required for IOV
> resources.
> Extract it to a helper and drop a few ifdefs.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/pci.h       | 18 ++++++++++++++----
>  drivers/pci/setup-bus.c |  5 +----
>  drivers/pci/setup-res.c |  4 +---
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 14d00ce45bfa9..c55f2d7a4f37e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -580,6 +580,10 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno);
>  resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
>  void pci_restore_iov_state(struct pci_dev *dev);
>  int pci_iov_bus_range(struct pci_bus *bus);
> +static inline bool pci_resource_is_iov(int resno)
> +{
> +	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> +}
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -589,12 +593,20 @@ static inline int pci_iov_init(struct pci_dev *dev)
>  }
>  static inline void pci_iov_release(struct pci_dev *dev) { }
>  static inline void pci_iov_remove(struct pci_dev *dev) { }
> +static inline void pci_iov_update_resource(struct pci_dev *dev, int resno) { }
> +static inline resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
> +{
> +	return 0;
> +}
>  static inline void pci_restore_iov_state(struct pci_dev *dev) { }
>  static inline int pci_iov_bus_range(struct pci_bus *bus)
>  {
>  	return 0;
>  }
> -
> +static inline bool pci_resource_is_iov(int resno)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_PCI_IOV */
>  
>  #ifdef CONFIG_PCIE_PTM
> @@ -616,12 +628,10 @@ unsigned long pci_cardbus_resource_alignment(struct resource *);
>  static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
>  						     struct resource *res)
>  {
> -#ifdef CONFIG_PCI_IOV
>  	int resno = res - dev->resource;
>  
> -	if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
> +	if (pci_resource_is_iov(resno))
>  		return pci_sriov_resource_alignment(dev, resno);
> -#endif
>  	if (dev->class >> 8 == PCI_CLASS_BRIDGE_CARDBUS)
>  		return pci_cardbus_resource_alignment(res);
>  	return resource_alignment(res);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 23082bc0ca37a..8909948bc9a9f 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1093,17 +1093,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  			     (r->flags & mask) != type3))
>  				continue;
>  			r_size = resource_size(r);
> -#ifdef CONFIG_PCI_IOV
>  			/* Put SRIOV requested res to the optional list */
> -			if (realloc_head && i >= PCI_IOV_RESOURCES &&
> -					i <= PCI_IOV_RESOURCE_END) {
> +			if (realloc_head && pci_resource_is_iov(i)) {
>  				add_align = max(pci_resource_alignment(dev, r), add_align);
>  				r->end = r->start - 1;
>  				add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
>  				children_add_size += r_size;
>  				continue;
>  			}
> -#endif

Heh, I realized I've made pretty much the same patch a few days back...

Please leave empty lines around this SRIOV / optional resource.

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

-- 
 i.


>  			/*
>  			 * aligns[0] is for 1MB (since bridge memory
>  			 * windows are always at least 1MB aligned), so
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index c6d933ddfd464..e2cf79253ebda 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -127,10 +127,8 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>  {
>  	if (resno <= PCI_ROM_RESOURCE)
>  		pci_std_update_resource(dev, resno);
> -#ifdef CONFIG_PCI_IOV
> -	else if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
> +	else if (pci_resource_is_iov(resno))
>  		pci_iov_update_resource(dev, resno);
> -#endif
>  }
>  
>  int pci_claim_resource(struct pci_dev *dev, int resource)
>