Re: [PATCH 1/2] PCI: Add support for VF Resizable Bar extended cap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Am 15.12.21 um 15:16 schrieb Michał Winiarski:
Similar to regular resizable BAR, VF BAR can also be resized.
The structures are very similar, which means we can reuse most of the
implementation. See PCIe r4.0, sec 9.3.7.4.

Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
---
  drivers/pci/pci.c             | 25 +++++++++++++++--
  drivers/pci/setup-res.c       | 53 ++++++++++++++++++++++++++++++++---
  include/uapi/linux/pci_regs.h |  1 +
  3 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3d2fb394986a4..89448c5104e46 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1682,12 +1682,15 @@ static void pci_restore_config_space(struct pci_dev *pdev)
  	}
  }
-static void pci_restore_rebar_state(struct pci_dev *pdev)
+static void __pci_restore_rebar_state(struct pci_dev *pdev, int cap)
  {
  	unsigned int pos, nbars, i;
  	u32 ctrl;
- pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	if (WARN_ON(cap != PCI_EXT_CAP_ID_REBAR && cap != PCI_EXT_CAP_ID_VF_REBAR))
+		return;
+
+	pos = pci_find_ext_capability(pdev, cap);
  	if (!pos)
  		return;
@@ -1709,6 +1712,14 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
  	}
  }
+static void pci_restore_rebar_state(struct pci_dev *pdev)
+{
+	__pci_restore_rebar_state(pdev, PCI_EXT_CAP_ID_REBAR);
+#ifdef CONFIG_PCI_IOV
+	__pci_restore_rebar_state(pdev, PCI_EXT_CAP_ID_VF_REBAR);
+#endif
+}
+

It's probably cleaner to let the caller specify the capability to restore directly.

  /**
   * pci_restore_state - Restore the saved state of a PCI device
   * @dev: PCI device that we're dealing with
@@ -3639,10 +3650,18 @@ void pci_acs_init(struct pci_dev *dev)
   */
  static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
  {
+	int cap = PCI_EXT_CAP_ID_REBAR;
  	unsigned int pos, nbars, i;
  	u32 ctrl;
- pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+#ifdef CONFIG_PCI_IOV
+	if (bar >= PCI_IOV_RESOURCES) {
+		cap = PCI_EXT_CAP_ID_VF_REBAR;
+		bar -= PCI_IOV_RESOURCES;
+	}
+#endif
+
+	pos = pci_find_ext_capability(pdev, cap);
  	if (!pos)
  		return -ENOTSUPP;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 7f1acb3918d0c..1946e52e7678a 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -407,13 +407,36 @@ void pci_release_resource(struct pci_dev *dev, int resno)
  }
  EXPORT_SYMBOL(pci_release_resource);
+static int pci_memory_decoding(struct pci_dev *dev)
+{
+	u16 cmd;
+
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+	if (cmd & PCI_COMMAND_MEMORY)
+		return -EBUSY;
+
+	return 0;
+}
+
+#ifdef CONFIG_PCI_IOV
+static int pci_vf_memory_decoding(struct pci_dev *dev)
+{
+	u16 cmd;
+
+	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd);
+	if (cmd & PCI_SRIOV_CTRL_MSE)
+		return -EBUSY;
+
+	return 0;
+}
+#endif
+
  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
  {
  	struct resource *res = dev->resource + resno;
  	struct pci_host_bridge *host;
  	int old, ret;
  	u32 sizes;
-	u16 cmd;
/* Check if we must preserve the firmware's resource assignment */
  	host = pci_find_host_bridge(dev->bus);
@@ -424,9 +447,14 @@ 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)
-		return -EBUSY;
+#ifdef CONFIG_PCI_IOV
+	if (resno >= PCI_IOV_RESOURCES)
+		ret = pci_vf_memory_decoding(dev);
+	else
+#endif
+	ret = pci_memory_decoding(dev);
+	if (ret)
+		return ret;

Way to many #ifdef spread around inside the code, please restructure that.

For example concentrating the logic in a single function should help:

static int pci_check_decoding_disabled(..., inr resno)
{

#ifdef CONFIG_PCI_IOV
	if (resno...
		return -EBUSY;
	else
		return 0;
#endif

	if (...)
		return -EBUSY;
	return 0;

}

sizes = pci_rebar_get_possible_sizes(dev, resno);
  	if (!sizes)
@@ -445,6 +473,14 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
res->end = res->start + pci_rebar_size_to_bytes(size) - 1; +#ifdef CONFIG_PCI_IOV
+	if (resno >= PCI_IOV_RESOURCES) {
+		dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = pci_rebar_size_to_bytes(size);
+		res->end = res->start +
+			resource_size(res) * pci_sriov_get_totalvfs(dev) - 1;
+	}
+#endif
+
  	/* Check if the new config works by trying to assign everything. */
  	if (dev->bus->self) {
  		ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
@@ -456,6 +492,15 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
  error_resize:
  	pci_rebar_set_size(dev, resno, old);
  	res->end = res->start + pci_rebar_size_to_bytes(old) - 1;
+
+#ifdef CONFIG_PCI_IOV
+	if (resno >= PCI_IOV_RESOURCES) {
+		dev->sriov->barsz[resno - PCI_IOV_RESOURCES] = pci_rebar_size_to_bytes(old);
+		res->end = res->start +
+			pci_rebar_size_to_bytes(old) * pci_sriov_get_totalvfs(dev) - 1;
+	}
+#endif
+

Looks like this deserves it's own function.

Regards,
Christian.

  	return ret;
  }
  EXPORT_SYMBOL(pci_resize_resource);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ff6ccbc6efe96..7f5726d23b038 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -734,6 +734,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_MAX	PCI_EXT_CAP_ID_PL_16GT




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux