linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs
@ 2025-01-31 18:29 Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 1/7] " Niklas Cassel
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Jingoo Han,
	Heiko Stuebner, Kishon Vijay Abraham I
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci, linux-arm-kernel,
	linux-rockchip

The PCI endpoint framework currently does not support resizable BARs.

Add a new BAR type BAR_RESIZABLE, so that EPC drivers can support resizable
BARs properly.

For a resizable BAR, we will only allow a single supported size.
This is by design, as we do not need/want the complexity of the host side
resizing our resizable BAR.

In the DWC driver specifically, the DWC driver currently handles resizable
BARs using an ugly hack where a resizable BAR is force set to a fixed size
BAR with 1 MB size if detected. This is bogus, as a resizable BAR can be
configured to sizes other than 1 MB.

With these changes, an EPF driver will be able to call pci_epc_set_bar()
to configure a resizable BAR to an arbitrary size, just like for
BAR_PROGRAMMABLE. Thus, DWC based EPF drivers will no longer be forced to
a bogus 1 MB forced size for resizable BARs.


Tested/verified on a Radxa Rock 5b (rk3588) by:
-Modifying pci-epf-test.c to request BAR sizes that are larger than 1 MB:
 -static size_t bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
 +static size_t bar_size[] = { SZ_1M, SZ_1M, SZ_2M, SZ_2M, SZ_4M, SZ_4M };
 (Make sure to set CONFIG_CMA_ALIGNMENT=10 such that dma_alloc_coherent()
  calls are aligned even for allocations larger than 1 MB.)
-Rebooting the host to make sure that the DWC EP driver configures the BARs
 correctly after receiving a link down event.
-Modifying EPC features to configure a BAR as 64-bit, to make sure that we
 handle 64-bit BARs correctly.
-Modifying the DWC EP driver to set a size larger than 2 GB, to make sure
 we handle BAR sizes larger than 2 GB (for 64-bit BARs) correctly.
-Running the consecutive BAR test in pci_endpoint_test.c to make sure that
 the address translation works correctly.


Changes since V3:
-Picked up tags.
-Addressed comments from Mani.


Kind regards,
Niklas

Niklas Cassel (7):
  PCI: endpoint: Allow EPF drivers to configure the size of Resizable
    BARs
  PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability()
  PCI: dwc: endpoint: Allow EPF drivers to configure the size of
    Resizable BARs
  PCI: keystone: Describe Resizable BARs as Resizable BARs
  PCI: keystone: Specify correct alignment requirement
  PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs

 drivers/pci/controller/dwc/pci-keystone.c     |   6 +-
 .../pci/controller/dwc/pcie-designware-ep.c   | 218 +++++++++++++++---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c |  22 +-
 drivers/pci/endpoint/pci-epc-core.c           |  31 +++
 drivers/pci/endpoint/pci-epf-core.c           |   4 +
 include/linux/pci-epc.h                       |   5 +
 6 files changed, 239 insertions(+), 47 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 1/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-02-07 17:06   ` Manivannan Sadhasivam
  2025-01-31 18:29 ` [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap() Niklas Cassel
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci

A resizable BAR is different from a normal BAR in a few ways:
-The minimum size of a resizable BAR is 1 MB.
-Each BAR that is resizable has a Capability and Control register in the
 Resizable BAR Capability structure.

These registers contain the supported sizes and the currently selected
size of a resizable BAR.

The supported sizes is a bitmap of the supported sizes. The selected size
is a single value that is equal to one of the supported sizes.

A resizable BAR thus has to be configured differently than a
BAR_PROGRAMMABLE BAR, which usually sets the BAR size/mask in a vendor
specific way.

The PCI endpoint framework currently does not support resizable BARs.

Add a BAR type BAR_RESIZABLE, so that an EPC driver can support resizable
BARs properly.

Note that the pci_epc_set_bar() API takes a struct pci_epf_bar which tells
the EPC driver how it wants to configure the BAR.

struct pci_epf_bar only has a single size struct member.

This means that an EPC driver will only be able to set a single supported
size. This is perfectly fine, as we do not need the complexity of allowing
a host to change the size of the BAR. If someone ever wants to support
resizing a resizable BAR, the pci_epc_set_bar() API can be extended in the
future.

With these changes, we allow an EPF driver to configure the size of
Resizable BARs, rather than forcing them to a 1 MB size.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/endpoint/pci-epc-core.c | 4 ++++
 drivers/pci/endpoint/pci-epf-core.c | 4 ++++
 include/linux/pci-epc.h             | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 9e9ca5f8e8f8..10dfc716328e 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -609,6 +609,10 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	if (!epc_features)
 		return -EINVAL;
 
+	if (epc_features->bar[bar].type == BAR_RESIZABLE &&
+	    (epf_bar->size < SZ_1M || (u64)epf_bar->size > (SZ_128G * 1024)))
+		return -EINVAL;
+
 	if (epc_features->bar[bar].type == BAR_FIXED &&
 	    (epc_features->bar[bar].fixed_size != epf_bar->size))
 		return -EINVAL;
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 50bc2892a36c..394395c7f8de 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -274,6 +274,10 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	if (size < 128)
 		size = 128;
 
+	/* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
+	if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
+		size = SZ_1M;
+
 	if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
 		if (size > bar_fixed_size) {
 			dev_err(&epf->dev,
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index e818e3fdcded..91ce39dc0fd4 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -188,11 +188,15 @@ struct pci_epc {
  * enum pci_epc_bar_type - configurability of endpoint BAR
  * @BAR_PROGRAMMABLE: The BAR mask can be configured by the EPC.
  * @BAR_FIXED: The BAR mask is fixed by the hardware.
+ * @BAR_RESIZABLE: The BAR implements the PCI-SIG Resizable BAR Capability.
+ *		   NOTE: An EPC driver can currently only set a single supported
+ *		   size.
  * @BAR_RESERVED: The BAR should not be touched by an EPF driver.
  */
 enum pci_epc_bar_type {
 	BAR_PROGRAMMABLE = 0,
 	BAR_FIXED,
+	BAR_RESIZABLE,
 	BAR_RESERVED,
 };
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 1/7] " Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-02-07 17:11   ` Manivannan Sadhasivam
  2025-02-18 16:48   ` Bjorn Helgaas
  2025-01-31 18:29 ` [PATCH v4 3/7] PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability() Niklas Cassel
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci

Add a helper function to convert a size to the representation used by the
Resizable BAR Capability Register.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/endpoint/pci-epc-core.c | 27 +++++++++++++++++++++++++++
 include/linux/pci-epc.h             |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 10dfc716328e..5d6aef956b13 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -638,6 +638,33 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 }
 EXPORT_SYMBOL_GPL(pci_epc_set_bar);
 
+/**
+ * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
+ *				     by the Resizable BAR Capability Register
+ * @size: the size to convert
+ * @cap: where to store the result
+ *
+ * Returns 0 on success and a negative error code in case of error.
+ */
+int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
+{
+	/*
+	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
+	 * thus disallow a requested BAR size smaller than 1 MB.
+	 * Disallow a requested BAR size larger than 128 TB.
+	 */
+	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
+		return -EINVAL;
+
+	*cap = ilog2(size) - ilog2(SZ_1M);
+
+	/* Sizes in REBAR_CAP start at BIT(4). */
+	*cap = BIT(*cap + 4);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);
+
 /**
  * pci_epc_write_header() - write standard configuration header
  * @epc: the EPC device to which the configuration header should be written
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 91ce39dc0fd4..713348322dea 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -275,6 +275,7 @@ void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
 			enum pci_epc_interface_type type);
 int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 			 struct pci_epf_header *hdr);
+int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap);
 int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 		    struct pci_epf_bar *epf_bar);
 void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 3/7] PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability()
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 1/7] " Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap() Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci

Move dw_pcie_ep_find_ext_capability() so that it is located next to
dw_pcie_ep_find_capability().

Additionally, a follow-up commit requires this to be defined earlier
in order to avoid a forward declaration.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 36 +++++++++----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 8e07d432e74f..6b494781da42 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -102,6 +102,24 @@ static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
 	return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
 }
 
+static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
+{
+	u32 header;
+	int pos = PCI_CFG_SPACE_SIZE;
+
+	while (pos) {
+		header = dw_pcie_readl_dbi(pci, pos);
+		if (PCI_EXT_CAP_ID(header) == cap)
+			return pos;
+
+		pos = PCI_EXT_CAP_NEXT(header);
+		if (!pos)
+			break;
+	}
+
+	return 0;
+}
+
 static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 				   struct pci_epf_header *hdr)
 {
@@ -690,24 +708,6 @@ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
 
-static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
-{
-	u32 header;
-	int pos = PCI_CFG_SPACE_SIZE;
-
-	while (pos) {
-		header = dw_pcie_readl_dbi(pci, pos);
-		if (PCI_EXT_CAP_ID(header) == cap)
-			return pos;
-
-		pos = PCI_EXT_CAP_NEXT(header);
-		if (!pos)
-			break;
-	}
-
-	return 0;
-}
-
 static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
 {
 	unsigned int offset;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (2 preceding siblings ...)
  2025-01-31 18:29 ` [PATCH v4 3/7] PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability() Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-02-07 17:17   ` Manivannan Sadhasivam
  2025-01-31 18:29 ` [PATCH v4 5/7] PCI: keystone: Describe Resizable BARs as " Niklas Cassel
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci

The DWC databook specifies three different BARn_SIZING_SCHEME_N
- Fixed Mask (0)
- Programmable Mask (1)
- Resizable BAR (2)

Each of these sizing schemes have different instructions for how to
initialize the BAR.

The DWC driver currently does not support resizable BARs.

Instead, in order to somewhat support resizable BARs, the DWC EP driver
currently has an ugly hack that force sets a resizable BAR to 1 MB, if
such a BAR is detected.

Additionally, this hack only works if the DWC glue driver also has lied
in their EPC features, and claimed that the resizable BAR is a 1 MB fixed
size BAR.

This is unintuitive (as you somehow need to know that you need to lie in
your EPC features), but other than that it is overly restrictive, since a
resizable BAR is capable of supporting sizes different than 1 MB.

Add proper support for resizable BARs in the DWC EP driver.

Note that the pci_epc_set_bar() API takes a struct pci_epf_bar which tells
the EPC driver how it wants to configure the BAR.

struct pci_epf_bar only has a single size struct member.

This means that an EPC driver will only be able to set a single supported
size. This is perfectly fine, as we do not need the complexity of allowing
a host to change the size of the BAR. If someone ever wants to support
resizing a resizable BAR, the pci_epc_set_bar() API can be extended in the
future.

With these changes, we allow an EPF driver to configure the size of
Resizable BARs, rather than forcing them to a 1 MB size.

This means that an EPC driver does not need to lie in EPC features, and an
EPF driver will be able to set an arbitrary size (not be forced to a 1 MB
size), just like BAR_PROGRAMMABLE.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 182 ++++++++++++++++--
 1 file changed, 167 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 6b494781da42..72418160e658 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -223,6 +223,125 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	ep->bar_to_atu[bar] = 0;
 }
 
+static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie *pci,
+						enum pci_barno bar)
+{
+	u32 reg, bar_index;
+	unsigned int offset, nbars;
+	int i;
+
+	offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
+	if (!offset)
+		return offset;
+
+	reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
+	nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
+
+	for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) {
+		reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
+		bar_index = reg & PCI_REBAR_CTRL_BAR_IDX;
+		if (bar_index == bar)
+			return offset;
+	}
+
+	return 0;
+}
+
+static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no,
+					struct pci_epf_bar *epf_bar)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar = epf_bar->barno;
+	size_t size = epf_bar->size;
+	int flags = epf_bar->flags;
+	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+	unsigned int rebar_offset;
+	u32 rebar_cap, rebar_ctrl;
+	int ret;
+
+	rebar_offset = dw_pcie_ep_get_rebar_offset(pci, bar);
+	if (!rebar_offset)
+		return -EINVAL;
+
+	ret = pci_epc_bar_size_to_rebar_cap(size, &rebar_cap);
+	if (ret)
+		return ret;
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	/*
+	 * A BAR mask should not be written for a resizable BAR. The BAR mask
+	 * is automatically derived by the controller every time the "selected
+	 * size" bits are updated, see "Figure 3-26 Resizable BAR Example for
+	 * 32-bit Memory BAR0" in DWC EP databook 5.96a. We simply need to write
+	 * BIT(0) to set the BAR enable bit.
+	 */
+	dw_pcie_ep_writel_dbi2(ep, func_no, reg, BIT(0));
+	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
+
+	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0);
+		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
+	}
+
+	/*
+	 * Bits 31:0 in PCI_REBAR_CAP define "supported sizes" bits for sizes
+	 * 1 MB to 128 TB. Bits 31:16 in PCI_REBAR_CTRL define "supported sizes"
+	 * bits for sizes 256 TB to 8 EB. Disallow sizes 256 TB to 8 EB.
+	 */
+	rebar_ctrl = dw_pcie_readl_dbi(pci, rebar_offset + PCI_REBAR_CTRL);
+	rebar_ctrl &= ~GENMASK(31, 16);
+	dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl);
+
+	/*
+	 * The "selected size" (bits 13:8) in PCI_REBAR_CTRL are automatically
+	 * updated when writing PCI_REBAR_CAP, see "Figure 3-26 Resizable BAR
+	 * Example for 32-bit Memory BAR0" in DWC EP databook 5.96a.
+	 */
+	dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CAP, rebar_cap);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	return 0;
+}
+
+static int dw_pcie_ep_set_bar_programmable(struct dw_pcie_ep *ep, u8 func_no,
+					   struct pci_epf_bar *epf_bar)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar = epf_bar->barno;
+	size_t size = epf_bar->size;
+	int flags = epf_bar->flags;
+	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
+	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
+
+	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
+		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
+	}
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	return 0;
+}
+
+static enum pci_epc_bar_type dw_pcie_ep_get_bar_type(struct dw_pcie_ep *ep,
+						     enum pci_barno bar)
+{
+	const struct pci_epc_features *epc_features;
+
+	if (!ep->ops->get_features)
+		return BAR_PROGRAMMABLE;
+
+	epc_features = ep->ops->get_features(ep);
+
+	return epc_features->bar[bar].type;
+}
+
 static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 			      struct pci_epf_bar *epf_bar)
 {
@@ -230,9 +349,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	enum pci_barno bar = epf_bar->barno;
 	size_t size = epf_bar->size;
+	enum pci_epc_bar_type bar_type;
 	int flags = epf_bar->flags;
 	int ret, type;
-	u32 reg;
 
 	/*
 	 * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs
@@ -264,19 +383,30 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 		goto config_atu;
 	}
 
-	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
-
-	dw_pcie_dbi_ro_wr_en(pci);
-
-	dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
-	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
-
-	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
-		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
-		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
+	bar_type = dw_pcie_ep_get_bar_type(ep, bar);
+	switch (bar_type) {
+	case BAR_FIXED:
+		/*
+		 * There is no need to write a BAR mask for a fixed BAR (except
+		 * to write 1 to the LSB of the BAR mask register, to enable the
+		 * BAR). Write the BAR mask regardless. (The fixed bits in the
+		 * BAR mask register will be read-only anyway.)
+		 */
+		fallthrough;
+	case BAR_PROGRAMMABLE:
+		ret = dw_pcie_ep_set_bar_programmable(ep, func_no, epf_bar);
+		break;
+	case BAR_RESIZABLE:
+		ret = dw_pcie_ep_set_bar_resizable(ep, func_no, epf_bar);
+		break;
+	default:
+		ret = -EINVAL;
+		dev_err(pci->dev, "Invalid BAR type\n");
+		break;
 	}
 
-	dw_pcie_dbi_ro_wr_dis(pci);
+	if (ret)
+		return ret;
 
 config_atu:
 	if (!(flags & PCI_BASE_ADDRESS_SPACE))
@@ -710,9 +840,11 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
 
 static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
 {
+	struct dw_pcie_ep *ep = &pci->ep;
 	unsigned int offset;
 	unsigned int nbars;
-	u32 reg, i;
+	enum pci_barno bar;
+	u32 reg, i, val;
 
 	offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
 
@@ -727,9 +859,29 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
 		 * PCIe r6.0, sec 7.8.6.2 require us to support at least one
 		 * size in the range from 1 MB to 512 GB. Advertise support
 		 * for 1 MB BAR size only.
+		 *
+		 * For a BAR that has been configured via dw_pcie_ep_set_bar(),
+		 * advertise support for only that size instead.
 		 */
-		for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
-			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, BIT(4));
+		for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) {
+			/*
+			 * While the RESBAR_CAP_REG_* fields are sticky, the
+			 * RESBAR_CTRL_REG_BAR_SIZE field is non-sticky (it is
+			 * sticky in certain versions of DWC PCIe, but not all).
+			 *
+			 * RESBAR_CTRL_REG_BAR_SIZE is updated automatically by
+			 * the controller when RESBAR_CAP_REG is written, which
+			 * is why RESBAR_CAP_REG is written here.
+			 */
+			val = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
+			bar = val & PCI_REBAR_CTRL_BAR_IDX;
+			if (ep->epf_bar[bar])
+				pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val);
+			else
+				val = BIT(4);
+
+			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, val);
+		}
 	}
 
 	dw_pcie_setup(pci);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 5/7] PCI: keystone: Describe Resizable BARs as Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (3 preceding siblings ...)
  2025-01-31 18:29 ` [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-01-31 18:29 ` [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement Niklas Cassel
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci

Looking at "12.2.2.4.15 PCIe Subsystem BAR Configuration" in the
AM65x TRM:
https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf

We can see that BAR2 and BAR5 are not Fixed BARs, but actually Resizable
BARs.

Now when we actually have support for Resizable BARs, let's configure
these BARs as such.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 63bd5003da45..fdc610ec7e5e 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -966,10 +966,10 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
 	.msix_capable = true,
 	.bar[BAR_0] = { .type = BAR_RESERVED, },
 	.bar[BAR_1] = { .type = BAR_RESERVED, },
-	.bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
+	.bar[BAR_2] = { .type = BAR_RESIZABLE, },
 	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
 	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
-	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
+	.bar[BAR_5] = { .type = BAR_RESIZABLE, },
 	.align = SZ_1M,
 };
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (4 preceding siblings ...)
  2025-01-31 18:29 ` [PATCH v4 5/7] PCI: keystone: Describe Resizable BARs as " Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-02-07 17:18   ` Manivannan Sadhasivam
  2025-01-31 18:29 ` [PATCH v4 7/7] PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs Niklas Cassel
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Kishon Vijay Abraham I
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, stable+noautosel, linux-pci

The support for a specific iATU alignment was added in
commit 2a9a801620ef ("PCI: endpoint: Add support to specify alignment for
buffers allocated to BARs").

This commit specifically mentions both that the alignment by each DWC
based EP driver should match CX_ATU_MIN_REGION_SIZE, and that AM65x
specifically has a 64 KB alignment.

This also matches the CX_ATU_MIN_REGION_SIZE value specified by
"12.2.2.4.7 PCIe Subsystem Address Translation" in the AM65x TRM:
https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf

This higher value, 1 MB, was obviously an ugly hack used to be able to
handle Resizable BARs which have a minimum size of 1 MB.

Now when we actually have support for Resizable BARs, let's configure the
iATU alignment requirement to the actual requirement.
(BARs described as Resizable will still get aligned to 1 MB.)

Cc: stable+noautosel@kernel.org # Depends on PCI endpoint Resizable BARs series
Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pci-keystone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index fdc610ec7e5e..76a37368ae4f 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -970,7 +970,7 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
 	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
 	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
 	.bar[BAR_5] = { .type = BAR_RESIZABLE, },
-	.align = SZ_1M,
+	.align = SZ_64K,
 };
 
 static const struct pci_epc_features*
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 7/7] PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (5 preceding siblings ...)
  2025-01-31 18:29 ` [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement Niklas Cassel
@ 2025-01-31 18:29 ` Niklas Cassel
  2025-02-13 13:33 ` [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of " Niklas Cassel
  2025-02-14 17:40 ` Manivannan Sadhasivam
  8 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-01-31 18:29 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, Niklas Cassel, linux-pci, linux-arm-kernel,
	linux-rockchip

Looking at "11.4.4.29 USP_PCIE_RESBAR Registers Summary" in the rk3588 TRM,
we can see that none of the BARs are Fixed BARs, but actually Resizable
BARs.

I couldn't find any reference in the rk3568 TRM, but looking at the
downstream PCIe endpoint driver, rk3568 and rk3588 are treated as the same,
so the BARs on rk3568 must also be Resizable BARs.

Now when we actually have support for Resizable BARs, let's configure
these BARs as such.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 93698abff4d9..df2eaa35d045 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -273,12 +273,12 @@ static const struct pci_epc_features rockchip_pcie_epc_features_rk3568 = {
 	.msi_capable = true,
 	.msix_capable = true,
 	.align = SZ_64K,
-	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
+	.bar[BAR_0] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_1] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_2] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_3] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_4] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_5] = { .type = BAR_RESIZABLE, },
 };
 
 /*
@@ -293,12 +293,12 @@ static const struct pci_epc_features rockchip_pcie_epc_features_rk3588 = {
 	.msi_capable = true,
 	.msix_capable = true,
 	.align = SZ_64K,
-	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
-	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
+	.bar[BAR_0] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_1] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_2] = { .type = BAR_RESIZABLE, },
+	.bar[BAR_3] = { .type = BAR_RESIZABLE, },
 	.bar[BAR_4] = { .type = BAR_RESERVED, },
-	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
+	.bar[BAR_5] = { .type = BAR_RESIZABLE, },
 };
 
 static const struct pci_epc_features *
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 ` [PATCH v4 1/7] " Niklas Cassel
@ 2025-02-07 17:06   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-07 17:06 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
	Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, linux-pci

On Fri, Jan 31, 2025 at 07:29:50PM +0100, Niklas Cassel wrote:
> A resizable BAR is different from a normal BAR in a few ways:
> -The minimum size of a resizable BAR is 1 MB.
> -Each BAR that is resizable has a Capability and Control register in the
>  Resizable BAR Capability structure.
> 
> These registers contain the supported sizes and the currently selected
> size of a resizable BAR.
> 
> The supported sizes is a bitmap of the supported sizes. The selected size
> is a single value that is equal to one of the supported sizes.
> 
> A resizable BAR thus has to be configured differently than a
> BAR_PROGRAMMABLE BAR, which usually sets the BAR size/mask in a vendor
> specific way.
> 
> The PCI endpoint framework currently does not support resizable BARs.
> 
> Add a BAR type BAR_RESIZABLE, so that an EPC driver can support resizable
> BARs properly.
> 
> Note that the pci_epc_set_bar() API takes a struct pci_epf_bar which tells
> the EPC driver how it wants to configure the BAR.
> 
> struct pci_epf_bar only has a single size struct member.
> 
> This means that an EPC driver will only be able to set a single supported
> size. This is perfectly fine, as we do not need the complexity of allowing
> a host to change the size of the BAR. If someone ever wants to support
> resizing a resizable BAR, the pci_epc_set_bar() API can be extended in the
> future.
> 
> With these changes, we allow an EPF driver to configure the size of
> Resizable BARs, rather than forcing them to a 1 MB size.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/endpoint/pci-epc-core.c | 4 ++++
>  drivers/pci/endpoint/pci-epf-core.c | 4 ++++
>  include/linux/pci-epc.h             | 4 ++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 9e9ca5f8e8f8..10dfc716328e 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -609,6 +609,10 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	if (!epc_features)
>  		return -EINVAL;
>  
> +	if (epc_features->bar[bar].type == BAR_RESIZABLE &&
> +	    (epf_bar->size < SZ_1M || (u64)epf_bar->size > (SZ_128G * 1024)))
> +		return -EINVAL;
> +
>  	if (epc_features->bar[bar].type == BAR_FIXED &&
>  	    (epc_features->bar[bar].fixed_size != epf_bar->size))
>  		return -EINVAL;
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 50bc2892a36c..394395c7f8de 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -274,6 +274,10 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>  	if (size < 128)
>  		size = 128;
>  
> +	/* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
> +	if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
> +		size = SZ_1M;
> +
>  	if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
>  		if (size > bar_fixed_size) {
>  			dev_err(&epf->dev,
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index e818e3fdcded..91ce39dc0fd4 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -188,11 +188,15 @@ struct pci_epc {
>   * enum pci_epc_bar_type - configurability of endpoint BAR
>   * @BAR_PROGRAMMABLE: The BAR mask can be configured by the EPC.
>   * @BAR_FIXED: The BAR mask is fixed by the hardware.
> + * @BAR_RESIZABLE: The BAR implements the PCI-SIG Resizable BAR Capability.
> + *		   NOTE: An EPC driver can currently only set a single supported
> + *		   size.
>   * @BAR_RESERVED: The BAR should not be touched by an EPF driver.
>   */
>  enum pci_epc_bar_type {
>  	BAR_PROGRAMMABLE = 0,
>  	BAR_FIXED,
> +	BAR_RESIZABLE,
>  	BAR_RESERVED,
>  };
>  
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-01-31 18:29 ` [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap() Niklas Cassel
@ 2025-02-07 17:11   ` Manivannan Sadhasivam
  2025-02-18 16:48   ` Bjorn Helgaas
  1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-07 17:11 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
	Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, linux-pci

On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> Add a helper function to convert a size to the representation used by the
> Resizable BAR Capability Register.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

One nit below.

> ---
>  drivers/pci/endpoint/pci-epc-core.c | 27 +++++++++++++++++++++++++++
>  include/linux/pci-epc.h             |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 10dfc716328e..5d6aef956b13 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -638,6 +638,33 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_set_bar);
>  
> +/**
> + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> + *				     by the Resizable BAR Capability Register
> + * @size: the size to convert
> + * @cap: where to store the result
> + *
> + * Returns 0 on success and a negative error code in case of error.
> + */
> +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> +{
> +	/*
> +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> +	 * thus disallow a requested BAR size smaller than 1 MB.
> +	 * Disallow a requested BAR size larger than 128 TB.
> +	 */
> +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))

You could've used (SZ_1T * 128). But no need to respin just for this.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 ` [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
@ 2025-02-07 17:17   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-07 17:17 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Damien Le Moal, Siddharth Vadapalli,
	Udit Kumar, Vignesh Raghavendra, linux-pci

On Fri, Jan 31, 2025 at 07:29:53PM +0100, Niklas Cassel wrote:
> The DWC databook specifies three different BARn_SIZING_SCHEME_N
> - Fixed Mask (0)
> - Programmable Mask (1)
> - Resizable BAR (2)
> 
> Each of these sizing schemes have different instructions for how to
> initialize the BAR.
> 
> The DWC driver currently does not support resizable BARs.
> 
> Instead, in order to somewhat support resizable BARs, the DWC EP driver
> currently has an ugly hack that force sets a resizable BAR to 1 MB, if
> such a BAR is detected.
> 
> Additionally, this hack only works if the DWC glue driver also has lied
> in their EPC features, and claimed that the resizable BAR is a 1 MB fixed
> size BAR.
> 
> This is unintuitive (as you somehow need to know that you need to lie in
> your EPC features), but other than that it is overly restrictive, since a
> resizable BAR is capable of supporting sizes different than 1 MB.
> 
> Add proper support for resizable BARs in the DWC EP driver.
> 
> Note that the pci_epc_set_bar() API takes a struct pci_epf_bar which tells
> the EPC driver how it wants to configure the BAR.
> 
> struct pci_epf_bar only has a single size struct member.
> 
> This means that an EPC driver will only be able to set a single supported
> size. This is perfectly fine, as we do not need the complexity of allowing
> a host to change the size of the BAR. If someone ever wants to support
> resizing a resizable BAR, the pci_epc_set_bar() API can be extended in the
> future.
> 
> With these changes, we allow an EPF driver to configure the size of
> Resizable BARs, rather than forcing them to a 1 MB size.
> 
> This means that an EPC driver does not need to lie in EPC features, and an
> EPF driver will be able to set an arbitrary size (not be forced to a 1 MB
> size), just like BAR_PROGRAMMABLE.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  .../pci/controller/dwc/pcie-designware-ep.c   | 182 ++++++++++++++++--
>  1 file changed, 167 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 6b494781da42..72418160e658 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -223,6 +223,125 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	ep->bar_to_atu[bar] = 0;
>  }
>  
> +static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie *pci,
> +						enum pci_barno bar)
> +{
> +	u32 reg, bar_index;
> +	unsigned int offset, nbars;
> +	int i;
> +
> +	offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> +	if (!offset)
> +		return offset;
> +
> +	reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> +	nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> +	for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) {
> +		reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> +		bar_index = reg & PCI_REBAR_CTRL_BAR_IDX;
> +		if (bar_index == bar)
> +			return offset;
> +	}
> +
> +	return 0;
> +}
> +
> +static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no,
> +					struct pci_epf_bar *epf_bar)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar = epf_bar->barno;
> +	size_t size = epf_bar->size;
> +	int flags = epf_bar->flags;
> +	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> +	unsigned int rebar_offset;
> +	u32 rebar_cap, rebar_ctrl;
> +	int ret;
> +
> +	rebar_offset = dw_pcie_ep_get_rebar_offset(pci, bar);
> +	if (!rebar_offset)
> +		return -EINVAL;
> +
> +	ret = pci_epc_bar_size_to_rebar_cap(size, &rebar_cap);
> +	if (ret)
> +		return ret;
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +
> +	/*
> +	 * A BAR mask should not be written for a resizable BAR. The BAR mask
> +	 * is automatically derived by the controller every time the "selected
> +	 * size" bits are updated, see "Figure 3-26 Resizable BAR Example for
> +	 * 32-bit Memory BAR0" in DWC EP databook 5.96a. We simply need to write
> +	 * BIT(0) to set the BAR enable bit.
> +	 */
> +	dw_pcie_ep_writel_dbi2(ep, func_no, reg, BIT(0));
> +	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
> +
> +	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0);
> +		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> +	}
> +
> +	/*
> +	 * Bits 31:0 in PCI_REBAR_CAP define "supported sizes" bits for sizes
> +	 * 1 MB to 128 TB. Bits 31:16 in PCI_REBAR_CTRL define "supported sizes"
> +	 * bits for sizes 256 TB to 8 EB. Disallow sizes 256 TB to 8 EB.
> +	 */
> +	rebar_ctrl = dw_pcie_readl_dbi(pci, rebar_offset + PCI_REBAR_CTRL);
> +	rebar_ctrl &= ~GENMASK(31, 16);
> +	dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl);
> +
> +	/*
> +	 * The "selected size" (bits 13:8) in PCI_REBAR_CTRL are automatically
> +	 * updated when writing PCI_REBAR_CAP, see "Figure 3-26 Resizable BAR
> +	 * Example for 32-bit Memory BAR0" in DWC EP databook 5.96a.
> +	 */
> +	dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CAP, rebar_cap);
> +
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
> +	return 0;
> +}
> +
> +static int dw_pcie_ep_set_bar_programmable(struct dw_pcie_ep *ep, u8 func_no,
> +					   struct pci_epf_bar *epf_bar)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar = epf_bar->barno;
> +	size_t size = epf_bar->size;
> +	int flags = epf_bar->flags;
> +	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +
> +	dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
> +	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
> +
> +	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
> +		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> +	}
> +
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
> +	return 0;
> +}
> +
> +static enum pci_epc_bar_type dw_pcie_ep_get_bar_type(struct dw_pcie_ep *ep,
> +						     enum pci_barno bar)
> +{
> +	const struct pci_epc_features *epc_features;
> +
> +	if (!ep->ops->get_features)
> +		return BAR_PROGRAMMABLE;
> +
> +	epc_features = ep->ops->get_features(ep);
> +
> +	return epc_features->bar[bar].type;
> +}
> +
>  static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  			      struct pci_epf_bar *epf_bar)
>  {
> @@ -230,9 +349,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	enum pci_barno bar = epf_bar->barno;
>  	size_t size = epf_bar->size;
> +	enum pci_epc_bar_type bar_type;
>  	int flags = epf_bar->flags;
>  	int ret, type;
> -	u32 reg;
>  
>  	/*
>  	 * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs
> @@ -264,19 +383,30 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  		goto config_atu;
>  	}
>  
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> -
> -	dw_pcie_dbi_ro_wr_en(pci);
> -
> -	dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
> -	dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
> -
> -	if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> -		dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
> -		dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> +	bar_type = dw_pcie_ep_get_bar_type(ep, bar);
> +	switch (bar_type) {
> +	case BAR_FIXED:
> +		/*
> +		 * There is no need to write a BAR mask for a fixed BAR (except
> +		 * to write 1 to the LSB of the BAR mask register, to enable the
> +		 * BAR). Write the BAR mask regardless. (The fixed bits in the
> +		 * BAR mask register will be read-only anyway.)
> +		 */
> +		fallthrough;
> +	case BAR_PROGRAMMABLE:
> +		ret = dw_pcie_ep_set_bar_programmable(ep, func_no, epf_bar);
> +		break;
> +	case BAR_RESIZABLE:
> +		ret = dw_pcie_ep_set_bar_resizable(ep, func_no, epf_bar);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		dev_err(pci->dev, "Invalid BAR type\n");
> +		break;
>  	}
>  
> -	dw_pcie_dbi_ro_wr_dis(pci);
> +	if (ret)
> +		return ret;
>  
>  config_atu:
>  	if (!(flags & PCI_BASE_ADDRESS_SPACE))
> @@ -710,9 +840,11 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
>  
>  static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
>  {
> +	struct dw_pcie_ep *ep = &pci->ep;
>  	unsigned int offset;
>  	unsigned int nbars;
> -	u32 reg, i;
> +	enum pci_barno bar;
> +	u32 reg, i, val;
>  
>  	offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>  
> @@ -727,9 +859,29 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
>  		 * PCIe r6.0, sec 7.8.6.2 require us to support at least one
>  		 * size in the range from 1 MB to 512 GB. Advertise support
>  		 * for 1 MB BAR size only.
> +		 *
> +		 * For a BAR that has been configured via dw_pcie_ep_set_bar(),
> +		 * advertise support for only that size instead.
>  		 */
> -		for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> -			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, BIT(4));
> +		for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) {
> +			/*
> +			 * While the RESBAR_CAP_REG_* fields are sticky, the
> +			 * RESBAR_CTRL_REG_BAR_SIZE field is non-sticky (it is
> +			 * sticky in certain versions of DWC PCIe, but not all).
> +			 *
> +			 * RESBAR_CTRL_REG_BAR_SIZE is updated automatically by
> +			 * the controller when RESBAR_CAP_REG is written, which
> +			 * is why RESBAR_CAP_REG is written here.
> +			 */
> +			val = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> +			bar = val & PCI_REBAR_CTRL_BAR_IDX;
> +			if (ep->epf_bar[bar])
> +				pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val);
> +			else
> +				val = BIT(4);
> +
> +			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, val);
> +		}
>  	}
>  
>  	dw_pcie_setup(pci);
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement
  2025-01-31 18:29 ` [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement Niklas Cassel
@ 2025-02-07 17:18   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-07 17:18 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Kishon Vijay Abraham I, Damien Le Moal,
	Siddharth Vadapalli, Udit Kumar, Vignesh Raghavendra,
	stable+noautosel, linux-pci

On Fri, Jan 31, 2025 at 07:29:55PM +0100, Niklas Cassel wrote:
> The support for a specific iATU alignment was added in
> commit 2a9a801620ef ("PCI: endpoint: Add support to specify alignment for
> buffers allocated to BARs").
> 
> This commit specifically mentions both that the alignment by each DWC
> based EP driver should match CX_ATU_MIN_REGION_SIZE, and that AM65x
> specifically has a 64 KB alignment.
> 
> This also matches the CX_ATU_MIN_REGION_SIZE value specified by
> "12.2.2.4.7 PCIe Subsystem Address Translation" in the AM65x TRM:
> https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf
> 
> This higher value, 1 MB, was obviously an ugly hack used to be able to
> handle Resizable BARs which have a minimum size of 1 MB.
> 
> Now when we actually have support for Resizable BARs, let's configure the
> iATU alignment requirement to the actual requirement.
> (BARs described as Resizable will still get aligned to 1 MB.)
> 
> Cc: stable+noautosel@kernel.org # Depends on PCI endpoint Resizable BARs series
> Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index fdc610ec7e5e..76a37368ae4f 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -970,7 +970,7 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
>  	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
>  	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
>  	.bar[BAR_5] = { .type = BAR_RESIZABLE, },
> -	.align = SZ_1M,
> +	.align = SZ_64K,
>  };
>  
>  static const struct pci_epc_features*
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (6 preceding siblings ...)
  2025-01-31 18:29 ` [PATCH v4 7/7] PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs Niklas Cassel
@ 2025-02-13 13:33 ` Niklas Cassel
  2025-02-14 17:40 ` Manivannan Sadhasivam
  8 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-02-13 13:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Jingoo Han,
	Heiko Stuebner, Kishon Vijay Abraham I
  Cc: Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, linux-pci, linux-arm-kernel, linux-rockchip

On Fri, Jan 31, 2025 at 07:29:49PM +0100, Niklas Cassel wrote:
> The PCI endpoint framework currently does not support resizable BARs.
> 
> Add a new BAR type BAR_RESIZABLE, so that EPC drivers can support resizable
> BARs properly.
> 
> For a resizable BAR, we will only allow a single supported size.
> This is by design, as we do not need/want the complexity of the host side
> resizing our resizable BAR.
> 
> In the DWC driver specifically, the DWC driver currently handles resizable
> BARs using an ugly hack where a resizable BAR is force set to a fixed size
> BAR with 1 MB size if detected. This is bogus, as a resizable BAR can be
> configured to sizes other than 1 MB.
> 
> With these changes, an EPF driver will be able to call pci_epc_set_bar()
> to configure a resizable BAR to an arbitrary size, just like for
> BAR_PROGRAMMABLE. Thus, DWC based EPF drivers will no longer be forced to
> a bogus 1 MB forced size for resizable BARs.
> 
> 
> Tested/verified on a Radxa Rock 5b (rk3588) by:
> -Modifying pci-epf-test.c to request BAR sizes that are larger than 1 MB:
>  -static size_t bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
>  +static size_t bar_size[] = { SZ_1M, SZ_1M, SZ_2M, SZ_2M, SZ_4M, SZ_4M };
>  (Make sure to set CONFIG_CMA_ALIGNMENT=10 such that dma_alloc_coherent()
>   calls are aligned even for allocations larger than 1 MB.)
> -Rebooting the host to make sure that the DWC EP driver configures the BARs
>  correctly after receiving a link down event.
> -Modifying EPC features to configure a BAR as 64-bit, to make sure that we
>  handle 64-bit BARs correctly.
> -Modifying the DWC EP driver to set a size larger than 2 GB, to make sure
>  we handle BAR sizes larger than 2 GB (for 64-bit BARs) correctly.
> -Running the consecutive BAR test in pci_endpoint_test.c to make sure that
>  the address translation works correctly.
> 
> 
> Changes since V3:
> -Picked up tags.
> -Addressed comments from Mani.
> 
> 
> Kind regards,
> Niklas
> 
> Niklas Cassel (7):
>   PCI: endpoint: Allow EPF drivers to configure the size of Resizable
>     BARs
>   PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
>   PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability()
>   PCI: dwc: endpoint: Allow EPF drivers to configure the size of
>     Resizable BARs
>   PCI: keystone: Describe Resizable BARs as Resizable BARs
>   PCI: keystone: Specify correct alignment requirement
>   PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs
> 
>  drivers/pci/controller/dwc/pci-keystone.c     |   6 +-
>  .../pci/controller/dwc/pcie-designware-ep.c   | 218 +++++++++++++++---
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c |  22 +-
>  drivers/pci/endpoint/pci-epc-core.c           |  31 +++
>  drivers/pci/endpoint/pci-epf-core.c           |   4 +
>  include/linux/pci-epc.h                       |   5 +
>  6 files changed, 239 insertions(+), 47 deletions(-)
> 
> -- 
> 2.48.1
> 

Considering that all patches are have at least one R-b tag,
any chance this series could get picked up?


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs
  2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
                   ` (7 preceding siblings ...)
  2025-02-13 13:33 ` [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of " Niklas Cassel
@ 2025-02-14 17:40 ` Manivannan Sadhasivam
  8 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-14 17:40 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Jingoo Han, Heiko Stuebner, Kishon Vijay Abraham I,
	Damien Le Moal, Siddharth Vadapalli, Udit Kumar,
	Vignesh Raghavendra, linux-pci, linux-arm-kernel, linux-rockchip

On Fri, Jan 31, 2025 at 07:29:49PM +0100, Niklas Cassel wrote:
> The PCI endpoint framework currently does not support resizable BARs.
> 
> Add a new BAR type BAR_RESIZABLE, so that EPC drivers can support resizable
> BARs properly.
> 
> For a resizable BAR, we will only allow a single supported size.
> This is by design, as we do not need/want the complexity of the host side
> resizing our resizable BAR.
> 
> In the DWC driver specifically, the DWC driver currently handles resizable
> BARs using an ugly hack where a resizable BAR is force set to a fixed size
> BAR with 1 MB size if detected. This is bogus, as a resizable BAR can be
> configured to sizes other than 1 MB.
> 
> With these changes, an EPF driver will be able to call pci_epc_set_bar()
> to configure a resizable BAR to an arbitrary size, just like for
> BAR_PROGRAMMABLE. Thus, DWC based EPF drivers will no longer be forced to
> a bogus 1 MB forced size for resizable BARs.
> 
> 
> Tested/verified on a Radxa Rock 5b (rk3588) by:
> -Modifying pci-epf-test.c to request BAR sizes that are larger than 1 MB:
>  -static size_t bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
>  +static size_t bar_size[] = { SZ_1M, SZ_1M, SZ_2M, SZ_2M, SZ_4M, SZ_4M };
>  (Make sure to set CONFIG_CMA_ALIGNMENT=10 such that dma_alloc_coherent()
>   calls are aligned even for allocations larger than 1 MB.)
> -Rebooting the host to make sure that the DWC EP driver configures the BARs
>  correctly after receiving a link down event.
> -Modifying EPC features to configure a BAR as 64-bit, to make sure that we
>  handle 64-bit BARs correctly.
> -Modifying the DWC EP driver to set a size larger than 2 GB, to make sure
>  we handle BAR sizes larger than 2 GB (for 64-bit BARs) correctly.
> -Running the consecutive BAR test in pci_endpoint_test.c to make sure that
>  the address translation works correctly.
> 

I took the liberty since this series is mostly related to endpoint and applied
to pci/endpoint!

- Mani

> 
> Changes since V3:
> -Picked up tags.
> -Addressed comments from Mani.
> 
> 
> Kind regards,
> Niklas
> 
> Niklas Cassel (7):
>   PCI: endpoint: Allow EPF drivers to configure the size of Resizable
>     BARs
>   PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
>   PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability()
>   PCI: dwc: endpoint: Allow EPF drivers to configure the size of
>     Resizable BARs
>   PCI: keystone: Describe Resizable BARs as Resizable BARs
>   PCI: keystone: Specify correct alignment requirement
>   PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs
> 
>  drivers/pci/controller/dwc/pci-keystone.c     |   6 +-
>  .../pci/controller/dwc/pcie-designware-ep.c   | 218 +++++++++++++++---
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c |  22 +-
>  drivers/pci/endpoint/pci-epc-core.c           |  31 +++
>  drivers/pci/endpoint/pci-epf-core.c           |   4 +
>  include/linux/pci-epc.h                       |   5 +
>  6 files changed, 239 insertions(+), 47 deletions(-)
> 
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-01-31 18:29 ` [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap() Niklas Cassel
  2025-02-07 17:11   ` Manivannan Sadhasivam
@ 2025-02-18 16:48   ` Bjorn Helgaas
  2025-02-19 17:24     ` Niklas Cassel
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2025-02-18 16:48 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Damien Le Moal,
	Siddharth Vadapalli, Udit Kumar, Vignesh Raghavendra, linux-pci

On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> Add a helper function to convert a size to the representation used by the
> Resizable BAR Capability Register.

> +/**
> + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> + *				     by the Resizable BAR Capability Register
> + * @size: the size to convert
> + * @cap: where to store the result
> + *
> + * Returns 0 on success and a negative error code in case of error.
> + */
> +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> +{
> +	/*
> +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> +	 * thus disallow a requested BAR size smaller than 1 MB.
> +	 * Disallow a requested BAR size larger than 128 TB.
> +	 */
> +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
> +		return -EINVAL;
> +
> +	*cap = ilog2(size) - ilog2(SZ_1M);
> +
> +	/* Sizes in REBAR_CAP start at BIT(4). */
> +	*cap = BIT(*cap + 4);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);

This doesn't seem specific to EPC.  It looks basically similar to
pci_rebar_bytes_to_size().  Can we consolidate anything there?

Since we're basing this on the spec, we should include a complete
citation, e.g., PCIe r6.0, sec xxx.

Bjorn

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-02-18 16:48   ` Bjorn Helgaas
@ 2025-02-19 17:24     ` Niklas Cassel
  2025-02-19 17:49       ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2025-02-19 17:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Damien Le Moal,
	Siddharth Vadapalli, Udit Kumar, Vignesh Raghavendra, linux-pci

On Tue, Feb 18, 2025 at 10:48:04AM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> > Add a helper function to convert a size to the representation used by the
> > Resizable BAR Capability Register.
> 
> > +/**
> > + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> > + *				     by the Resizable BAR Capability Register
> > + * @size: the size to convert
> > + * @cap: where to store the result
> > + *
> > + * Returns 0 on success and a negative error code in case of error.
> > + */
> > +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> > +{
> > +	/*
> > +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> > +	 * thus disallow a requested BAR size smaller than 1 MB.
> > +	 * Disallow a requested BAR size larger than 128 TB.
> > +	 */
> > +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
> > +		return -EINVAL;
> > +
> > +	*cap = ilog2(size) - ilog2(SZ_1M);
> > +
> > +	/* Sizes in REBAR_CAP start at BIT(4). */
> > +	*cap = BIT(*cap + 4);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);
> 
> This doesn't seem specific to EPC.  It looks basically similar to
> pci_rebar_bytes_to_size().  Can we consolidate anything there?

This function is to convert a size to
"Resizable BAR Capability Register", which includes one or more supported
BAR sizes.

This register should only ever by written by a PCI endpoint function,
so I think its current home in drivers/pci/endpoint/ is correct.


pci_rebar_bytes_to_size() is used to convert a size to
"Resizable BAR Control Register", specifically the field
"BAR Size".

This "BAR Size" field in the "Resizable BAR Control Register" can be
written by the host side (or endpoint side), to select one of the
supported bar sizes. So I think it makes sense for
pci_rebar_bytes_to_size() to live in pci.h.


> 
> Since we're basing this on the spec, we should include a complete
> citation, e.g., PCIe r6.0, sec xxx.

I agree:
https://lore.kernel.org/linux-pci/20250219171454.2903059-2-cassel@kernel.org/T/#u


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-02-19 17:24     ` Niklas Cassel
@ 2025-02-19 17:49       ` Bjorn Helgaas
  2025-02-19 18:00         ` Niklas Cassel
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2025-02-19 17:49 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Damien Le Moal,
	Siddharth Vadapalli, Udit Kumar, Vignesh Raghavendra, linux-pci

On Wed, Feb 19, 2025 at 06:24:10PM +0100, Niklas Cassel wrote:
> On Tue, Feb 18, 2025 at 10:48:04AM -0600, Bjorn Helgaas wrote:
> > On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> > > Add a helper function to convert a size to the representation used by the
> > > Resizable BAR Capability Register.
> > 
> > > +/**
> > > + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> > > + *				     by the Resizable BAR Capability Register
> > > + * @size: the size to convert
> > > + * @cap: where to store the result
> > > + *
> > > + * Returns 0 on success and a negative error code in case of error.
> > > + */
> > > +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> > > +{
> > > +	/*
> > > +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> > > +	 * thus disallow a requested BAR size smaller than 1 MB.
> > > +	 * Disallow a requested BAR size larger than 128 TB.
> > > +	 */
> > > +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
> > > +		return -EINVAL;
> > > +
> > > +	*cap = ilog2(size) - ilog2(SZ_1M);
> > > +
> > > +	/* Sizes in REBAR_CAP start at BIT(4). */
> > > +	*cap = BIT(*cap + 4);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);
> > 
> > This doesn't seem specific to EPC.  It looks basically similar to
> > pci_rebar_bytes_to_size().  Can we consolidate anything there?
> 
> This function is to convert a size to
> "Resizable BAR Capability Register", which includes one or more supported
> BAR sizes.
> 
> This register should only ever by written by a PCI endpoint function,
> so I think its current home in drivers/pci/endpoint/ is correct.
> 
> pci_rebar_bytes_to_size() is used to convert a size to
> "Resizable BAR Control Register", specifically the field
> "BAR Size".
> 
> This "BAR Size" field in the "Resizable BAR Control Register" can be
> written by the host side (or endpoint side), to select one of the
> supported bar sizes. So I think it makes sense for
> pci_rebar_bytes_to_size() to live in pci.h.

Thanks, I agree.

It looks like pci_epc_bar_size_to_rebar_cap() is only called once per
BAR.  Does that mean an endpoint driver can only set a single
supported size for a Resizable BAR in the Capability register?

Bjorn

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()
  2025-02-19 17:49       ` Bjorn Helgaas
@ 2025-02-19 18:00         ` Niklas Cassel
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2025-02-19 18:00 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Damien Le Moal,
	Siddharth Vadapalli, Udit Kumar, Vignesh Raghavendra, linux-pci

On Wed, Feb 19, 2025 at 11:49:27AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 19, 2025 at 06:24:10PM +0100, Niklas Cassel wrote:
> > On Tue, Feb 18, 2025 at 10:48:04AM -0600, Bjorn Helgaas wrote:
> > > On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> > > > Add a helper function to convert a size to the representation used by the
> > > > Resizable BAR Capability Register.
> > > 
> > > > +/**
> > > > + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> > > > + *				     by the Resizable BAR Capability Register
> > > > + * @size: the size to convert
> > > > + * @cap: where to store the result
> > > > + *
> > > > + * Returns 0 on success and a negative error code in case of error.
> > > > + */
> > > > +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> > > > +{
> > > > +	/*
> > > > +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> > > > +	 * thus disallow a requested BAR size smaller than 1 MB.
> > > > +	 * Disallow a requested BAR size larger than 128 TB.
> > > > +	 */
> > > > +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
> > > > +		return -EINVAL;
> > > > +
> > > > +	*cap = ilog2(size) - ilog2(SZ_1M);
> > > > +
> > > > +	/* Sizes in REBAR_CAP start at BIT(4). */
> > > > +	*cap = BIT(*cap + 4);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);
> > > 
> > > This doesn't seem specific to EPC.  It looks basically similar to
> > > pci_rebar_bytes_to_size().  Can we consolidate anything there?
> > 
> > This function is to convert a size to
> > "Resizable BAR Capability Register", which includes one or more supported
> > BAR sizes.
> > 
> > This register should only ever by written by a PCI endpoint function,
> > so I think its current home in drivers/pci/endpoint/ is correct.
> > 
> > pci_rebar_bytes_to_size() is used to convert a size to
> > "Resizable BAR Control Register", specifically the field
> > "BAR Size".
> > 
> > This "BAR Size" field in the "Resizable BAR Control Register" can be
> > written by the host side (or endpoint side), to select one of the
> > supported bar sizes. So I think it makes sense for
> > pci_rebar_bytes_to_size() to live in pci.h.
> 
> Thanks, I agree.
> 
> It looks like pci_epc_bar_size_to_rebar_cap() is only called once per
> BAR.  Does that mean an endpoint driver can only set a single
> supported size for a Resizable BAR in the Capability register?

Yes, that is the current design.

(Which is an improvement to before this series was merged, where a Resizable
BAR had to be forced to 1 MB size, so EPF drivers could not configure a size
larger/different than 1 MB.)



It is possible to allow the endpoint framework to allow multiple Resizable
BAR sizes to be supported in the future.

On e.g. rk3588, the EP gets an IRQ when the host side writes the "BAR Size"
field in the "Resizable BAR Control Register".

So one can image that the drivers could call some function in the EPC
framework that allocates + copies + frees new backing memory for the BARs.

But right now, I don't have a need for such a use-case, so let's defer that
until someone actually wants/needs that, since it will undoubtedly also add
more code complexity to the EPC framework.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-02-19 18:00 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-31 18:29 [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
2025-01-31 18:29 ` [PATCH v4 1/7] " Niklas Cassel
2025-02-07 17:06   ` Manivannan Sadhasivam
2025-01-31 18:29 ` [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap() Niklas Cassel
2025-02-07 17:11   ` Manivannan Sadhasivam
2025-02-18 16:48   ` Bjorn Helgaas
2025-02-19 17:24     ` Niklas Cassel
2025-02-19 17:49       ` Bjorn Helgaas
2025-02-19 18:00         ` Niklas Cassel
2025-01-31 18:29 ` [PATCH v4 3/7] PCI: dwc: ep: Move dw_pcie_ep_find_ext_capability() Niklas Cassel
2025-01-31 18:29 ` [PATCH v4 4/7] PCI: dwc: endpoint: Allow EPF drivers to configure the size of Resizable BARs Niklas Cassel
2025-02-07 17:17   ` Manivannan Sadhasivam
2025-01-31 18:29 ` [PATCH v4 5/7] PCI: keystone: Describe Resizable BARs as " Niklas Cassel
2025-01-31 18:29 ` [PATCH v4 6/7] PCI: keystone: Specify correct alignment requirement Niklas Cassel
2025-02-07 17:18   ` Manivannan Sadhasivam
2025-01-31 18:29 ` [PATCH v4 7/7] PCI: dw-rockchip: Describe Resizable BARs as Resizable BARs Niklas Cassel
2025-02-13 13:33 ` [PATCH v4 0/7] PCI: endpoint: Allow EPF drivers to configure the size of " Niklas Cassel
2025-02-14 17:40 ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).