[PATCH v6 0/6] PCI: VF resizable BAR

[PATCH v6 0/6] PCI: VF resizable BAR

[Michał Winiarski]

Hi,

Next relatively small update - now that the PCI cap for regular
resizable BAR is cached, we're following suit with VF rebar cap.

v5 can be found here:
https://lore.kernel.org/linux-pci/20250312225949.969716-1-michal.winiarski@intel.com/

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 for drivers to change the VF BAR size
without resizing the entire underlying reservation (within the original
resource boundary).

Thanks,
-Michał

v5 -> v6:
- Rebased on latest pci/next
- Cache the VF resizable BAR capability position to avoid multiple
  lookups (Ilpo)
- Use pci_resource_n helper (Ilpo)

v4 -> v5:
- Rename pci_resource_to/from_iov helpers and add WARN if called without
  CONFIG_PCI_IOV (Ilpo)
- Reword kerneldoc for pci_iov_vf_bar_get_sizes (Bjorn)
- Reword commit message for VF BAR size check, extract the additional
  size check to separate conditional (Bjorn)

v3 -> v4:
- Change the approach to extending the BAR (Christian)
- Tidy the commit messages, use 80 line limit where necessary (Bjorn)
- Add kerneldocs to exported functions (Bjorn)
- Add pci_resource_to_iov() / pci_resource_from_iov() helpers (Ilpo)
- Use FIELD_GET(), tidy whitespace (Ilpo)

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)

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)

Michał Winiarski (6):
  PCI/IOV: Restore VF resizable BAR state after reset
  PCI: Add a helper to convert between VF BAR number and IOV resource
  PCI: Allow IOV resources to be resized in pci_resize_resource()
  PCI/IOV: Check that VF BAR fits within the reservation
  PCI: Allow drivers to control VF BAR size
  drm/xe/pf: Set VF LMEM BAR size

 drivers/gpu/drm/xe/regs/xe_bars.h |   1 +
 drivers/gpu/drm/xe/xe_pci_sriov.c |  22 +++++
 drivers/pci/iov.c                 | 156 +++++++++++++++++++++++++++---
 drivers/pci/pci.c                 |   8 +-
 drivers/pci/pci.h                 |  29 ++++++
 drivers/pci/setup-bus.c           |   3 +-
 drivers/pci/setup-res.c           |  35 ++++++-
 include/linux/pci.h               |   6 ++
 include/uapi/linux/pci_regs.h     |   1 +
 9 files changed, 243 insertions(+), 18 deletions(-)

-- 
2.49.0

[PATCH v6 1/6] 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>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/iov.c             | 30 +++++++++++++++++++++++++++++-
 drivers/pci/pci.h             |  1 +
 include/uapi/linux/pci_regs.h |  1 +
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 121540f57d4bf..bf95387993cd5 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>
@@ -830,6 +831,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
 	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
 		iov->link = PCI_DEVFN(PCI_SLOT(dev->devfn), iov->link);
+	iov->vf_rebar_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
 
 	if (pdev)
 		iov->dev = pci_dev_get(pdev);
@@ -868,6 +870,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 = dev->sriov->vf_rebar_cap;
+	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 = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, ctrl);
+		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;
@@ -1027,8 +1053,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/drivers/pci/pci.h b/drivers/pci/pci.h
index b81e99cd4b62a..adc54bb2c8b34 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -482,6 +482,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 */
+	u16		vf_rebar_cap;	/* VF Resizable BAR capability offset */
 	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
 };
 
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ba326710f9c8b..bb2a334e50386 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -745,6 +745,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.49.0

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

[Michał Winiarski]

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       | 22 ++++++++++++----------
 drivers/pci/pci.h       | 19 +++++++++++++++++++
 drivers/pci/setup-bus.c |  3 ++-
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index bf95387993cd5..985ea11339c45 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)
@@ -322,12 +322,13 @@ 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];
+		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
 		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,
+					     pci_resource_num_from_vf_bar(i));
 		resource_set_range(&virtfn->resource[i],
 				   res->start + size * id, size);
 		rc = request_resource(res, &virtfn->resource[i]);
@@ -624,8 +625,8 @@ 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];
+		bars |= (1 << pci_resource_num_from_vf_bar(i));
+		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
 		if (res->parent)
 			nres++;
 	}
@@ -791,8 +792,9 @@ 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);
+		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
+		res_name = pci_resource_name(dev,
+					     pci_resource_num_from_vf_bar(i));
 
 		/*
 		 * If it is already FIXED, don't change it, something
@@ -851,7 +853,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;
 	}
 
@@ -913,7 +915,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);
@@ -979,7 +981,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..55e91ba1e74a2 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];
+		struct resource *r =
+			&dev->resource[pci_resource_num_from_vf_bar(i)];
 		struct pci_bus_region region;
 
 		/* Not assigned or rejected by kernel? */
-- 
2.49.0

[PATCH v6 3/6] 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.

Extend the pci_resize_resource() function to accept IOV resources.

See PCIe r4.0, sec 9.3.7.4.

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

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 985ea11339c45..cbf335725d4fb 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -154,6 +154,27 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
 	return dev->sriov->barsz[pci_resource_num_to_vf_bar(resno)];
 }
 
+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[pci_resource_num_to_vf_bar(resno)] = 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 ff69f3d653ced..1fad9f4c54977 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3745,7 +3745,13 @@ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
 	unsigned int pos, nbars, i;
 	u32 ctrl;
 
-	pos = pdev->rebar_cap;
+	if (pci_resource_is_iov(bar)) {
+		pos = pdev->physfn ? pdev->sriov->vf_rebar_cap : 0;
+		bar = pci_resource_num_to_vf_bar(bar);
+	} else {
+		pos = pdev->rebar_cap;
+	}
+
 	if (!pos)
 		return -ENOTSUPP;
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f44840ee3c327..643cd8c737f66 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -689,6 +689,9 @@ 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);
+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);
 static inline bool pci_resource_is_iov(int resno)
 {
 	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
@@ -722,6 +725,12 @@ static inline int pci_iov_bus_range(struct pci_bus *bus)
 {
 	return 0;
 }
+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;
+}
 static inline bool pci_resource_is_iov(int resno)
 {
 	return false;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index c6657cdd06f67..d2b3ed51e8804 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -423,13 +423,39 @@ 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 = pci_resource_n(dev, resno);
+
+	if (!pci_resource_is_iov(resno)) {
+		resource_set_size(res, res_size);
+	} else {
+		resource_set_size(res, res_size * pci_sriov_get_totalvfs(dev));
+		pci_iov_resource_set_size(dev, resno, res_size);
+	}
+}
+
 int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 {
 	struct resource *res = pci_resource_n(dev, 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);
@@ -440,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);
@@ -459,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 	if (ret)
 		return ret;
 
-	resource_set_size(res, pci_rebar_size_to_bytes(size));
+	pci_resize_resource_set_size(dev, resno, size);
 
 	/* Check if the new config works by trying to assign everything. */
 	if (dev->bus->self) {
@@ -471,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 
 error_resize:
 	pci_rebar_set_size(dev, resno, old);
-	resource_set_size(res, pci_rebar_size_to_bytes(old));
+	pci_resize_resource_set_size(dev, resno, old);
 	return ret;
 }
 EXPORT_SYMBOL(pci_resize_resource);
-- 
2.49.0

[PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the reservation

[Michał Winiarski]

When the resource representing VF MMIO BAR reservation is created, its
size is always large enough to accommodate the BAR of all SR-IOV Virtual
Functions that can potentially be created (total VFs). If for whatever
reason it's not possible to accommodate all VFs - the resource is not
assigned and no VFs can be created.

The following patch will allow VF BAR size to be modified by drivers at
a later point in time, which means that the check for resource
assignment is no longer sufficient.

Add an additional check that verifies that VF BAR for all enabled VFs
fits within the underlying reservation resource.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/pci/iov.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index cbf335725d4fb..861273ad9a580 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 
 	nres = 0;
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
+		resource_size_t vf_bar_sz =
+			pci_iov_resource_size(dev,
+					      pci_resource_num_from_vf_bar(i));
 		bars |= (1 << pci_resource_num_from_vf_bar(i));
 		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
+		if (vf_bar_sz * nr_virtfn > resource_size(res))
+			continue;
 		if (res->parent)
 			nres++;
 	}
-- 
2.49.0

[PATCH v6 5/6] PCI: Allow drivers to control VF BAR size

[Michał Winiarski]

Drivers could leverage the fact that the VF BAR MMIO reservation is
created for total number of VFs supported by the device by resizing the
BAR to larger size when smaller number of VFs is enabled.

Add a pci_iov_vf_bar_set_size() function to control the size and a
pci_iov_vf_bar_get_sizes() helper to get the VF BAR sizes that will
allow up to num_vfs to be successfully enabled with the current
underlying reservation size.

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

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 861273ad9a580..751eef232685c 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -1291,3 +1291,81 @@ int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
 	return nr_virtfn;
 }
 EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
+
+/**
+ * pci_iov_vf_bar_set_size - set a new size for a VF BAR
+ * @dev: the PCI device
+ * @resno: the resource number
+ * @size: new size as defined in the spec (0=1MB, 31=128TB)
+ *
+ * Set the new size of a VF BAR that supports VF resizable BAR capability.
+ * Unlike pci_resize_resource(), this does not cause the resource that
+ * reserves the MMIO space (originally up to total_VFs) to be resized, which
+ * means that following calls to pci_enable_sriov() can fail if the resources
+ * no longer fit.
+ *
+ * Returns 0 on success, or negative on failure.
+ */
+int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
+{
+	int ret;
+	u32 sizes;
+
+	if (!pci_resource_is_iov(resno))
+		return -EINVAL;
+
+	if (pci_iov_is_memory_decoding_enabled(dev))
+		return -EBUSY;
+
+	sizes = pci_rebar_get_possible_sizes(dev, resno);
+	if (!sizes)
+		return -ENOTSUPP;
+
+	if (!(sizes & BIT(size)))
+		return -EINVAL;
+
+	ret = pci_rebar_set_size(dev, resno, size);
+	if (ret)
+		return ret;
+
+	pci_iov_resource_set_size(dev, resno, pci_rebar_size_to_bytes(size));
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_iov_vf_bar_set_size);
+
+/**
+ * pci_iov_vf_bar_get_sizes - get VF BAR sizes allowing to create up to num_vfs
+ * @dev: the PCI device
+ * @resno: the resource number
+ * @num_vfs: number of VFs
+ *
+ * Get the sizes of a VF resizable BAR that can be accommodated within the
+ * resource that reserves the MMIO space if num_vfs are enabled.
+ *
+ * Returns 0 if BAR isn't resizable, otherwise returns a bitmask in format
+ * defined in the spec (bit 0=1MB, bit 31=128TB).
+ */
+u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
+{
+	resource_size_t size;
+	u32 sizes;
+	int i;
+
+	sizes = pci_rebar_get_possible_sizes(dev, resno);
+	if (!sizes)
+		return 0;
+
+	while (sizes > 0) {
+		i = __fls(sizes);
+		size = pci_rebar_size_to_bytes(i);
+
+		if (size * num_vfs <= pci_resource_len(dev, resno))
+			break;
+
+		sizes &= ~BIT(i);
+	}
+
+	return sizes;
+}
+EXPORT_SYMBOL_GPL(pci_iov_vf_bar_get_sizes);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0e8e3fd77e967..c8708f3749757 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2389,6 +2389,8 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size);
+u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs);
 void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
 
 /* Arch may override these (weak) */
@@ -2441,6 +2443,10 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
+{ return -ENODEV; }
+static inline u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
+{ return 0; }
 static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }
 #endif
 
-- 
2.49.0

[PATCH v6 6/6] drm/xe/pf: Set VF LMEM BAR size

[Michał Winiarski]

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

Always set the largest possible BAR size that allows to fit the number
of enabled VFs.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_bars.h |  1 +
 drivers/gpu/drm/xe/xe_pci_sriov.c | 22 ++++++++++++++++++++++
 2 files changed, 23 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_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
index aaceee748287e..57cdeb41ef1d9 100644
--- a/drivers/gpu/drm/xe/xe_pci_sriov.c
+++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
@@ -3,6 +3,10 @@
  * Copyright © 2023-2024 Intel Corporation
  */
 
+#include <linux/bitops.h>
+#include <linux/pci.h>
+
+#include "regs/xe_bars.h"
 #include "xe_assert.h"
 #include "xe_device.h"
 #include "xe_gt_sriov_pf_config.h"
@@ -62,6 +66,18 @@ static void pf_reset_vfs(struct xe_device *xe, unsigned int num_vfs)
 			xe_gt_sriov_pf_control_trigger_flr(gt, n);
 }
 
+static int resize_vf_vram_bar(struct xe_device *xe, int num_vfs)
+{
+	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+	u32 sizes;
+
+	sizes = pci_iov_vf_bar_get_sizes(pdev, VF_LMEM_BAR, num_vfs);
+	if (!sizes)
+		return 0;
+
+	return pci_iov_vf_bar_set_size(pdev, VF_LMEM_BAR, __fls(sizes));
+}
+
 static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
 {
 	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
@@ -88,6 +104,12 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
 	if (err < 0)
 		goto failed;
 
+	if (IS_DGFX(xe)) {
+		err = resize_vf_vram_bar(xe, num_vfs);
+		if (err)
+			xe_sriov_info(xe, "Failed to set VF LMEM BAR size: %d\n", err);
+	}
+
 	err = pci_enable_sriov(pdev, num_vfs);
 	if (err < 0)
 		goto failed;
-- 
2.49.0

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

[Ilpo Järvinen]

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

On Thu, 20 Mar 2025, 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.

Usually it's best o refer to latest gen doc, the section number seems to 
be the same also in r6.2.

This didn't refer to spec section that specified VF Rebar ext capability
(7.8.7) though. I think it should and it would also be good to mention the 
capability layout is the same as with the rebar cap.

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/pci/iov.c             | 30 +++++++++++++++++++++++++++++-
>  drivers/pci/pci.h             |  1 +
>  include/uapi/linux/pci_regs.h |  1 +
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 121540f57d4bf..bf95387993cd5 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>
> @@ -830,6 +831,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
>  	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
>  		iov->link = PCI_DEVFN(PCI_SLOT(dev->devfn), iov->link);
> +	iov->vf_rebar_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
>  
>  	if (pdev)
>  		iov->dev = pci_dev_get(pdev);
> @@ -868,6 +870,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 = dev->sriov->vf_rebar_cap;
> +	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 = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, ctrl);
> +		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);

I started to wonder if we'd still want to have the VF Rebar ones in 
uapi/linux/pci_regs.h (despite the same capability layout):

/*
 * PCI Resizable BAR and PCI VF Resizable BAR extended capabilities have 
 * the same layout of fields.
 */
#define PCI_VF_REBAR_CTRL		PCI_REBAR_CTRL
#define PCI_VF_REBAR_CTRL_BAR_IDX	PCI_REBAR_CTRL_BAR_IDX
etc.

as then it would be possible grep to pick up only the relevant lines.

I'd not duplicate _SHIFT defines though. FIELD_PREP/GET() in general does 
not need _SHIFT defines at all and they are just duplicated information.

> +	}
> +}
> +
>  static void sriov_restore_state(struct pci_dev *dev)
>  {
>  	int i;
> @@ -1027,8 +1053,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/drivers/pci/pci.h b/drivers/pci/pci.h
> index b81e99cd4b62a..adc54bb2c8b34 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -482,6 +482,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 */
> +	u16		vf_rebar_cap;	/* VF Resizable BAR capability offset */
>  	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
>  };
>  
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index ba326710f9c8b..bb2a334e50386 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -745,6 +745,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 */

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

-- 
 i.

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

[Ilpo Järvinen]

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

On Thu, 20 Mar 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       | 22 ++++++++++++----------
>  drivers/pci/pci.h       | 19 +++++++++++++++++++
>  drivers/pci/setup-bus.c |  3 ++-
>  3 files changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index bf95387993cd5..985ea11339c45 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)
> @@ -322,12 +322,13 @@ 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];
> +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
>  		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,
> +					     pci_resource_num_from_vf_bar(i));
>  		resource_set_range(&virtfn->resource[i],
>  				   res->start + size * id, size);
>  		rc = request_resource(res, &virtfn->resource[i]);
> @@ -624,8 +625,8 @@ 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];
> +		bars |= (1 << pci_resource_num_from_vf_bar(i));
> +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
>  		if (res->parent)
>  			nres++;
>  	}
> @@ -791,8 +792,9 @@ 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);
> +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> +		res_name = pci_resource_name(dev,
> +					     pci_resource_num_from_vf_bar(i));

All these get easier to read if you add (same comment for the cases 
above):

		int idx = pci_resource_num_from_vf_bar(i);

>  
>  		/*
>  		 * If it is already FIXED, don't change it, something
> @@ -851,7 +853,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;
>  	}
>  
> @@ -913,7 +915,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);
> @@ -979,7 +981,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..55e91ba1e74a2 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];
> +		struct resource *r =
> +			&dev->resource[pci_resource_num_from_vf_bar(i)];

I'd add int idx here as well.

>  		struct pci_bus_region region;
>  
>  		/* Not assigned or rejected by kernel? */
> 

-- 
 i.

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

[Ilpo Järvinen]

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

On Wed, 26 Mar 2025, Ilpo Järvinen wrote:

> On Thu, 20 Mar 2025, 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.
> 
> Usually it's best o refer to latest gen doc, the section number seems to 
> be the same also in r6.2.

Actually, it isn't. r6.2 9.3.7 does specify capability IDs so I though you 
be refering to that section, but there's no 9.3.7.4 section at all.

--
 i.

> This didn't refer to spec section that specified VF Rebar ext capability
> (7.8.7) though. I think it should and it would also be good to mention the 
> capability layout is the same as with the rebar cap.
> 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Reviewed-by: Christian König <christian.koenig@amd.com>
> > ---
> >  drivers/pci/iov.c             | 30 +++++++++++++++++++++++++++++-
> >  drivers/pci/pci.h             |  1 +
> >  include/uapi/linux/pci_regs.h |  1 +
> >  3 files changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index 121540f57d4bf..bf95387993cd5 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>
> > @@ -830,6 +831,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
> >  	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
> >  	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
> >  		iov->link = PCI_DEVFN(PCI_SLOT(dev->devfn), iov->link);
> > +	iov->vf_rebar_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
> >  
> >  	if (pdev)
> >  		iov->dev = pci_dev_get(pdev);
> > @@ -868,6 +870,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 = dev->sriov->vf_rebar_cap;
> > +	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 = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, ctrl);
> > +		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);
> 
> I started to wonder if we'd still want to have the VF Rebar ones in 
> uapi/linux/pci_regs.h (despite the same capability layout):
> 
> /*
>  * PCI Resizable BAR and PCI VF Resizable BAR extended capabilities have 
>  * the same layout of fields.
>  */
> #define PCI_VF_REBAR_CTRL		PCI_REBAR_CTRL
> #define PCI_VF_REBAR_CTRL_BAR_IDX	PCI_REBAR_CTRL_BAR_IDX
> etc.
> 
> as then it would be possible grep to pick up only the relevant lines.
> 
> I'd not duplicate _SHIFT defines though. FIELD_PREP/GET() in general does 
> not need _SHIFT defines at all and they are just duplicated information.
> 
> > +	}
> > +}
> > +
> >  static void sriov_restore_state(struct pci_dev *dev)
> >  {
> >  	int i;
> > @@ -1027,8 +1053,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/drivers/pci/pci.h b/drivers/pci/pci.h
> > index b81e99cd4b62a..adc54bb2c8b34 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -482,6 +482,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 */
> > +	u16		vf_rebar_cap;	/* VF Resizable BAR capability offset */
> >  	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
> >  };
> >  
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index ba326710f9c8b..bb2a334e50386 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -745,6 +745,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 */
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> 

Re: [PATCH v6 3/6] PCI: Allow IOV resources to be resized in pci_resize_resource()

[Ilpo Järvinen]

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

On Thu, 20 Mar 2025, 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.

There are differences in resizing which should be described (size calc 
and mem decode check).

> Extend the pci_resize_resource() function to accept IOV resources.

> See PCIe r4.0, sec 9.3.7.4.

Can you update this to r6* please.

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c       | 21 +++++++++++++++++++++
>  drivers/pci/pci.c       |  8 +++++++-
>  drivers/pci/pci.h       |  9 +++++++++
>  drivers/pci/setup-res.c | 35 ++++++++++++++++++++++++++++++-----
>  4 files changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 985ea11339c45..cbf335725d4fb 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -154,6 +154,27 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
>  	return dev->sriov->barsz[pci_resource_num_to_vf_bar(resno)];
>  }
>  
> +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[pci_resource_num_to_vf_bar(resno)] = 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 ff69f3d653ced..1fad9f4c54977 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3745,7 +3745,13 @@ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
>  	unsigned int pos, nbars, i;
>  	u32 ctrl;
>  
> -	pos = pdev->rebar_cap;
> +	if (pci_resource_is_iov(bar)) {
> +		pos = pdev->physfn ? pdev->sriov->vf_rebar_cap : 0;

I'd explicitly do:

		if (!pdev->physfn)
			return -ENOTSUPP;

rather than relying pos = 0 triggering that return later on as the intent 
is more obvious that way.

> +		bar = pci_resource_num_to_vf_bar(bar);
> +	} else {
> +		pos = pdev->rebar_cap;
> +	}
> +
>  	if (!pos)
>  		return -ENOTSUPP;
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index f44840ee3c327..643cd8c737f66 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -689,6 +689,9 @@ 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);
> +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);
>  static inline bool pci_resource_is_iov(int resno)
>  {
>  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> @@ -722,6 +725,12 @@ static inline int pci_iov_bus_range(struct pci_bus *bus)
>  {
>  	return 0;
>  }
> +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;
> +}
>  static inline bool pci_resource_is_iov(int resno)
>  {
>  	return false;
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index c6657cdd06f67..d2b3ed51e8804 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -423,13 +423,39 @@ 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 = pci_resource_n(dev, resno);
> +
> +	if (!pci_resource_is_iov(resno)) {
> +		resource_set_size(res, res_size);
> +	} else {
> +		resource_set_size(res, res_size * pci_sriov_get_totalvfs(dev));
> +		pci_iov_resource_set_size(dev, resno, res_size);
> +	}
> +}
> +
>  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  {
>  	struct resource *res = pci_resource_n(dev, 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);
> @@ -440,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);
> @@ -459,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  	if (ret)
>  		return ret;
>  
> -	resource_set_size(res, pci_rebar_size_to_bytes(size));
> +	pci_resize_resource_set_size(dev, resno, size);
>  
>  	/* Check if the new config works by trying to assign everything. */
>  	if (dev->bus->self) {
> @@ -471,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  
>  error_resize:
>  	pci_rebar_set_size(dev, resno, old);
> -	resource_set_size(res, pci_rebar_size_to_bytes(old));
> +	pci_resize_resource_set_size(dev, resno, old);
>  	return ret;
>  }
>  EXPORT_SYMBOL(pci_resize_resource);
> 

-- 
 i.

Re: [PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the reservation

[Ilpo Järvinen]

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

On Thu, 20 Mar 2025, Michał Winiarski wrote:

> When the resource representing VF MMIO BAR reservation is created, its
> size is always large enough to accommodate the BAR of all SR-IOV Virtual
> Functions that can potentially be created (total VFs). If for whatever
> reason it's not possible to accommodate all VFs - the resource is not
> assigned and no VFs can be created.
> 
> The following patch will allow VF BAR size to be modified by drivers at

"The following patch" sounds to be like you're referring to patch that 
follows this description, ie., the patch below. "An upcoming change" is 
alternative that doesn't suffer from the same problem.

> a later point in time, which means that the check for resource
> assignment is no longer sufficient.
> 
> Add an additional check that verifies that VF BAR for all enabled VFs
> fits within the underlying reservation resource.

So this does not solve the case where the initial size was too large to 
fix and such VF BARs remain unassigned, right?

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index cbf335725d4fb..861273ad9a580 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>  
>  	nres = 0;
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> +		resource_size_t vf_bar_sz =
> +			pci_iov_resource_size(dev,
> +					      pci_resource_num_from_vf_bar(i));

Please add int idx = pci_resource_num_from_vf_bar(i);

>  		bars |= (1 << pci_resource_num_from_vf_bar(i));
>  		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> +		if (vf_bar_sz * nr_virtfn > resource_size(res))
> +			continue;

Not directly related to this patch, I suspect this could actually try to 
assign an unassigned resource by doing something like this (perhaps in own 
patch, it doesn't even need to be part of this series but can be sent 
later if you find the suggestion useful):

		/* Retry assignment if the initial size didn't fit */
		if (!res->parent && pci_assign_resource(res, idx))
			continue;

Although I suspect reset_resource() might have been called for the 
resource and IIRC it breaks the resource somehow but it could have been 
that IOV resources can be resummoned from that state though thanks to 
their size not being stored into the resource itself but comes from iov 
structures.

>  		if (res->parent)
>  			nres++;
>  	}
> 

-- 
 i.

Re: [PATCH v6 5/6] PCI: Allow drivers to control VF BAR size

[Ilpo Järvinen]

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

On Thu, 20 Mar 2025, Michał Winiarski wrote:

> Drivers could leverage the fact that the VF BAR MMIO reservation is
> created for total number of VFs supported by the device by resizing the
> BAR to larger size when smaller number of VFs is enabled.
> 
> Add a pci_iov_vf_bar_set_size() function to control the size and a
> pci_iov_vf_bar_get_sizes() helper to get the VF BAR sizes that will
> allow up to num_vfs to be successfully enabled with the current
> underlying reservation size.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/pci/iov.c   | 78 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  6 ++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 861273ad9a580..751eef232685c 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -1291,3 +1291,81 @@ int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
>  	return nr_virtfn;
>  }
>  EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
> +
> +/**
> + * pci_iov_vf_bar_set_size - set a new size for a VF BAR
> + * @dev: the PCI device
> + * @resno: the resource number
> + * @size: new size as defined in the spec (0=1MB, 31=128TB)
> + *
> + * Set the new size of a VF BAR that supports VF resizable BAR capability.
> + * Unlike pci_resize_resource(), this does not cause the resource that
> + * reserves the MMIO space (originally up to total_VFs) to be resized, which
> + * means that following calls to pci_enable_sriov() can fail if the resources
> + * no longer fit.
> + *
> + * Returns 0 on success, or negative on failure.

Return: is the correct kernel doc style.

> + */
> +int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
> +{
> +	int ret;
> +	u32 sizes;
> +
> +	if (!pci_resource_is_iov(resno))
> +		return -EINVAL;
> +
> +	if (pci_iov_is_memory_decoding_enabled(dev))
> +		return -EBUSY;
> +
> +	sizes = pci_rebar_get_possible_sizes(dev, resno);
> +	if (!sizes)
> +		return -ENOTSUPP;
> +
> +	if (!(sizes & BIT(size)))
> +		return -EINVAL;
> +
> +	ret = pci_rebar_set_size(dev, resno, size);
> +	if (ret)
> +		return ret;
> +
> +	pci_iov_resource_set_size(dev, resno, pci_rebar_size_to_bytes(size));
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_set_size);
> +
> +/**
> + * pci_iov_vf_bar_get_sizes - get VF BAR sizes allowing to create up to num_vfs
> + * @dev: the PCI device
> + * @resno: the resource number
> + * @num_vfs: number of VFs
> + *
> + * Get the sizes of a VF resizable BAR that can be accommodated within the
> + * resource that reserves the MMIO space if num_vfs are enabled.

I'd rephrase to:

Get the sizes of a VF resizable BAR that can be accommodate @num_vfs 
within the currently assigned size of the resource @resno.

> + *
> + * Returns 0 if BAR isn't resizable, otherwise returns a bitmask in format

Return:

a bitmask of sizes

> + * defined in the spec (bit 0=1MB, bit 31=128TB).
> + */
> +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> +{
> +	resource_size_t size;
> +	u32 sizes;
> +	int i;
> +
> +	sizes = pci_rebar_get_possible_sizes(dev, resno);
> +	if (!sizes)
> +		return 0;
> +
> +	while (sizes > 0) {
> +		i = __fls(sizes);
> +		size = pci_rebar_size_to_bytes(i);
> +
> +		if (size * num_vfs <= pci_resource_len(dev, resno))
> +			break;
> +
> +		sizes &= ~BIT(i);
> +	}

Couldn't this be handled without a loop:

	bar_sizes = (round_up(pci_resource_len(dev, resno) / num_vfs) - 1) >>
		    ilog2(SZ_1M);

	sizes &= bar_sizes;

(Just to given an idea, I wrote this into the email so it might contain 
some off-by-one errors or like).

> +
> +	return sizes;
> +}
> +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_get_sizes);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 0e8e3fd77e967..c8708f3749757 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2389,6 +2389,8 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size);
> +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs);
>  void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
>  
>  /* Arch may override these (weak) */
> @@ -2441,6 +2443,10 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
> +{ return -ENODEV; }
> +static inline u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> +{ return 0; }
>  static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }
>  #endif
>  
> 

-- 
 i.

Re: [PATCH v6 6/6] drm/xe/pf: Set VF LMEM BAR size

[Ilpo Järvinen]

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

On Thu, 20 Mar 2025, Michał Winiarski wrote:

> 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().
> 
> Always set the largest possible BAR size that allows to fit the number
> of enabled VFs.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_bars.h |  1 +
>  drivers/gpu/drm/xe/xe_pci_sriov.c | 22 ++++++++++++++++++++++
>  2 files changed, 23 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_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
> index aaceee748287e..57cdeb41ef1d9 100644
> --- a/drivers/gpu/drm/xe/xe_pci_sriov.c
> +++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
> @@ -3,6 +3,10 @@
>   * Copyright © 2023-2024 Intel Corporation
>   */
>  
> +#include <linux/bitops.h>
> +#include <linux/pci.h>
> +
> +#include "regs/xe_bars.h"
>  #include "xe_assert.h"
>  #include "xe_device.h"
>  #include "xe_gt_sriov_pf_config.h"
> @@ -62,6 +66,18 @@ static void pf_reset_vfs(struct xe_device *xe, unsigned int num_vfs)
>  			xe_gt_sriov_pf_control_trigger_flr(gt, n);
>  }
>  
> +static int resize_vf_vram_bar(struct xe_device *xe, int num_vfs)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	u32 sizes;
> +
> +	sizes = pci_iov_vf_bar_get_sizes(pdev, VF_LMEM_BAR, num_vfs);
> +	if (!sizes)
> +		return 0;
> +
> +	return pci_iov_vf_bar_set_size(pdev, VF_LMEM_BAR, __fls(sizes));
> +}
> +
>  static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>  {
>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> @@ -88,6 +104,12 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>  	if (err < 0)
>  		goto failed;
>  
> +	if (IS_DGFX(xe)) {
> +		err = resize_vf_vram_bar(xe, num_vfs);
> +		if (err)
> +			xe_sriov_info(xe, "Failed to set VF LMEM BAR size: %d\n", err);

If you intended this error to not result in failure, please mention it 
in the changelog so that it's recorded somewhere for those who have to 
look up things from the git history one day :-).

> +	}
> +
>  	err = pci_enable_sriov(pdev, num_vfs);
>  	if (err < 0)
>  		goto failed;

Seems pretty straightforward after reading the support code on the PCI 
core side,

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

-- 
 i.

Re: [PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the reservation

[Ilpo Järvinen]

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

On Wed, 26 Mar 2025, Ilpo Järvinen wrote:

> On Thu, 20 Mar 2025, Michał Winiarski wrote:
> 
> > When the resource representing VF MMIO BAR reservation is created, its
> > size is always large enough to accommodate the BAR of all SR-IOV Virtual
> > Functions that can potentially be created (total VFs). If for whatever
> > reason it's not possible to accommodate all VFs - the resource is not
> > assigned and no VFs can be created.
> > 
> > The following patch will allow VF BAR size to be modified by drivers at
> 
> "The following patch" sounds to be like you're referring to patch that 
> follows this description, ie., the patch below. "An upcoming change" is 
> alternative that doesn't suffer from the same problem.
> 
> > a later point in time, which means that the check for resource
> > assignment is no longer sufficient.
> > 
> > Add an additional check that verifies that VF BAR for all enabled VFs
> > fits within the underlying reservation resource.
> 
> So this does not solve the case where the initial size was too large to 
> fix and such VF BARs remain unassigned, right?
> 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/pci/iov.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index cbf335725d4fb..861273ad9a580 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> >  
> >  	nres = 0;
> >  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > +		resource_size_t vf_bar_sz =
> > +			pci_iov_resource_size(dev,
> > +					      pci_resource_num_from_vf_bar(i));
> 
> Please add int idx = pci_resource_num_from_vf_bar(i);
> 
> >  		bars |= (1 << pci_resource_num_from_vf_bar(i));
> >  		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> > +		if (vf_bar_sz * nr_virtfn > resource_size(res))
> > +			continue;
> 
> Not directly related to this patch, I suspect this could actually try to 
> assign an unassigned resource by doing something like this (perhaps in own 
> patch, it doesn't even need to be part of this series but can be sent 
> later if you find the suggestion useful):
> 
> 		/* Retry assignment if the initial size didn't fit */
> 		if (!res->parent && pci_assign_resource(res, idx))
> 			continue;
> 
> Although I suspect reset_resource() might have been called for the 
> resource and IIRC it breaks the resource somehow but it could have been 
> that IOV resources can be resummoned from that state though thanks to 
> their size not being stored into the resource itself but comes from iov 
> structures.

I realized reset_resource() will zero the flags so it won't work without 
getting rid of reset_resource() calls first which I've not yet completed. 

And once I get the rebar sizes included into bridge window sizing 
algorithm, the default size could possibly be shrunk by the resource
fitting/assignment code so the resource assignment should no longer fail 
just because the initial size was too large. So it shouldn't be necessary 
after that.

> >  		if (res->parent)
> >  			nres++;
> >  	}
> > 
> 
> 

-- 
 i.

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

[Michał Winiarski]

On Wed, Mar 26, 2025 at 04:52:22PM +0200, Ilpo Järvinen wrote:
> On Wed, 26 Mar 2025, Ilpo Järvinen wrote:
> 
> > On Thu, 20 Mar 2025, 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.
> > 
> > Usually it's best o refer to latest gen doc, the section number seems to 
> > be the same also in r6.2.
> 
> Actually, it isn't. r6.2 9.3.7 does specify capability IDs so I though you 
> be refering to that section, but there's no 9.3.7.4 section at all.

Yeah - I'll change it to refer to r6.2:
"See PCIe r6.2, sec 7.8.7."

> 
> --
>  i.
> 
> > This didn't refer to spec section that specified VF Rebar ext capability
> > (7.8.7) though. I think it should and it would also be good to mention the 
> > capability layout is the same as with the rebar cap.
> > 
> > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Reviewed-by: Christian König <christian.koenig@amd.com>
> > > ---
> > >  drivers/pci/iov.c             | 30 +++++++++++++++++++++++++++++-
> > >  drivers/pci/pci.h             |  1 +
> > >  include/uapi/linux/pci_regs.h |  1 +
> > >  3 files changed, 31 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > > index 121540f57d4bf..bf95387993cd5 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>
> > > @@ -830,6 +831,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
> > >  	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
> > >  	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
> > >  		iov->link = PCI_DEVFN(PCI_SLOT(dev->devfn), iov->link);
> > > +	iov->vf_rebar_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VF_REBAR);
> > >  
> > >  	if (pdev)
> > >  		iov->dev = pci_dev_get(pdev);
> > > @@ -868,6 +870,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 = dev->sriov->vf_rebar_cap;
> > > +	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 = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, ctrl);
> > > +		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);
> > 
> > I started to wonder if we'd still want to have the VF Rebar ones in 
> > uapi/linux/pci_regs.h (despite the same capability layout):
> > 
> > /*
> >  * PCI Resizable BAR and PCI VF Resizable BAR extended capabilities have 
> >  * the same layout of fields.
> >  */
> > #define PCI_VF_REBAR_CTRL		PCI_REBAR_CTRL
> > #define PCI_VF_REBAR_CTRL_BAR_IDX	PCI_REBAR_CTRL_BAR_IDX
> > etc.
> > 
> > as then it would be possible grep to pick up only the relevant lines.
> > 
> > I'd not duplicate _SHIFT defines though. FIELD_PREP/GET() in general does 
> > not need _SHIFT defines at all and they are just duplicated information.

And I'll also add the defines for VF_REBAR and usage for config space
restore.

Thanks,
-Michał

> > 
> > > +	}
> > > +}
> > > +
> > >  static void sriov_restore_state(struct pci_dev *dev)
> > >  {
> > >  	int i;
> > > @@ -1027,8 +1053,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/drivers/pci/pci.h b/drivers/pci/pci.h
> > > index b81e99cd4b62a..adc54bb2c8b34 100644
> > > --- a/drivers/pci/pci.h
> > > +++ b/drivers/pci/pci.h
> > > @@ -482,6 +482,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 */
> > > +	u16		vf_rebar_cap;	/* VF Resizable BAR capability offset */
> > >  	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
> > >  };
> > >  
> > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > index ba326710f9c8b..bb2a334e50386 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -745,6 +745,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 */
> > 
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > 
> > 

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

[Michał Winiarski]

On Wed, Mar 26, 2025 at 04:46:19PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 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       | 22 ++++++++++++----------
> >  drivers/pci/pci.h       | 19 +++++++++++++++++++
> >  drivers/pci/setup-bus.c |  3 ++-
> >  3 files changed, 33 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index bf95387993cd5..985ea11339c45 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)
> > @@ -322,12 +322,13 @@ 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];
> > +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> >  		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,
> > +					     pci_resource_num_from_vf_bar(i));
> >  		resource_set_range(&virtfn->resource[i],
> >  				   res->start + size * id, size);
> >  		rc = request_resource(res, &virtfn->resource[i]);
> > @@ -624,8 +625,8 @@ 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];
> > +		bars |= (1 << pci_resource_num_from_vf_bar(i));
> > +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> >  		if (res->parent)
> >  			nres++;
> >  	}
> > @@ -791,8 +792,9 @@ 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);
> > +		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> > +		res_name = pci_resource_name(dev,
> > +					     pci_resource_num_from_vf_bar(i));
> 
> All these get easier to read if you add (same comment for the cases 
> above):
> 
> 		int idx = pci_resource_num_from_vf_bar(i);

Ok.

> 
> >  
> >  		/*
> >  		 * If it is already FIXED, don't change it, something
> > @@ -851,7 +853,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;
> >  	}
> >  
> > @@ -913,7 +915,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);
> > @@ -979,7 +981,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..55e91ba1e74a2 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];
> > +		struct resource *r =
> > +			&dev->resource[pci_resource_num_from_vf_bar(i)];
> 
> I'd add int idx here as well.

Ok, makes sense, I'll use the similar approach in the following patches
as well.

Thanks,
-Michał

> 
> >  		struct pci_bus_region region;
> >  
> >  		/* Not assigned or rejected by kernel? */
> > 
> 
> -- 
>  i.

Re: [PATCH v6 3/6] PCI: Allow IOV resources to be resized in pci_resize_resource()

[Michał Winiarski]

On Wed, Mar 26, 2025 at 04:58:14PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, 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.
> 
> There are differences in resizing which should be described (size calc 
> and mem decode check).

I'll change it to:

"The capability layout is the same as PCI_EXT_CAP_ID_REBAR, which means
we can reuse most of the implementation, the only differences being
resource size calculation (which is multiplied by total VFs) and memory
decoding (which is controlled by a separate VF MSE field in SR-IOV
cap)."

> 
> > Extend the pci_resize_resource() function to accept IOV resources.
> 
> > See PCIe r4.0, sec 9.3.7.4.
> 
> Can you update this to r6* please.

Sure, r6.2, sec 7.8.7.

> 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/pci/iov.c       | 21 +++++++++++++++++++++
> >  drivers/pci/pci.c       |  8 +++++++-
> >  drivers/pci/pci.h       |  9 +++++++++
> >  drivers/pci/setup-res.c | 35 ++++++++++++++++++++++++++++++-----
> >  4 files changed, 67 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index 985ea11339c45..cbf335725d4fb 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -154,6 +154,27 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
> >  	return dev->sriov->barsz[pci_resource_num_to_vf_bar(resno)];
> >  }
> >  
> > +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[pci_resource_num_to_vf_bar(resno)] = 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 ff69f3d653ced..1fad9f4c54977 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3745,7 +3745,13 @@ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
> >  	unsigned int pos, nbars, i;
> >  	u32 ctrl;
> >  
> > -	pos = pdev->rebar_cap;
> > +	if (pci_resource_is_iov(bar)) {
> > +		pos = pdev->physfn ? pdev->sriov->vf_rebar_cap : 0;
> 
> I'd explicitly do:
> 
> 		if (!pdev->physfn)
> 			return -ENOTSUPP;
> 
> rather than relying pos = 0 triggering that return later on as the intent 
> is more obvious that way.

Ok.

Thanks,
-Michał

> 
> > +		bar = pci_resource_num_to_vf_bar(bar);
> > +	} else {
> > +		pos = pdev->rebar_cap;
> > +	}
> > +
> >  	if (!pos)
> >  		return -ENOTSUPP;
> >  
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index f44840ee3c327..643cd8c737f66 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -689,6 +689,9 @@ 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);
> > +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);
> >  static inline bool pci_resource_is_iov(int resno)
> >  {
> >  	return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> > @@ -722,6 +725,12 @@ static inline int pci_iov_bus_range(struct pci_bus *bus)
> >  {
> >  	return 0;
> >  }
> > +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;
> > +}
> >  static inline bool pci_resource_is_iov(int resno)
> >  {
> >  	return false;
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index c6657cdd06f67..d2b3ed51e8804 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -423,13 +423,39 @@ 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 = pci_resource_n(dev, resno);
> > +
> > +	if (!pci_resource_is_iov(resno)) {
> > +		resource_set_size(res, res_size);
> > +	} else {
> > +		resource_set_size(res, res_size * pci_sriov_get_totalvfs(dev));
> > +		pci_iov_resource_set_size(dev, resno, res_size);
> > +	}
> > +}
> > +
> >  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  {
> >  	struct resource *res = pci_resource_n(dev, 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);
> > @@ -440,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);
> > @@ -459,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  	if (ret)
> >  		return ret;
> >  
> > -	resource_set_size(res, pci_rebar_size_to_bytes(size));
> > +	pci_resize_resource_set_size(dev, resno, size);
> >  
> >  	/* Check if the new config works by trying to assign everything. */
> >  	if (dev->bus->self) {
> > @@ -471,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >  
> >  error_resize:
> >  	pci_rebar_set_size(dev, resno, old);
> > -	resource_set_size(res, pci_rebar_size_to_bytes(old));
> > +	pci_resize_resource_set_size(dev, resno, old);
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(pci_resize_resource);
> > 
> 
> -- 
>  i.

Re: [PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the reservation

[Michał Winiarski]

On Wed, Mar 26, 2025 at 05:11:04PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, Michał Winiarski wrote:
> 
> > When the resource representing VF MMIO BAR reservation is created, its
> > size is always large enough to accommodate the BAR of all SR-IOV Virtual
> > Functions that can potentially be created (total VFs). If for whatever
> > reason it's not possible to accommodate all VFs - the resource is not
> > assigned and no VFs can be created.
> > 
> > The following patch will allow VF BAR size to be modified by drivers at
> 
> "The following patch" sounds to be like you're referring to patch that 
> follows this description, ie., the patch below. "An upcoming change" is 
> alternative that doesn't suffer from the same problem.

Ok.

> 
> > a later point in time, which means that the check for resource
> > assignment is no longer sufficient.
> > 
> > Add an additional check that verifies that VF BAR for all enabled VFs
> > fits within the underlying reservation resource.
> 
> So this does not solve the case where the initial size was too large to 
> fix and such VF BARs remain unassigned, right?

Right - and in my opinion VF enabling is not the right point in time to
try and salvage the PF resource resevation.

-Michał

> 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/pci/iov.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index cbf335725d4fb..861273ad9a580 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> >  
> >  	nres = 0;
> >  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > +		resource_size_t vf_bar_sz =
> > +			pci_iov_resource_size(dev,
> > +					      pci_resource_num_from_vf_bar(i));
> 
> Please add int idx = pci_resource_num_from_vf_bar(i);
> 
> >  		bars |= (1 << pci_resource_num_from_vf_bar(i));
> >  		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> > +		if (vf_bar_sz * nr_virtfn > resource_size(res))
> > +			continue;
> 
> Not directly related to this patch, I suspect this could actually try to 
> assign an unassigned resource by doing something like this (perhaps in own 
> patch, it doesn't even need to be part of this series but can be sent 
> later if you find the suggestion useful):
> 
> 		/* Retry assignment if the initial size didn't fit */
> 		if (!res->parent && pci_assign_resource(res, idx))
> 			continue;
> 
> Although I suspect reset_resource() might have been called for the 
> resource and IIRC it breaks the resource somehow but it could have been 
> that IOV resources can be resummoned from that state though thanks to 
> their size not being stored into the resource itself but comes from iov 
> structures.
> 
> >  		if (res->parent)
> >  			nres++;
> >  	}
> > 
> 
> -- 
>  i.

Re: [PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the reservation

[Michał Winiarski]

On Fri, Mar 28, 2025 at 06:39:29PM +0200, Ilpo Järvinen wrote:
> On Wed, 26 Mar 2025, Ilpo Järvinen wrote:
> 
> > On Thu, 20 Mar 2025, Michał Winiarski wrote:
> > 
> > > When the resource representing VF MMIO BAR reservation is created, its
> > > size is always large enough to accommodate the BAR of all SR-IOV Virtual
> > > Functions that can potentially be created (total VFs). If for whatever
> > > reason it's not possible to accommodate all VFs - the resource is not
> > > assigned and no VFs can be created.
> > > 
> > > The following patch will allow VF BAR size to be modified by drivers at
> > 
> > "The following patch" sounds to be like you're referring to patch that 
> > follows this description, ie., the patch below. "An upcoming change" is 
> > alternative that doesn't suffer from the same problem.
> > 
> > > a later point in time, which means that the check for resource
> > > assignment is no longer sufficient.
> > > 
> > > Add an additional check that verifies that VF BAR for all enabled VFs
> > > fits within the underlying reservation resource.
> > 
> > So this does not solve the case where the initial size was too large to 
> > fix and such VF BARs remain unassigned, right?
> > 
> > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > > ---
> > >  drivers/pci/iov.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > > index cbf335725d4fb..861273ad9a580 100644
> > > --- a/drivers/pci/iov.c
> > > +++ b/drivers/pci/iov.c
> > > @@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> > >  
> > >  	nres = 0;
> > >  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > > +		resource_size_t vf_bar_sz =
> > > +			pci_iov_resource_size(dev,
> > > +					      pci_resource_num_from_vf_bar(i));
> > 
> > Please add int idx = pci_resource_num_from_vf_bar(i);
> > 
> > >  		bars |= (1 << pci_resource_num_from_vf_bar(i));
> > >  		res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> > > +		if (vf_bar_sz * nr_virtfn > resource_size(res))
> > > +			continue;
> > 
> > Not directly related to this patch, I suspect this could actually try to 
> > assign an unassigned resource by doing something like this (perhaps in own 
> > patch, it doesn't even need to be part of this series but can be sent 
> > later if you find the suggestion useful):
> > 
> > 		/* Retry assignment if the initial size didn't fit */
> > 		if (!res->parent && pci_assign_resource(res, idx))
> > 			continue;
> > 
> > Although I suspect reset_resource() might have been called for the 
> > resource and IIRC it breaks the resource somehow but it could have been 
> > that IOV resources can be resummoned from that state though thanks to 
> > their size not being stored into the resource itself but comes from iov 
> > structures.
> 
> I realized reset_resource() will zero the flags so it won't work without 
> getting rid of reset_resource() calls first which I've not yet completed. 
> 
> And once I get the rebar sizes included into bridge window sizing 
> algorithm, the default size could possibly be shrunk by the resource
> fitting/assignment code so the resource assignment should no longer fail 
> just because the initial size was too large. So it shouldn't be necessary 
> after that.

Yeah - and even if something fails in the resource constrained
environment, I think the flow used to reassign the PF resource (and
bring back the ability to create VFs) should involve remove -> rescan
(not just VF enabling).

Thanks,
-Michał

> 
> > >  		if (res->parent)
> > >  			nres++;
> > >  	}
> > > 
> > 
> > 
> 
> -- 
>  i.

Re: [PATCH v6 5/6] PCI: Allow drivers to control VF BAR size

[Michał Winiarski]

On Wed, Mar 26, 2025 at 05:22:50PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, Michał Winiarski wrote:
> 
> > Drivers could leverage the fact that the VF BAR MMIO reservation is
> > created for total number of VFs supported by the device by resizing the
> > BAR to larger size when smaller number of VFs is enabled.
> > 
> > Add a pci_iov_vf_bar_set_size() function to control the size and a
> > pci_iov_vf_bar_get_sizes() helper to get the VF BAR sizes that will
> > allow up to num_vfs to be successfully enabled with the current
> > underlying reservation size.
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/pci/iov.c   | 78 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci.h |  6 ++++
> >  2 files changed, 84 insertions(+)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index 861273ad9a580..751eef232685c 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -1291,3 +1291,81 @@ int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
> >  	return nr_virtfn;
> >  }
> >  EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
> > +
> > +/**
> > + * pci_iov_vf_bar_set_size - set a new size for a VF BAR
> > + * @dev: the PCI device
> > + * @resno: the resource number
> > + * @size: new size as defined in the spec (0=1MB, 31=128TB)
> > + *
> > + * Set the new size of a VF BAR that supports VF resizable BAR capability.
> > + * Unlike pci_resize_resource(), this does not cause the resource that
> > + * reserves the MMIO space (originally up to total_VFs) to be resized, which
> > + * means that following calls to pci_enable_sriov() can fail if the resources
> > + * no longer fit.
> > + *
> > + * Returns 0 on success, or negative on failure.
> 
> Return: is the correct kernel doc style.

Yeah, I just blindly followed the style from surrounding docs. I'll
change it here.

> 
> > + */
> > +int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
> > +{
> > +	int ret;
> > +	u32 sizes;
> > +
> > +	if (!pci_resource_is_iov(resno))
> > +		return -EINVAL;
> > +
> > +	if (pci_iov_is_memory_decoding_enabled(dev))
> > +		return -EBUSY;
> > +
> > +	sizes = pci_rebar_get_possible_sizes(dev, resno);
> > +	if (!sizes)
> > +		return -ENOTSUPP;
> > +
> > +	if (!(sizes & BIT(size)))
> > +		return -EINVAL;
> > +
> > +	ret = pci_rebar_set_size(dev, resno, size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_iov_resource_set_size(dev, resno, pci_rebar_size_to_bytes(size));
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_set_size);
> > +
> > +/**
> > + * pci_iov_vf_bar_get_sizes - get VF BAR sizes allowing to create up to num_vfs
> > + * @dev: the PCI device
> > + * @resno: the resource number
> > + * @num_vfs: number of VFs
> > + *
> > + * Get the sizes of a VF resizable BAR that can be accommodated within the
> > + * resource that reserves the MMIO space if num_vfs are enabled.
> 
> I'd rephrase to:
> 
> Get the sizes of a VF resizable BAR that can be accommodate @num_vfs 
> within the currently assigned size of the resource @resno.

Ok.

> 
> > + *
> > + * Returns 0 if BAR isn't resizable, otherwise returns a bitmask in format
> 
> Return:
> 
> a bitmask of sizes

Ok.

> 
> > + * defined in the spec (bit 0=1MB, bit 31=128TB).
> > + */
> > +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> > +{
> > +	resource_size_t size;
> > +	u32 sizes;
> > +	int i;
> > +
> > +	sizes = pci_rebar_get_possible_sizes(dev, resno);
> > +	if (!sizes)
> > +		return 0;
> > +
> > +	while (sizes > 0) {
> > +		i = __fls(sizes);
> > +		size = pci_rebar_size_to_bytes(i);
> > +
> > +		if (size * num_vfs <= pci_resource_len(dev, resno))
> > +			break;
> > +
> > +		sizes &= ~BIT(i);
> > +	}
> 
> Couldn't this be handled without a loop:
> 
> 	bar_sizes = (round_up(pci_resource_len(dev, resno) / num_vfs) - 1) >>
> 		    ilog2(SZ_1M);
> 
> 	sizes &= bar_sizes;
> 
> (Just to given an idea, I wrote this into the email so it might contain 
> some off-by-one errors or like).

I think the division will need to be wrapped with something like do_div
(because IIUC, we have 32bit architectures where resource_size_t is
u64).

But yeah, we can drop the loop, turning it into something like this:

	vf_len = pci_resource_len(dev, resno);
	do_div(vf_len, num_vfs);
	sizes = (roundup_pow_of_two(vf_len + 1) - 1) >> ilog2(SZ_1M);

Thanks,
-Michał

> 
> > +
> > +	return sizes;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_get_sizes);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 0e8e3fd77e967..c8708f3749757 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2389,6 +2389,8 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size);
> > +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs);
> >  void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
> >  
> >  /* Arch may override these (weak) */
> > @@ -2441,6 +2443,10 @@ 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_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
> > +{ return -ENODEV; }
> > +static inline u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> > +{ return 0; }
> >  static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }
> >  #endif
> >  
> > 
> 
> -- 
>  i.

Re: [PATCH v6 6/6] drm/xe/pf: Set VF LMEM BAR size

[Michał Winiarski]

On Wed, Mar 26, 2025 at 05:29:31PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, Michał Winiarski wrote:
> 
> > 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().
> > 
> > Always set the largest possible BAR size that allows to fit the number
> > of enabled VFs.
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/gpu/drm/xe/regs/xe_bars.h |  1 +
> >  drivers/gpu/drm/xe/xe_pci_sriov.c | 22 ++++++++++++++++++++++
> >  2 files changed, 23 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_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
> > index aaceee748287e..57cdeb41ef1d9 100644
> > --- a/drivers/gpu/drm/xe/xe_pci_sriov.c
> > +++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
> > @@ -3,6 +3,10 @@
> >   * Copyright © 2023-2024 Intel Corporation
> >   */
> >  
> > +#include <linux/bitops.h>
> > +#include <linux/pci.h>
> > +
> > +#include "regs/xe_bars.h"
> >  #include "xe_assert.h"
> >  #include "xe_device.h"
> >  #include "xe_gt_sriov_pf_config.h"
> > @@ -62,6 +66,18 @@ static void pf_reset_vfs(struct xe_device *xe, unsigned int num_vfs)
> >  			xe_gt_sriov_pf_control_trigger_flr(gt, n);
> >  }
> >  
> > +static int resize_vf_vram_bar(struct xe_device *xe, int num_vfs)
> > +{
> > +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> > +	u32 sizes;
> > +
> > +	sizes = pci_iov_vf_bar_get_sizes(pdev, VF_LMEM_BAR, num_vfs);
> > +	if (!sizes)
> > +		return 0;
> > +
> > +	return pci_iov_vf_bar_set_size(pdev, VF_LMEM_BAR, __fls(sizes));
> > +}
> > +
> >  static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
> >  {
> >  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> > @@ -88,6 +104,12 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
> >  	if (err < 0)
> >  		goto failed;
> >  
> > +	if (IS_DGFX(xe)) {
> > +		err = resize_vf_vram_bar(xe, num_vfs);
> > +		if (err)
> > +			xe_sriov_info(xe, "Failed to set VF LMEM BAR size: %d\n", err);
> 
> If you intended this error to not result in failure, please mention it 
> in the changelog so that it's recorded somewhere for those who have to 
> look up things from the git history one day :-).

I'll rephrase the commit message into something like:

"Always try to set the largest possible BAR size that allows to fit the
number of enabled VFs and inform the user in case the resize attempt is
not succesfull."

> 
> > +	}
> > +
> >  	err = pci_enable_sriov(pdev, num_vfs);
> >  	if (err < 0)
> >  		goto failed;
> 
> Seems pretty straightforward after reading the support code on the PCI 
> core side,
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thanks,
-Michał

> 
> -- 
>  i.

Re: [PATCH v6 5/6] PCI: Allow drivers to control VF BAR size

[Ilpo Järvinen]

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

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

> On Wed, Mar 26, 2025 at 05:22:50PM +0200, Ilpo Järvinen wrote:
> > On Thu, 20 Mar 2025, Michał Winiarski wrote:
> > 
> > > Drivers could leverage the fact that the VF BAR MMIO reservation is
> > > created for total number of VFs supported by the device by resizing the
> > > BAR to larger size when smaller number of VFs is enabled.
> > > 
> > > Add a pci_iov_vf_bar_set_size() function to control the size and a
> > > pci_iov_vf_bar_get_sizes() helper to get the VF BAR sizes that will
> > > allow up to num_vfs to be successfully enabled with the current
> > > underlying reservation size.
> > > 
> > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>


> > > +/**
> > > + * pci_iov_vf_bar_get_sizes - get VF BAR sizes allowing to create up to num_vfs
> > > + * @dev: the PCI device
> > > + * @resno: the resource number
> > > + * @num_vfs: number of VFs
> > > + *
> > > + * Get the sizes of a VF resizable BAR that can be accommodated within the
> > > + * resource that reserves the MMIO space if num_vfs are enabled.
> > 
> > I'd rephrase to:
> > 
> > Get the sizes of a VF resizable BAR that can be accommodate @num_vfs 
> > within the currently assigned size of the resource @resno.
> 
> Ok.

I have small grammar error in that:

"can be accomodate" -> "can accomodate"

> > > + * defined in the spec (bit 0=1MB, bit 31=128TB).
> > > + */
> > > +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> > > +{
> > > +	resource_size_t size;
> > > +	u32 sizes;
> > > +	int i;
> > > +
> > > +	sizes = pci_rebar_get_possible_sizes(dev, resno);
> > > +	if (!sizes)
> > > +		return 0;
> > > +
> > > +	while (sizes > 0) {
> > > +		i = __fls(sizes);
> > > +		size = pci_rebar_size_to_bytes(i);
> > > +
> > > +		if (size * num_vfs <= pci_resource_len(dev, resno))
> > > +			break;
> > > +
> > > +		sizes &= ~BIT(i);
> > > +	}
> > 
> > Couldn't this be handled without a loop:
> > 
> > 	bar_sizes = (round_up(pci_resource_len(dev, resno) / num_vfs) - 1) >>
> > 		    ilog2(SZ_1M);
> > 
> > 	sizes &= bar_sizes;
> > 
> > (Just to given an idea, I wrote this into the email so it might contain 
> > some off-by-one errors or like).
> 
> I think the division will need to be wrapped with something like do_div
> (because IIUC, we have 32bit architectures where resource_size_t is
> u64).
> 
> But yeah, we can drop the loop, turning it into something like this:
> 
> 	vf_len = pci_resource_len(dev, resno);
> 	do_div(vf_len, num_vfs);
> 	sizes = (roundup_pow_of_two(vf_len + 1) - 1) >> ilog2(SZ_1M);

Yes, good point, 64-bit division is required.

-- 
 i.