[PATCH v3 0/5] PCI: VF resizable BAR

[PATCH v3 0/5] PCI: VF resizable BAR

[Michał Winiarski]

Hi,

Resurrecting an old series [1], now that we have an in-tree user for it.

For regular BAR, drivers can use pci_resize_resource to resize it to the
desired size provided that it is supported by the hardware, which the
driver can query using pci_rebar_get_possible_sizes.
This series expands the API to work with IOV BAR as well.
It also adds the additional API to allow extending the VF BAR within the
original resource boundary.

Thanks,
-Michał

[1] https://lore.kernel.org/all/20211215141626.3090807-1-michal.winiarski@intel.com/

v1 -> v2:
- Add pci_iov_resource_extend() and usage in Xe driver
- Reduce the number of ifdefs (Christian)
- Drop patch 2/2 from v1 (Christian)
- Add a helper to avoid upsetting static analysis tools (Krzysztof)

v2 -> v3:
- Extract introducing pci_resource_is_iov to separate commit and
  use it elsewhere in PCI subsystem (Christian)
- Extract restoring VF rebar state to separate commit (Christian)
- Reorganize memory decoding check (Christian)
- Don't use dev_WARN (Ilpo)
- Fix build without CONFIG_PCI_IOV (CI)

Michał Winiarski (5):
  PCI/IOV: Restore VF resizable BAR state after reset
  PCI: Add a helper to identify IOV resources
  PCI: Allow IOV resources to be resized in pci_resize_resource
  PCI/IOV: Allow extending VF BAR within original resource boundary
  drm/xe/pf: Extend the VF LMEM BAR

 drivers/gpu/drm/xe/regs/xe_bars.h |   1 +
 drivers/gpu/drm/xe/xe_sriov_pf.c  |   8 ++
 drivers/pci/iov.c                 | 141 +++++++++++++++++++++++++++++-
 drivers/pci/pci.c                 |   9 +-
 drivers/pci/pci.h                 |  27 +++++-
 drivers/pci/setup-bus.c           |   5 +-
 drivers/pci/setup-res.c           |  37 ++++++--
 include/linux/pci.h               |   3 +
 include/uapi/linux/pci_regs.h     |   1 +
 9 files changed, 213 insertions(+), 19 deletions(-)

-- 
2.47.0

[PATCH v3 1/5] PCI/IOV: Restore VF resizable BAR state after reset

[Michał Winiarski]

Similar to regular resizable BAR, VF BAR can also be resized, e.g. by
the system firmware, or the PCI subsystem itself.
Add the capability ID and restore it as a part of IOV state.
See PCIe r4.0, sec 9.3.7.4.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/pci/iov.c             | 29 ++++++++++++++++++++++++++++-
 include/uapi/linux/pci_regs.h |  1 +
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index aaa33e8dc4c97..fd5c059b29c13 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2009 Intel Corporation, Yu Zhao <yu.zhao@intel.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/export.h>
@@ -862,6 +863,30 @@ static void sriov_release(struct pci_dev *dev)
 	dev->sriov = NULL;
 }
 
+static void sriov_restore_vf_rebar_state(struct pci_dev *dev)
+{
+	unsigned int pos, nbars, i;
+	u32 ctrl;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
+	if (!pos)
+		return;
+
+	pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
+	nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, ctrl);
+
+	for (i = 0; i < nbars; i++, pos += 8) {
+		int bar_idx, size;
+
+		pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
+		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
+		size = pci_rebar_bytes_to_size(dev->sriov->barsz[bar_idx]);
+		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
+		ctrl |= FIELD_PREP(PCI_REBAR_CTRL_BAR_SIZE, size);
+		pci_write_config_dword(dev, pos + PCI_REBAR_CTRL, ctrl);
+	}
+}
+
 static void sriov_restore_state(struct pci_dev *dev)
 {
 	int i;
@@ -1021,8 +1046,10 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
  */
 void pci_restore_iov_state(struct pci_dev *dev)
 {
-	if (dev->is_physfn)
+	if (dev->is_physfn) {
+		sriov_restore_vf_rebar_state(dev);
 		sriov_restore_state(dev);
+	}
 }
 
 /**
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 12323b3334a9c..a0cf701c4c3af 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -740,6 +740,7 @@
 #define PCI_EXT_CAP_ID_L1SS	0x1E	/* L1 PM Substates */
 #define PCI_EXT_CAP_ID_PTM	0x1F	/* Precision Time Measurement */
 #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
+#define PCI_EXT_CAP_ID_VF_REBAR 0x24	/* VF Resizable BAR */
 #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
 #define PCI_EXT_CAP_ID_NPEM	0x29	/* Native PCIe Enclosure Management */
-- 
2.47.0

Re: [PATCH v3 1/5] PCI/IOV: Restore VF resizable BAR state after reset

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

On Thu, 10 Oct 2024, Michał Winiarski wrote:

> Similar to regular resizable BAR, VF BAR can also be resized, e.g. by
> the system firmware, or the PCI subsystem itself.
> Add the capability ID and restore it as a part of IOV state.
> See PCIe r4.0, sec 9.3.7.4.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c             | 29 ++++++++++++++++++++++++++++-
>  include/uapi/linux/pci_regs.h |  1 +
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index aaa33e8dc4c97..fd5c059b29c13 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -7,6 +7,7 @@
>   * Copyright (C) 2009 Intel Corporation, Yu Zhao <yu.zhao@intel.com>
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <linux/export.h>
> @@ -862,6 +863,30 @@ static void sriov_release(struct pci_dev *dev)
>  	dev->sriov = NULL;
>  }
>  
> +static void sriov_restore_vf_rebar_state(struct pci_dev *dev)
> +{
> +	unsigned int pos, nbars, i;
> +	u32 ctrl;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
> +	nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, ctrl);
> +
> +	for (i = 0; i < nbars; i++, pos += 8) {
> +		int bar_idx, size;
> +
> +		pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;

Use FIELD_GET().

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

-- 
 i.

> +		size = pci_rebar_bytes_to_size(dev->sriov->barsz[bar_idx]);
> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> +		ctrl |= FIELD_PREP(PCI_REBAR_CTRL_BAR_SIZE, size);
> +		pci_write_config_dword(dev, pos + PCI_REBAR_CTRL, ctrl);
> +	}
> +}
> +
>  static void sriov_restore_state(struct pci_dev *dev)
>  {
>  	int i;
> @@ -1021,8 +1046,10 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
>   */
>  void pci_restore_iov_state(struct pci_dev *dev)
>  {
> -	if (dev->is_physfn)
> +	if (dev->is_physfn) {
> +		sriov_restore_vf_rebar_state(dev);
>  		sriov_restore_state(dev);
> +	}
>  }
>  
>  /**
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 12323b3334a9c..a0cf701c4c3af 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -740,6 +740,7 @@
>  #define PCI_EXT_CAP_ID_L1SS	0x1E	/* L1 PM Substates */
>  #define PCI_EXT_CAP_ID_PTM	0x1F	/* Precision Time Measurement */
>  #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
> +#define PCI_EXT_CAP_ID_VF_REBAR 0x24	/* VF Resizable BAR */
>  #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
>  #define PCI_EXT_CAP_ID_NPEM	0x29	/* Native PCIe Enclosure Management */
> 

Re: [PATCH v3 1/5] PCI/IOV: Restore VF resizable BAR state after reset

[Bjorn Helgaas]

On Thu, Oct 10, 2024 at 12:31:59PM +0200, Michał Winiarski wrote:
> Similar to regular resizable BAR, VF BAR can also be resized, e.g. by
> the system firmware, or the PCI subsystem itself.
> Add the capability ID and restore it as a part of IOV state.
> See PCIe r4.0, sec 9.3.7.4.

Add blank line between paragraphs.

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c             | 29 ++++++++++++++++++++++++++++-
>  include/uapi/linux/pci_regs.h |  1 +
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index aaa33e8dc4c97..fd5c059b29c13 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -7,6 +7,7 @@
>   * Copyright (C) 2009 Intel Corporation, Yu Zhao <yu.zhao@intel.com>
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <linux/export.h>
> @@ -862,6 +863,30 @@ static void sriov_release(struct pci_dev *dev)
>  	dev->sriov = NULL;
>  }
>  
> +static void sriov_restore_vf_rebar_state(struct pci_dev *dev)
> +{
> +	unsigned int pos, nbars, i;
> +	u32 ctrl;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
> +	nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, ctrl);
> +
> +	for (i = 0; i < nbars; i++, pos += 8) {
> +		int bar_idx, size;
> +
> +		pci_read_config_dword(dev, pos + PCI_REBAR_CTRL, &ctrl);
> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
> +		size = pci_rebar_bytes_to_size(dev->sriov->barsz[bar_idx]);
> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> +		ctrl |= FIELD_PREP(PCI_REBAR_CTRL_BAR_SIZE, size);
> +		pci_write_config_dword(dev, pos + PCI_REBAR_CTRL, ctrl);
> +	}
> +}
> +
>  static void sriov_restore_state(struct pci_dev *dev)
>  {
>  	int i;
> @@ -1021,8 +1046,10 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
>   */
>  void pci_restore_iov_state(struct pci_dev *dev)
>  {
> -	if (dev->is_physfn)
> +	if (dev->is_physfn) {
> +		sriov_restore_vf_rebar_state(dev);
>  		sriov_restore_state(dev);
> +	}
>  }
>  
>  /**
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 12323b3334a9c..a0cf701c4c3af 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -740,6 +740,7 @@
>  #define PCI_EXT_CAP_ID_L1SS	0x1E	/* L1 PM Substates */
>  #define PCI_EXT_CAP_ID_PTM	0x1F	/* Precision Time Measurement */
>  #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
> +#define PCI_EXT_CAP_ID_VF_REBAR 0x24	/* VF Resizable BAR */
>  #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
>  #define PCI_EXT_CAP_ID_NPEM	0x29	/* Native PCIe Enclosure Management */
> -- 
> 2.47.0
> 

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

[Michał Winiarski]

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

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4259 bytes --]

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)
> 

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

[Bjorn Helgaas]

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
> 

[PATCH v3 3/5] PCI: Allow IOV resources to be resized in pci_resize_resource

[Michał Winiarski]

Similar to regular resizable BAR, VF BAR can also be resized.
The structures are very similar, which means we can reuse most of the
implementation. See PCIe r4.0, sec 9.3.7.4.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/pci/iov.c       | 20 ++++++++++++++++++++
 drivers/pci/pci.c       |  9 ++++++++-
 drivers/pci/pci.h       |  8 ++++++++
 drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++-----
 4 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index fd5c059b29c13..591a3eae1618a 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
 	return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
 }
 
+void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size)
+{
+	if (!pci_resource_is_iov(resno)) {
+		pci_warn(dev, "%s is not an IOV resource\n",
+			 pci_resource_name(dev, resno));
+		return;
+	}
+
+	dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size;
+}
+
+bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
+{
+	u16 cmd;
+
+	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd);
+
+	return cmd & PCI_SRIOV_CTRL_MSE;
+}
+
 static void pci_read_vf_config_common(struct pci_dev *virtfn)
 {
 	struct pci_dev *physfn = virtfn->physfn;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7d85c04fbba2a..788ae61731213 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev)
  */
 static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
 {
+	int cap = PCI_EXT_CAP_ID_REBAR;
 	unsigned int pos, nbars, i;
 	u32 ctrl;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+#ifdef CONFIG_PCI_IOV
+	if (pci_resource_is_iov(bar)) {
+		cap = PCI_EXT_CAP_ID_VF_REBAR;
+		bar -= PCI_IOV_RESOURCES;
+	}
+#endif
+	pos = pci_find_ext_capability(pdev, cap);
 	if (!pos)
 		return -ENOTSUPP;
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index c55f2d7a4f37e..e15fd8fe0f81f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno)
 {
 	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
 }
+void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size);
+bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev);
 extern const struct attribute_group sriov_pf_dev_attr_group;
 extern const struct attribute_group sriov_vf_dev_attr_group;
 #else
@@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno)
 {
 	return false;
 }
+static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno,
+					     resource_size_t size) { }
+static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
+{
+	return false;
+}
 #endif /* CONFIG_PCI_IOV */
 
 #ifdef CONFIG_PCIE_PTM
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index e2cf79253ebda..95a13a5fa379c 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno)
 }
 EXPORT_SYMBOL(pci_release_resource);
 
+static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno)
+{
+	u16 cmd;
+
+	if (pci_resource_is_iov(resno))
+		return pci_iov_is_memory_decoding_enabled(dev);
+
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+
+	return cmd & PCI_COMMAND_MEMORY;
+}
+
+static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size)
+{
+	resource_size_t res_size = pci_rebar_size_to_bytes(size);
+	struct resource *res = dev->resource + resno;
+
+	if (!pci_resource_is_iov(resno)) {
+		res->end = res->start + res_size - 1;
+	} else {
+		res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1;
+		pci_iov_resource_set_size(dev, resno, res_size);
+	}
+}
+
 int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 {
 	struct resource *res = dev->resource + resno;
 	struct pci_host_bridge *host;
 	int old, ret;
 	u32 sizes;
-	u16 cmd;
 
 	/* Check if we must preserve the firmware's resource assignment */
 	host = pci_find_host_bridge(dev->bus);
@@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 	if (!(res->flags & IORESOURCE_UNSET))
 		return -EBUSY;
 
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	if (cmd & PCI_COMMAND_MEMORY)
+	if (pci_resize_is_memory_decoding_enabled(dev, resno))
 		return -EBUSY;
 
 	sizes = pci_rebar_get_possible_sizes(dev, resno);
@@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 	if (ret)
 		return ret;
 
-	res->end = res->start + pci_rebar_size_to_bytes(size) - 1;
+	pci_resize_resource_set_size(dev, resno, size);
 
 	/* Check if the new config works by trying to assign everything. */
 	if (dev->bus->self) {
@@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 
 error_resize:
 	pci_rebar_set_size(dev, resno, old);
-	res->end = res->start + pci_rebar_size_to_bytes(old) - 1;
+	pci_resize_resource_set_size(dev, resno, old);
 	return ret;
 }
 EXPORT_SYMBOL(pci_resize_resource);
-- 
2.47.0

Re: [PATCH v3 3/5] PCI: Allow IOV resources to be resized in pci_resize_resource

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 6208 bytes --]

On Thu, 10 Oct 2024, Michał Winiarski wrote:

> Similar to regular resizable BAR, VF BAR can also be resized.
> The structures are very similar, which means we can reuse most of the
> implementation. See PCIe r4.0, sec 9.3.7.4.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c       | 20 ++++++++++++++++++++
>  drivers/pci/pci.c       |  9 ++++++++-
>  drivers/pci/pci.h       |  8 ++++++++
>  drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++-----
>  4 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index fd5c059b29c13..591a3eae1618a 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
>  	return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
>  }
>  
> +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size)
> +{
> +	if (!pci_resource_is_iov(resno)) {
> +		pci_warn(dev, "%s is not an IOV resource\n",
> +			 pci_resource_name(dev, resno));
> +		return;
> +	}
> +
> +	dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size;
> +}
> +
> +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
> +{
> +	u16 cmd;
> +
> +	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd);
> +
> +	return cmd & PCI_SRIOV_CTRL_MSE;
> +}
> +
>  static void pci_read_vf_config_common(struct pci_dev *virtfn)
>  {
>  	struct pci_dev *physfn = virtfn->physfn;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7d85c04fbba2a..788ae61731213 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev)
>   */
>  static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
>  {
> +	int cap = PCI_EXT_CAP_ID_REBAR;
>  	unsigned int pos, nbars, i;
>  	u32 ctrl;
>  
> -	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +#ifdef CONFIG_PCI_IOV
> +	if (pci_resource_is_iov(bar)) {
> +		cap = PCI_EXT_CAP_ID_VF_REBAR;
> +		bar -= PCI_IOV_RESOURCES;
> +	}
> +#endif

Perhaps abstracting bar -= PCI_IOV_RESOURCES too into some static inline 
function would be useful so you could drop the ifdefs. That calculation 
seems to be done in few places besides this one.

> +	pos = pci_find_ext_capability(pdev, cap);
>  	if (!pos)
>  		return -ENOTSUPP;
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index c55f2d7a4f37e..e15fd8fe0f81f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
>  }
> +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size);
> +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev);
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return false;
>  }
> +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno,
> +					     resource_size_t size) { }
> +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_PCI_IOV */
>  
>  #ifdef CONFIG_PCIE_PTM
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e2cf79253ebda..95a13a5fa379c 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>  }
>  EXPORT_SYMBOL(pci_release_resource);
>  
> +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno)
> +{
> +	u16 cmd;
> +
> +	if (pci_resource_is_iov(resno))
> +		return pci_iov_is_memory_decoding_enabled(dev);
> +
> +	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> +
> +	return cmd & PCI_COMMAND_MEMORY;
> +}
> +
> +static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size)
> +{
> +	resource_size_t res_size = pci_rebar_size_to_bytes(size);
> +	struct resource *res = dev->resource + resno;
> +
> +	if (!pci_resource_is_iov(resno)) {
> +		res->end = res->start + res_size - 1;
> +	} else {
> +		res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1;

I wish Bjorn would pick up my resource_set_{range,size}() series [1] so we 
wouldn't need to open-code this resource size calculation everywhere.

> +		pci_iov_resource_set_size(dev, resno, res_size);
> +	}
> +}
> +
>  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  {
>  	struct resource *res = dev->resource + resno;
>  	struct pci_host_bridge *host;
>  	int old, ret;
>  	u32 sizes;
> -	u16 cmd;
>  
>  	/* Check if we must preserve the firmware's resource assignment */
>  	host = pci_find_host_bridge(dev->bus);
> @@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  	if (!(res->flags & IORESOURCE_UNSET))
>  		return -EBUSY;
>  
> -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> -	if (cmd & PCI_COMMAND_MEMORY)
> +	if (pci_resize_is_memory_decoding_enabled(dev, resno))
>  		return -EBUSY;
>  
>  	sizes = pci_rebar_get_possible_sizes(dev, resno);
> @@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  	if (ret)
>  		return ret;
>  
> -	res->end = res->start + pci_rebar_size_to_bytes(size) - 1;
> +	pci_resize_resource_set_size(dev, resno, size);
>  
>  	/* Check if the new config works by trying to assign everything. */
>  	if (dev->bus->self) {
> @@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  
>  error_resize:
>  	pci_rebar_set_size(dev, resno, old);
> -	res->end = res->start + pci_rebar_size_to_bytes(old) - 1;
> +	pci_resize_resource_set_size(dev, resno, old);
>  	return ret;
>  }
>  EXPORT_SYMBOL(pci_resize_resource);
> 

[1] https://patchwork.kernel.org/project/linux-pci/patch/20240614100606.15830-2-ilpo.jarvinen@linux.intel.com/

-- 
 i.

Re: [PATCH v3 3/5] PCI: Allow IOV resources to be resized in pci_resize_resource

[Michał Winiarski]

On Thu, Oct 10, 2024 at 02:17:11PM +0300, Ilpo Järvinen wrote:
> On Thu, 10 Oct 2024, Michał Winiarski wrote:
> 
> > Similar to regular resizable BAR, VF BAR can also be resized.
> > The structures are very similar, which means we can reuse most of the
> > implementation. See PCIe r4.0, sec 9.3.7.4.
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/pci/iov.c       | 20 ++++++++++++++++++++
> >  drivers/pci/pci.c       |  9 ++++++++-
> >  drivers/pci/pci.h       |  8 ++++++++
> >  drivers/pci/setup-res.c | 33 ++++++++++++++++++++++++++++-----
> >  4 files changed, 64 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index fd5c059b29c13..591a3eae1618a 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -154,6 +154,26 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
> >  	return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
> >  }
> >  
> > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size)
> > +{
> > +	if (!pci_resource_is_iov(resno)) {
> > +		pci_warn(dev, "%s is not an IOV resource\n",
> > +			 pci_resource_name(dev, resno));
> > +		return;
> > +	}
> > +
> > +	dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = size;
> > +}
> > +
> > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
> > +{
> > +	u16 cmd;
> > +
> > +	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd);
> > +
> > +	return cmd & PCI_SRIOV_CTRL_MSE;
> > +}
> > +
> >  static void pci_read_vf_config_common(struct pci_dev *virtfn)
> >  {
> >  	struct pci_dev *physfn = virtfn->physfn;
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 7d85c04fbba2a..788ae61731213 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3718,10 +3718,17 @@ void pci_acs_init(struct pci_dev *dev)
> >   */
> >  static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
> >  {
> > +	int cap = PCI_EXT_CAP_ID_REBAR;
> >  	unsigned int pos, nbars, i;
> >  	u32 ctrl;
> >  
> > -	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> > +#ifdef CONFIG_PCI_IOV
> > +	if (pci_resource_is_iov(bar)) {
> > +		cap = PCI_EXT_CAP_ID_VF_REBAR;
> > +		bar -= PCI_IOV_RESOURCES;
> > +	}
> > +#endif
> 
> Perhaps abstracting bar -= PCI_IOV_RESOURCES too into some static inline 
> function would be useful so you could drop the ifdefs. That calculation 
> seems to be done in few places besides this one.

I have a version of this series with helpers for conversion in both
ways:
pci_iov_resource_to_vf_bar (which is this one)
and pci_vf_bar_to_iov_resource (+= PCI_IOV_RESOURCES)

But decided to leave it out for now.
I can include it in v4 (or send it separately, depending on the
direction in which this revision goes).

> 
> > +	pos = pci_find_ext_capability(pdev, cap);
> >  	if (!pos)
> >  		return -ENOTSUPP;
> >  
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index c55f2d7a4f37e..e15fd8fe0f81f 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno)
> >  {
> >  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> >  }
> > +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size);
> > +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev);
> >  extern const struct attribute_group sriov_pf_dev_attr_group;
> >  extern const struct attribute_group sriov_vf_dev_attr_group;
> >  #else
> > @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno)
> >  {
> >  	return false;
> >  }
> > +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno,
> > +					     resource_size_t size) { }
> > +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
> > +{
> > +	return false;
> > +}
> >  #endif /* CONFIG_PCI_IOV */
> >  
> >  #ifdef CONFIG_PCIE_PTM
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index e2cf79253ebda..95a13a5fa379c 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno)
> >  }
> >  EXPORT_SYMBOL(pci_release_resource);
> >  
> > +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno)
> > +{
> > +	u16 cmd;
> > +
> > +	if (pci_resource_is_iov(resno))
> > +		return pci_iov_is_memory_decoding_enabled(dev);
> > +
> > +	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> > +
> > +	return cmd & PCI_COMMAND_MEMORY;
> > +}
> > +
> > +static void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size)
> > +{
> > +	resource_size_t res_size = pci_rebar_size_to_bytes(size);
> > +	struct resource *res = dev->resource + resno;
> > +
> > +	if (!pci_resource_is_iov(resno)) {
> > +		res->end = res->start + res_size - 1;
> > +	} else {
> > +		res->end = res->start + res_size * pci_sriov_get_totalvfs(dev) - 1;
> 
> I wish Bjorn would pick up my resource_set_{range,size}() series [1] so we 
> wouldn't need to open-code this resource size calculation everywhere.

Yeah - that looks useful indeed.

Thanks,
-Michał

> 
> > +		pci_iov_resource_set_size(dev, resno, res_size);
> > +	}
> > +}
> > +
> >  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  {
> >  	struct resource *res = dev->resource + resno;
> >  	struct pci_host_bridge *host;
> >  	int old, ret;
> >  	u32 sizes;
> > -	u16 cmd;
> >  
> >  	/* Check if we must preserve the firmware's resource assignment */
> >  	host = pci_find_host_bridge(dev->bus);
> > @@ -442,8 +466,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  	if (!(res->flags & IORESOURCE_UNSET))
> >  		return -EBUSY;
> >  
> > -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> > -	if (cmd & PCI_COMMAND_MEMORY)
> > +	if (pci_resize_is_memory_decoding_enabled(dev, resno))
> >  		return -EBUSY;
> >  
> >  	sizes = pci_rebar_get_possible_sizes(dev, resno);
> > @@ -461,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  	if (ret)
> >  		return ret;
> >  
> > -	res->end = res->start + pci_rebar_size_to_bytes(size) - 1;
> > +	pci_resize_resource_set_size(dev, resno, size);
> >  
> >  	/* Check if the new config works by trying to assign everything. */
> >  	if (dev->bus->self) {
> > @@ -473,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  
> >  error_resize:
> >  	pci_rebar_set_size(dev, resno, old);
> > -	res->end = res->start + pci_rebar_size_to_bytes(old) - 1;
> > +	pci_resize_resource_set_size(dev, resno, old);
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(pci_resize_resource);
> > 
> 
> [1] https://patchwork.kernel.org/project/linux-pci/patch/20240614100606.15830-2-ilpo.jarvinen@linux.intel.com/
> 
> -- 
>  i.

Re: [PATCH v3 3/5] PCI: Allow IOV resources to be resized in pci_resize_resource

[Bjorn Helgaas]

On Thu, Oct 10, 2024 at 12:32:01PM +0200, Michał Winiarski wrote:
> Similar to regular resizable BAR, VF BAR can also be resized.
> The structures are very similar, which means we can reuse most of the
> implementation. See PCIe r4.0, sec 9.3.7.4.

Add blank line between paragraphs.

Add "()" after function name in subject.

Add what the patch does in the commit log, not just what can be done.

>  static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
>  {
> +	int cap = PCI_EXT_CAP_ID_REBAR;
>  	unsigned int pos, nbars, i;
>  	u32 ctrl;
>  
> -	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +#ifdef CONFIG_PCI_IOV
> +	if (pci_resource_is_iov(bar)) {
> +		cap = PCI_EXT_CAP_ID_VF_REBAR;
> +		bar -= PCI_IOV_RESOURCES;
> +	}
> +#endif

Personal preference, but I'd rather set "cap" directly here instead of
setting a default and then overriding it in some cases.  Setting it
here means both settings are in the same place.

> +	pos = pci_find_ext_capability(pdev, cap);
>  	if (!pos)
>  		return -ENOTSUPP;

>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index c55f2d7a4f37e..e15fd8fe0f81f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -584,6 +584,8 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
>  }
> +void pci_iov_resource_set_size(struct pci_dev *dev, int resno, resource_size_t size);
> +bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev);
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -607,6 +609,12 @@ static inline bool pci_resource_is_iov(int resno)
>  {
>  	return false;
>  }
> +static inline void pci_iov_resource_set_size(struct pci_dev *dev, int resno,
> +					     resource_size_t size) { }
> +static inline bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_PCI_IOV */
>  
>  #ifdef CONFIG_PCIE_PTM
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e2cf79253ebda..95a13a5fa379c 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -425,13 +425,37 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>  }
>  EXPORT_SYMBOL(pci_release_resource);
>  
> +static bool pci_resize_is_memory_decoding_enabled(struct pci_dev *dev, int resno)

Wrap this (and others) to fit in 80 columns like the rest of the file.

Bjorn

[PATCH v3 4/5] PCI/IOV: Allow extending VF BAR within original resource boundary

[Michał Winiarski]

VF MMIO resource reservation, either created by system firmware and
inherited by Linux PCI subsystem or created by the subsystem itself,
contains enough space to fit the BAR of all SR-IOV Virtual Functions
that can potentially be created (total VFs supported by the device).
This can be leveraged when the device is exposing lower than optimal BAR
size as a default, allowing access to the entire resource when lower
number of VFs are created.
It is achieved by dynamically resizing the BAR to largest possible value
that allows to fit all newly created VFs within the original resource
boundary.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/pci/iov.c   | 92 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/pci.h   |  1 +
 include/linux/pci.h |  3 ++
 3 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 591a3eae1618a..f9071c1cfe9ee 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -174,6 +174,86 @@ bool pci_iov_is_memory_decoding_enabled(struct pci_dev *dev)
 	return cmd & PCI_SRIOV_CTRL_MSE;
 }
 
+static void pci_iov_resource_do_extend(struct pci_dev *dev, int resno, u16 num_vfs)
+{
+	resource_size_t size;
+	int ret, old, i;
+	u32 sizes;
+
+	pci_config_pm_runtime_get(dev);
+
+	if (pci_iov_is_memory_decoding_enabled(dev)) {
+		ret = -EBUSY;
+		goto err;
+	}
+
+	sizes = pci_rebar_get_possible_sizes(dev, resno);
+	if (!sizes) {
+		ret = -ENOTSUPP;
+		goto err;
+	}
+
+	old = pci_rebar_get_current_size(dev, resno);
+	if (old < 0) {
+		ret = old;
+		goto err;
+	}
+
+	while (sizes > 0) {
+		i = __fls(sizes);
+		size = pci_rebar_size_to_bytes(i);
+		if (size * num_vfs <= pci_resource_len(dev, resno)) {
+			if (i != old) {
+				ret = pci_rebar_set_size(dev, resno, size);
+				if (ret)
+					goto err;
+
+				pci_iov_resource_set_size(dev, resno, size);
+				pci_iov_update_resource(dev, resno);
+			}
+			break;
+		}
+		sizes &= ~BIT(i);
+	}
+
+	pci_config_pm_runtime_put(dev);
+
+	return;
+
+err:
+	pci_warn(dev, "Failed to extend %s: %d\n",
+		 pci_resource_name(dev, resno), ret);
+
+	pci_config_pm_runtime_put(dev);
+}
+
+static void pci_iov_resource_do_restore(struct pci_dev *dev, int resno)
+{
+	if (dev->sriov->rebar_extend[resno - PCI_IOV_RESOURCES])
+		pci_iov_resource_do_extend(dev, resno, dev->sriov->total_VFs);
+}
+
+int pci_iov_resource_extend(struct pci_dev *dev, int resno, bool enable)
+{
+	if (!pci_resource_is_iov(resno)) {
+		pci_warn(dev, "%s is not an IOV resource\n",
+			 pci_resource_name(dev, resno));
+
+		return -ENODEV;
+	}
+
+	if (!pci_rebar_get_possible_sizes(dev, resno))
+		return -ENOTSUPP;
+
+	if (!enable)
+		pci_iov_resource_do_restore(dev, resno);
+
+	dev->sriov->rebar_extend[resno - PCI_IOV_RESOURCES] = enable;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_iov_resource_extend);
+
 static void pci_read_vf_config_common(struct pci_dev *virtfn)
 {
 	struct pci_dev *physfn = virtfn->physfn;
@@ -438,7 +518,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
 				  const char *buf, size_t count)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	int ret = 0;
+	int i, ret = 0;
 	u16 num_vfs;
 
 	if (kstrtou16(buf, 0, &num_vfs) < 0)
@@ -480,6 +560,11 @@ static ssize_t sriov_numvfs_store(struct device *dev,
 		goto exit;
 	}
 
+	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
+		if (pdev->sriov->rebar_extend[i])
+			pci_iov_resource_do_extend(pdev, i + PCI_IOV_RESOURCES, num_vfs);
+	}
+
 	ret = pdev->driver->sriov_configure(pdev, num_vfs);
 	if (ret < 0)
 		goto exit;
@@ -874,8 +959,13 @@ static int sriov_init(struct pci_dev *dev, int pos)
 
 static void sriov_release(struct pci_dev *dev)
 {
+	int i;
+
 	BUG_ON(dev->sriov->num_VFs);
 
+	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++)
+		pci_iov_resource_do_restore(dev, i + PCI_IOV_RESOURCES);
+
 	if (dev != dev->sriov->dev)
 		pci_dev_put(dev->sriov->dev);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e15fd8fe0f81f..57e79f75e4c8f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -403,6 +403,7 @@ struct pci_sriov {
 	u16		subsystem_vendor; /* VF subsystem vendor */
 	u16		subsystem_device; /* VF subsystem device */
 	resource_size_t	barsz[PCI_SRIOV_NUM_BARS];	/* VF BAR size */
+	bool		rebar_extend[PCI_SRIOV_NUM_BARS];	/* Resize VF BAR */
 	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
 };
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 573b4c4c2be61..023c0fa1dd9f2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2371,6 +2371,7 @@ int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
 int pci_sriov_get_totalvfs(struct pci_dev *dev);
 int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn);
 resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
+int pci_iov_resource_extend(struct pci_dev *dev, int resno, bool enable);
 void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
 
 /* Arch may override these (weak) */
@@ -2423,6 +2424,8 @@ static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
 #define pci_sriov_configure_simple	NULL
 static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
 { return 0; }
+static inline int pci_iov_resource_extend(struct pci_dev *dev, int resno, bool enable)
+{ return -ENODEV; }
 static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }
 #endif
 
-- 
2.47.0

Re: [PATCH v3 4/5] PCI/IOV: Allow extending VF BAR within original resource boundary

[Bjorn Helgaas]

On Thu, Oct 10, 2024 at 12:32:02PM +0200, Michał Winiarski wrote:
> VF MMIO resource reservation, either created by system firmware and
> inherited by Linux PCI subsystem or created by the subsystem itself,
> contains enough space to fit the BAR of all SR-IOV Virtual Functions
> that can potentially be created (total VFs supported by the device).

It's *possible* that this is true, but there's no guarantee that
firmware has assigned enough space for all BARs of all possible VFs.

> This can be leveraged when the device is exposing lower than optimal BAR
> size as a default, allowing access to the entire resource when lower
> number of VFs are created.
> It is achieved by dynamically resizing the BAR to largest possible value
> that allows to fit all newly created VFs within the original resource
> boundary.

Add blank lines between paragraphs.

This log doesn't actually say what the patch does.  It describes a
possible configuration and ways that it may be used, and even *how*
something might be done, but something along the lines of the subject
line should be included in the commit log.

> +static void pci_iov_resource_do_extend(struct pci_dev *dev, int resno, u16 num_vfs)

Please wrap to fit in 80 columns like the rest of the file.

> +int pci_iov_resource_extend(struct pci_dev *dev, int resno, bool enable)

Please add kerneldoc here to help users of this exported function.

> @@ -480,6 +560,11 @@ static ssize_t sriov_numvfs_store(struct device *dev,
>  		goto exit;
>  	}
>  
> +	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> +		if (pdev->sriov->rebar_extend[i])
> +			pci_iov_resource_do_extend(pdev, i + PCI_IOV_RESOURCES, num_vfs);

Wrap to fit in 80 columns.

Bjorn

[PATCH v3 5/5] drm/xe/pf: Extend the VF LMEM BAR

[Michał Winiarski]

Opt into extending the VF BAR.
LMEM is partitioned between multiple VFs, and we expect that the more
VFs we have, the less LMEM is assigned to each VF.
This means that we can achieve full LMEM BAR access without the need to
attempt full VF LMEM BAR resize via pci_resize_resource().

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_bars.h | 1 +
 drivers/gpu/drm/xe/xe_sriov_pf.c  | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/xe/regs/xe_bars.h b/drivers/gpu/drm/xe/regs/xe_bars.h
index ce05b6ae832f1..880140d6ccdca 100644
--- a/drivers/gpu/drm/xe/regs/xe_bars.h
+++ b/drivers/gpu/drm/xe/regs/xe_bars.h
@@ -7,5 +7,6 @@
 
 #define GTTMMADR_BAR			0 /* MMIO + GTT */
 #define LMEM_BAR			2 /* VRAM */
+#define VF_LMEM_BAR			9 /* VF VRAM */
 
 #endif
diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c b/drivers/gpu/drm/xe/xe_sriov_pf.c
index 0f721ae17b266..a26719b87ac1e 100644
--- a/drivers/gpu/drm/xe/xe_sriov_pf.c
+++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
@@ -4,7 +4,9 @@
  */
 
 #include <drm/drm_managed.h>
+#include <linux/pci.h>
 
+#include "regs/xe_bars.h"
 #include "xe_assert.h"
 #include "xe_device.h"
 #include "xe_module.h"
@@ -80,8 +82,14 @@ bool xe_sriov_pf_readiness(struct xe_device *xe)
  */
 int xe_sriov_pf_init_early(struct xe_device *xe)
 {
+	int err;
+
 	xe_assert(xe, IS_SRIOV_PF(xe));
 
+	err = pci_iov_resource_extend(to_pci_dev(xe->drm.dev), VF_LMEM_BAR, true);
+	if (err)
+		xe_sriov_info(xe, "Failed to extend VF LMEM BAR: %d", err);
+
 	return drmm_mutex_init(&xe->drm, &xe->sriov.pf.master_lock);
 }
 
-- 
2.47.0