minor PCI endpoint cleanups

Hello all,

This series used to be called:
"PCI: endpoint: set prefetchable bit for 64-bit BARs"

However, since after discussions with Arnd and Mani, that patch has been dropped, however, the other cleanups are still worth including IMO, thus the series has been renamed.

Changes since v3:
-Picked up tags from Mani.
-Fixed minor comments from Mani.
-Dropped patch [PATCH v3 1/9] PCI: endpoint: pci-epf-test: Fix incorrect
loop increment.
-Dropped patch [PATCH v3 9/9] PCI: endpoint: Set prefetch when allocating
memory for 64-bit BARs.
-Reordered some of the patches to have a more logical ordering.

Niklas Cassel (7):
PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop
PCI: endpoint: Allocate a 64-bit BAR if that is the only option
PCI: endpoint: pci-epf-test: Remove superfluous code
PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop
PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind()
PCI: cadence: Set a 64-bit BAR if requested
PCI: rockchip-ep: Set a 64-bit BAR if requested

 .../pci/controller/cadence/pcie-cadence-ep.c  |  5 +-
 drivers/pci/controller/pcie-rockchip-ep.c     |  2 +-
 drivers/pci/endpoint/functions/pci-epf-test.c | 62 +++++--------------
 drivers/pci/endpoint/pci-epf-core.c           |  9 ++-
 4 files changed, 23 insertions(+), 55 deletions(-)

2.44.0

Make pci-epf-test use pci_epc_get_next_free_bar() just like pci-epf-ntb.c and pci-epf-vntb.c.

Using pci_epc_get_next_free_bar() also makes it more obvious that pci-epf-test does no special configuration at all.

(The only configuration pci-epf-test does is setting PCI_BASE_ADDRESS_MEM_TYPE_64 if epc_features has marked the specific BAR as only_64bit. pci_epc_get_next_free_bar() already takes only_64bit into account when looping.)

This way, the code is more consistent between EPF drivers, and pci-epf-test does not need to explicitly check if the BAR is reserved, or if the index belongs to a BAR succeeding a 64-bit only BAR.

---
 drivers/pci/endpoint/functions/pci-epf-test.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

index cd4ffb39dcdc..16dfd61cd9fb 100644
@@ -817,14 +817,13 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	struct device *dev = &epf->dev;
-	struct pci_epf_bar *epf_bar;
 	size_t msix_table_size = 0;
 	size_t test_reg_bar_size;
 	size_t pba_size = 0;
 	bool msix_capable;
 	void *base;
-	int bar, add;
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	enum pci_barno bar;
 	const struct pci_epc_features *epc_features;
 	size_t test_reg_size;
 

@@ -849,16 +848,14 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	}
 	epf_test->reg[test_reg_bar] = base;
 
-	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
-		epf_bar = &epf->bar[bar];
-		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
+	for (bar = BAR_0; bar < PCI_STD_NUM_BARS; bar++) {
+		bar = pci_epc_get_next_free_bar(epc_features, bar);
+		if (bar == NO_BAR)
+			break;
 
 		if (bar == test_reg_bar)
 			continue;
 
-		if (epc_features->bar[bar].type == BAR_RESERVED)
-			continue;
-
 		base = pci_epf_alloc_space(epf, bar_size[bar], bar,
 					   epc_features, PRIMARY_INTERFACE);
 		if (!base)

2.44.0

pci_epf_alloc_space() already sets the 64-bit flag if the BAR size is larger than 2GB, even if the caller did not explicitly request a 64-bit BAR.

Thus, let pci_epf_alloc_space() also set the 64-bit flag if the hardware description says that the specific BAR can only be 64-bit.

---
 drivers/pci/endpoint/pci-epf-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 0a28a0b0911b..323f2a60ab16 100644
@@ -255,6 +255,8 @@ EXPORT_SYMBOL_GPL(pci_epf_free_space);
  * @type: Identifies if the allocation is for primary EPC or secondary EPC
  *
  * Invoke to allocate memory for the PCI EPF register space.
+ * Flag PCI_BASE_ADDRESS_MEM_TYPE_64 will automatically get set if the BAR
+ * can only be a 64-bit BAR, or if the requested size is larger than 2 GB.
  */
 void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 			  const struct pci_epc_features *epc_features,

@@ -304,9 +306,10 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	epf_bar[bar].addr = space;
 	epf_bar[bar].size = size;
 	epf_bar[bar].barno = bar;
-	epf_bar[bar].flags |= upper_32_bits(size) ?
-				PCI_BASE_ADDRESS_MEM_TYPE_64 :
-				PCI_BASE_ADDRESS_MEM_TYPE_32;
+	if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+	else
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
 
 	return space;
 }

2.44.0

The only reason why we call pci_epf_configure_bar() is to set PCI_BASE_ADDRESS_MEM_TYPE_64 in case the hardware requires it.

However, this flag is now automatically set when allocating a BAR that can only be a 64-bit BAR, so we can drop pci_epf_configure_bar() completely.

---
 drivers/pci/endpoint/functions/pci-epf-test.c | 14 --------------
 1 file changed, 14 deletions(-)

index 16dfd61cd9fb..0a83a1901bb7 100644
@@ -867,19 +867,6 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	return 0;
 }
 
-static void pci_epf_configure_bar(struct pci_epf *epf,
-				  const struct pci_epc_features *epc_features)
-{
-	struct pci_epf_bar *epf_bar;
-	int i;
-
-	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
-		epf_bar = &epf->bar[i];
-		if (epc_features->bar[i].only_64bit)
-			epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
-	}
-}
-
 static int pci_epf_test_bind(struct pci_epf *epf)
 {
 	int ret;

@@ -904,7 +891,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 	test_reg_bar = pci_epc_get_first_free_bar(epc_features);
 	if (test_reg_bar < 0)
 		return -EINVAL;
-	pci_epf_configure_bar(epf, epc_features);
 
 	epf_test->test_reg_bar = test_reg_bar;
 	epf_test->epc_features = epc_features;

2.44.0

Simplify the loop in pci_epf_test_set_bar().
If we allocated memory for the BAR, we need to call set_bar() for that BAR, if we did not allocated memory for that BAR, we need to skip. It is as simple as that. This also matches the logic in pci_epf_test_unbind().

A 64-bit BAR will still only be one allocation, with the BAR succeeding the 64-bit BAR being null.

While at it, remove the misleading comment.
A EPC .set_bar() callback should never change the epf_bar->flags. (E.g. to set a 64-bit BAR if we requested a 32-bit BAR.)

A .set_bar() callback should do what we request it to do. If it can't satisfy the request, it should return an error.

If platform has a specific requirement, e.g. that a certain BAR has to be a 64-bit BAR, then it should specify that by setting the .only_64bit flag for that specific BAR in epc_features->bar[], such that pci_epf_alloc_space() will return a epf_bar with the 64-bit flag set. (Such that .set_bar() will receive a request to set a 64-bit BAR.)

---
 drivers/pci/endpoint/functions/pci-epf-test.c | 21 ++++---------------
 1 file changed, 4 insertions(+), 17 deletions(-)

index 0a83a1901bb7..faf347216b6b 100644
@@ -709,31 +709,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 
 static int pci_epf_test_set_bar(struct pci_epf *epf)
 {
-	int bar, add;
-	int ret;
-	struct pci_epf_bar *epf_bar;
+	int bar, ret;
 	struct pci_epc *epc = epf->epc;
 	struct device *dev = &epf->dev;
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
-	const struct pci_epc_features *epc_features;
 
-	epc_features = epf_test->epc_features;
-
-	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
-		epf_bar = &epf->bar[bar];
-		/*
-		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
-		 * if the specific implementation required a 64-bit BAR,
-		 * even if we only requested a 32-bit BAR.
-		 */
-		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
-
-		if (epc_features->bar[bar].type == BAR_RESERVED)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
+		if (!epf_test->reg[bar])
 			continue;
 
 		ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no,
-				      epf_bar);
+				      &epf->bar[bar]);
 		if (ret) {
 			pci_epf_free_space(epf, epf_test->reg[bar], bar,
 					   PRIMARY_INTERFACE);

2.44.0

- Mani

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

Clean up pci_epf_test_unbind() by using a continue if we did not allocate memory for the BAR index. This reduces the indentation level by one.

This makes pci_epf_test_unbind() more similar to pci_epf_test_set_bar().

---
 drivers/pci/endpoint/functions/pci-epf-test.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

index faf347216b6b..d244a5083d04 100644
@@ -690,20 +690,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	struct pci_epc *epc = epf->epc;
-	struct pci_epf_bar *epf_bar;
 	int bar;
 
 	cancel_delayed_work(&epf_test->cmd_handler);
 	pci_epf_test_clean_dma_chan(epf_test);
 	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
-		epf_bar = &epf->bar[bar];
+		if (!epf_test->reg[bar])
+			continue;
 
-		if (epf_test->reg[bar]) {
-			pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
-					  epf_bar);
-			pci_epf_free_space(epf, epf_test->reg[bar], bar,
-					   PRIMARY_INTERFACE);
-		}
+		pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
+				  &epf->bar[bar]);
+		pci_epf_free_space(epf, epf_test->reg[bar], bar,
+				   PRIMARY_INTERFACE);
 	}
 }
 

2.44.0

- Mani

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

Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB is invalid if 64-bit flag is not set") it has been impossible to get the .set_bar() callback with a BAR size > 2 GB (yes, 2GB!), if the BAR was also not requested to be configured as a 64-bit BAR.

Thus, forcing setting the 64-bit flag for BARs larger than 2 GB in the lower level driver is dead code and can be removed.

It is however possible that an EPF driver configures a BAR as 64-bit, even if the requested size is < 4 GB.

Respect the requested BAR configuration, just like how it is already repected with regards to the prefetchable bit.

---
 drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index 2d0a8d78bffb..de10e5edd1b0 100644
@@ -99,14 +99,11 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn,
 		ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS;
 	} else {
 		bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH);
-		bool is_64bits = sz > SZ_2G;
+		bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64);
 
 		if (is_64bits && (bar & 1))
 			return -EINVAL;
 
-		if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))
-			epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
-
 		if (is_64bits && is_prefetch)
 			ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS;
 		else if (is_prefetch)

2.44.0

Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB is invalid if 64-bit flag is not set") it has been impossible to get the .set_bar() callback with a BAR size > 2 GB (yes, 2GB!), if the BAR was also not requested to be configured as a 64-bit BAR.

It is however possible that an EPF driver configures a BAR as 64-bit, even if the requested size is < 4 GB.

Respect the requested BAR configuration, just like how it is already repected with regards to the prefetchable bit.

---
 drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index c9046e97a1d2..57472cf48997 100644
@@ -153,7 +153,7 @@ static int rockchip_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn,
 		ctrl = ROCKCHIP_PCIE_CORE_BAR_CFG_CTRL_IO_32BITS;
 	} else {
 		bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH);
-		bool is_64bits = sz > SZ_2G;
+		bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64);
 
 		if (is_64bits && (bar & 1))
 			return -EINVAL;

2.44.0

Hello,

[...]

Applied to endpoint, thank you!

[01/05] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop

        https://git.kernel.org/pci/pci/c/417660525d6f
[02/05] PCI: endpoint: Allocate a 64-bit BAR if that is the only option
        https://git.kernel.org/pci/pci/c/29a025b6fbf3
[03/05] PCI: endpoint: pci-epf-test: Remove superfluous code
        https://git.kernel.org/pci/pci/c/828e870431aa
[04/05] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop
        https://git.kernel.org/pci/pci/c/e49eab944cfb
[05/05] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind()
        https://git.kernel.org/pci/pci/c/597ac0fa37b8

Applied to controller/cadence, thank you!

[1/1] PCI: cadence: Set a 64-bit BAR if requested

      https://git.kernel.org/pci/pci/c/07db0fa80cf3

Applied to controller/rockchip, thank you!

[1/1] PCI: rockchip-ep: Set a 64-bit BAR if requested

      https://git.kernel.org/pci/pci/c/de66b37a174f

	Krzysztof