[PATCH v20 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

Frank Li via B4 Relay posted 9 patches 5 months, 1 week ago
There is a newer version of this series
Documentation/PCI/endpoint/pci-test-howto.rst      |  14 +++
arch/arm64/boot/dts/freescale/imx95.dtsi           |   1 +
drivers/misc/pci_endpoint_test.c                   |  85 ++++++++++++-
drivers/pci/controller/dwc/pci-imx6.c              |  25 ++--
drivers/pci/endpoint/Kconfig                       |   8 ++
drivers/pci/endpoint/Makefile                      |   1 +
drivers/pci/endpoint/functions/pci-epf-test.c      | 136 +++++++++++++++++++++
drivers/pci/endpoint/pci-ep-msi.c                  |  98 +++++++++++++++
drivers/pci/endpoint/pci-epf-core.c                |  44 +++++++
include/linux/pci-ep-msi.h                         |  28 +++++
include/linux/pci-epf.h                            |  18 +++
include/uapi/linux/pcitest.h                       |   1 +
.../selftests/pci_endpoint/pci_endpoint_test.c     |  28 +++++
13 files changed, 478 insertions(+), 9 deletions(-)
[PATCH v20 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
Posted by Frank Li via B4 Relay 5 months, 1 week ago
┌────────────┐   ┌───────────────────────────────────┐   ┌────────────────┐
│            │   │                                   │   │                │
│            │   │ PCI Endpoint                      │   │ PCI Host       │
│            │   │                                   │   │                │
│            │◄──┤ 1.platform_msi_domain_alloc_irqs()│   │                │
│            │   │                                   │   │                │
│ MSI        ├──►│ 2.write_msi_msg()                 ├──►├─BAR<n>         │
│ Controller │   │   update doorbell register address│   │                │
│            │   │   for BAR                         │   │                │
│            │   │                                   │   │ 3. Write BAR<n>│
│            │◄──┼───────────────────────────────────┼───┤                │
│            │   │                                   │   │                │
│            ├──►│ 4.Irq Handle                      │   │                │
│            │   │                                   │   │                │
│            │   │                                   │   │                │
└────────────┘   └───────────────────────────────────┘   └────────────────┘

This patches based on old https://lore.kernel.org/imx/20221124055036.1630573-1-Frank.Li@nxp.com/

Original patch only target to vntb driver. But actually it is common
method.

This patches add new API to pci-epf-core, so any EP driver can use it.

Previous v2 discussion here.
https://lore.kernel.org/imx/20230911220920.1817033-1-Frank.Li@nxp.com/

Changes in v20:
- remove set epf of_node's patch and only support one epf now.
- move imx6's patch to first
- detail change see each patches' change log
- Link to v19: https://lore.kernel.org/r/20250609-ep-msi-v19-0-77362eaa48fa@nxp.com

Changes in v19:
- irq part already in v6.16-rc1, only missed pcie/dts part
- rebase to v6.16-rc1
- update commit message for patch IMMUTABLE check.
- Link to v18: https://lore.kernel.org/r/20250414-ep-msi-v18-0-f69b49917464@nxp.com

Changes in v18:
- pci-ep.yaml: sort property order, fix maxvalue to 0x7ffff for msi-map-mask and
iommu-map-mask
- Link to v17: https://lore.kernel.org/r/20250407-ep-msi-v17-0-633ab45a31d0@nxp.com

Changes in v17:
- move document part to pci-ep.yaml
- Link to v16: https://lore.kernel.org/r/20250404-ep-msi-v16-0-d4919d68c0d0@nxp.com

Changes in v16:
- remove arm64: dts: imx95-19x19-evk: Add PCIe1 endpoint function overlay file
because there are better patches, which under review.
- Add document for pcie-ep msi-map usage
- other change to see each patch's change log
About IMMUTABLE (No change for this part, tglx provide feedback)
> - This IMMUTABLE thing serves no purpose, because you don't randomly
>   plug this end-point block on any MSI controller. They come as part
>   of an SoC.

"Yes and no. The problem is that the EP implementation is meant to be a
generic library and while GIC-ITS guarantees immutability of the
address/data pair after setup, there are architectures (x86, loongson,
riscv) where the base MSI controller does not and immutability is only
achieved when interrupt remapping is enabled. The latter can be disabled
at boot-time and then the EP implementation becomes a lottery across
affinity changes.

That was my concern about this library implementation and that's why I
asked for a mechanism to ensure that the underlying irqdomain provides a
immutable address/data pair.

So it does not matter for GIC-ITS, but in the larger picture it matters.

Thanks,

        tglx
"

So it does not matter for GIC-ITS, but in the larger picture it matters.

- Link to v15: https://lore.kernel.org/r/20250211-ep-msi-v15-0-bcacc1f2b1a9@nxp.com

Changes in v15:
- rebase to v6.14-rc1
- fix build issue find by kernel test robot
- Link to v14: https://lore.kernel.org/r/20250207-ep-msi-v14-0-9671b136f2b8@nxp.com

Changes in v14:
Marc Zyngier raised concerns about adding DOMAIN_BUS_DEVICE_PCI_EP_MSI. As
a result, the approach has been reverted to the v9 method. However, there
are several improvements:

MSI now supports msi-map in addition to msi-parent.
  - The struct device: id is used as the endpoint function (EPF) device
identity to map to the stream ID (sideband information).
  - The EPC device tree source (DTS) utilizes msi-map to provide such
information.
  - The EPF device's of_node is set to the EPC controller’s node. This
approach is commonly used for multi-function device (MFD) platform child
devices, allowing them to inherit properties from the MFD device’s DTS,
such as reset-cells and gpio-cells. This method is well-suited for the
current case, as the EPF is inherently created/binded to the EPC and
should inherit the EPC’s DTS node properties.

Additionally:

Since the basic IMX95 LUT support has already been merged into the
mainline, a DTS and driver increment patch is added to complete the
solution. The patch is rebased onto the latest linux-next tree and
aligned with the new pcitest framework.

- Link to v13: https://lore.kernel.org/r/20241218-ep-msi-v13-0-646e2192dc24@nxp.com

Changes in v13:
- Change to use DOMAIN_BUS_PCI_DEVICE_EP_MSI
- Change request id as  func | vfunc << 3
- Remove IRQ_DOMAIN_MSI_IMMUTABLE

Thomas Gleixner:

I hope capture all your points in review comments. If missed, let me know.

- Link to v12: https://lore.kernel.org/r/20241211-ep-msi-v12-0-33d4532fa520@nxp.com

Changes in v12:
- Change to use IRQ_DOMAIN_MSI_IMMUTABLE and add help function
irq_domain_msi_is_immuatble().
- split PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check to 3 patches
- Link to v11: https://lore.kernel.org/r/20241209-ep-msi-v11-0-7434fa8397bd@nxp.com

Changes in v11:
- Change to use MSI_FLAG_MSG_IMMUTABLE
- Link to v10: https://lore.kernel.org/r/20241204-ep-msi-v10-0-87c378dbcd6d@nxp.com

Changes in v10:

Thomas Gleixner:
	There are big change in pci-ep-msi.c. I am sure if go on the
corrent path. The key improvement is remove only 1 function devices's
limitation.

	I use new patch for imutable check, which relative additional
feature compared to base enablement patch.

- Remove patch Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
- Add new patch irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info
- Remove only support 1 endpoint function limiation.
- Create one MSI domain for each endpoint function devices.
- Use "msi-map" in pci ep controler node, instead of of msi-parent. first
argument is
	(func_no << 8 | vfunc_no)

- Link to v9: https://lore.kernel.org/r/20241203-ep-msi-v9-0-a60dbc3f15dd@nxp.com

Changes in v9
- Add patch platform-msi: Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
- Remove patch PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering
- Remove API pci_epf_align_inbound_addr_lo_hi
- Move doorbell_alloc in to doorbell_enable function.
- Link to v8: https://lore.kernel.org/r/20241116-ep-msi-v8-0-6f1f68ffd1bb@nxp.com

Changes in v8:
- update helper function name to pci_epf_align_inbound_addr()
- Link to v7: https://lore.kernel.org/r/20241114-ep-msi-v7-0-d4ac7aafbd2c@nxp.com

Changes in v7:
- Add helper function pci_epf_align_addr();
- Link to v6: https://lore.kernel.org/r/20241112-ep-msi-v6-0-45f9722e3c2a@nxp.com

Changes in v6:
- change doorbell_addr to doorbell_offset
- use round_down()
- add Niklas's test by tag
- rebase to pci/endpoint
- Link to v5: https://lore.kernel.org/r/20241108-ep-msi-v5-0-a14951c0d007@nxp.com

Changes in v5:
- Move request_irq to epf test function driver for more flexiable user case
- Add fixed size bar handler
- Some minor improvememtn to see each patches's changelog.
- Link to v4: https://lore.kernel.org/r/20241031-ep-msi-v4-0-717da2d99b28@nxp.com

Changes in v4:
- Remove patch genirq/msi: Add cleanup guard define for msi_lock_descs()/msi_unlock_descs()
- Use new method to avoid compatible problem.
  Add new command DOORBELL_ENABLE and DOORBELL_DISABLE.
  pcitest -B send DOORBELL_ENABLE first, EP test function driver try to
remap one of BAR_N (except test register bar) to ITS MSI MMIO space. Old
driver don't support new command, so failure return, not side effect.
  After test, DOORBELL_DISABLE command send out to recover original map, so
pcitest bar test can pass as normal.
- Other detail change see each patches's change log
- Link to v3: https://lore.kernel.org/r/20241015-ep-msi-v3-0-cedc89a16c1a@nxp.com

Change from v2 to v3
- Fixed manivannan's comments
- Move common part to pci-ep-msi.c and pci-ep-msi.h
- rebase to 6.12-rc1
- use RevID to distingiush old version

mkdir /sys/kernel/config/pci_ep/functions/pci_epf_test/func1
echo 16 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/msi_interrupts
echo 0x080c > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/deviceid
echo 0x1957 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/vendorid
echo 1 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/revid
^^^^^^ to enable platform msi support.
ln -s /sys/kernel/config/pci_ep/functions/pci_epf_test/func1 /sys/kernel/config/pci_ep/controllers/4c380000.pcie-ep

- use new device ID, which identify support doorbell to avoid broken
compatility.

    Enable doorbell support only for PCI_DEVICE_ID_IMX8_DB, while other devices
    keep the same behavior as before.

           EP side             RC with old driver      RC with new driver
    PCI_DEVICE_ID_IMX8_DB          no probe              doorbell enabled
    Other device ID             doorbell disabled*       doorbell disabled*

    * Behavior remains unchanged.

Change from v1 to v2
- Add missed patch for endpont/pci-epf-test.c
- Move alloc and free to epc driver from epf.
- Provide general help function for EPC driver to alloc platform msi irq.
- Fixed manivannan's comments.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (9):
      PCI: imx6: Add helper function imx_pcie_add_lut_by_rid()
      PCI: imx6: Add LUT configuration for MSI/IOMMU in Endpoint mode
      PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
      PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check
      PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
      PCI: endpoint: pci-epf-test: Add doorbell test support
      misc: pci_endpoint_test: Add doorbell test case
      selftests: pci_endpoint: Add doorbell test case
      arm64: dts: imx95: Add msi-map for pci-ep device

 Documentation/PCI/endpoint/pci-test-howto.rst      |  14 +++
 arch/arm64/boot/dts/freescale/imx95.dtsi           |   1 +
 drivers/misc/pci_endpoint_test.c                   |  85 ++++++++++++-
 drivers/pci/controller/dwc/pci-imx6.c              |  25 ++--
 drivers/pci/endpoint/Kconfig                       |   8 ++
 drivers/pci/endpoint/Makefile                      |   1 +
 drivers/pci/endpoint/functions/pci-epf-test.c      | 136 +++++++++++++++++++++
 drivers/pci/endpoint/pci-ep-msi.c                  |  98 +++++++++++++++
 drivers/pci/endpoint/pci-epf-core.c                |  44 +++++++
 include/linux/pci-ep-msi.h                         |  28 +++++
 include/linux/pci-epf.h                            |  18 +++
 include/uapi/linux/pcitest.h                       |   1 +
 .../selftests/pci_endpoint/pci_endpoint_test.c     |  28 +++++
 13 files changed, 478 insertions(+), 9 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20241010-ep-msi-8b4cab33b1be

Best regards,
--
Frank Li <Frank.Li@nxp.com>


Re: [PATCH v20 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
Posted by Niklas Cassel 5 months, 1 week ago
Hello Frank,

I tested v20 of your series, but unfortunately, it still doesn't work.

When enabling the doorbell, the programming of the inbound iATU fails:

## pci_epf_test_enable_doorbell()
## keeps the BAR size, and BAR type of a BAR that has already been configured,
## but changes the address translation for this BAR to redirect to the GIC ITS
## rather than to the memory allocated by pci_epf_alloc_space()
## (does not free the memory allocated by pci_epf_alloc_space())

[   39.347502] pci_epf_test_enable_doorbell: msg hi: 0x0 msg low: 0xfe670040
[   39.348103] pci_epf_test_enable_doorbell: base: 0xfe670000 off: 0x40
[   39.348658] dw_pcie_ep_inbound_atu index: 1 parent_bus_addr: 0xfe670000 bar: 1 size: 0x100000
[   39.349403] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xfe670000 pci->region_align: 0x10000 IS_ALIGNED: 1
[   39.350260] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xfe670000 size: 0x100000 IS_ALIGNED: 0
[   39.351028] rockchip-dw-pcie a40000000.pcie-ep: Failed to program IB window

## pci_epf_test_disable_doorbell()
## changes the address translation for this BAR to redirect to the memory
## allocated by pci_epf_alloc_space() (which was never freed when enabling the
## doorbell)

[   39.351656] dw_pcie_ep_inbound_atu index: 1 parent_bus_addr: 0xa2e00000 bar: 1 size: 0x100000
[   39.352401] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xa2e00000 pci->region_align: 0x10000 IS_ALIGNED: 1
[   39.353257] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xa2e00000 size: 0x100000 IS_ALIGNED: 1


The reason why pci_epf_test_enable_doorbell() fails is because of this check:
https://github.com/torvalds/linux/blob/v6.16-rc5/drivers/pci/controller/dwc/pcie-designware.c#L663

If you want to understand why this very important check is there, it is
because the DWC controller requires that the physical address programmed in
the iATU is aligned to the size of the BAR (BAR_MASK+1), see this commit:
https://github.com/torvalds/linux/commit/129f6af747b2


Applying the following patch on top of your v20 series makes things work as
intended and makes the pcie_ep_doorbell.DOORBELL_TEST selftest pass for me:

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index dfdd25cfc003..7d356b0201ae 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -738,9 +738,9 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
 	reg->doorbell_bar = cpu_to_le32(bar);
 
 	msg = &epf->db_msg[0].msg;
-	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
+	ret = pci_epf_align_inbound_addr(epf, epf->bar[bar].size,
+					((u64)msg->address_hi << 32) | msg->address_lo,
 					&epf_test->db_bar.phys_addr, &offset);
-
 	if (ret)
 		goto err_doorbell;
 
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index c21d8e786eb3..b3d4117182e2 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -478,44 +478,36 @@ struct pci_epf *pci_epf_create(const char *name)
 EXPORT_SYMBOL_GPL(pci_epf_create);
 
 /**
- * pci_epf_align_inbound_addr() - Align the given address based on the BAR
- *				 alignment requirement
+ * pci_epf_align_inbound_addr() - Align the given address based on the BAR size
+ *
  * @epf: the EPF device
+ * @bar_size: the current BAR size
  * @addr: inbound address to be aligned
- * @bar: the BAR number corresponding to the given addr
- * @base: base address matching the @bar alignment requirement.
+ * @base: base address matching the @bar_size alignment requirement.
  * @off: offset to be added to the @base address.
  *
- * Helper function to align input 'addr' to base and offset, which match
- * BAR's alignment requirement.
+ * Helper function to align input 'addr' to base and offset, when dynamically
+ * changing a BAR.
  *
  * The pci_epf_alloc_space() function already accounts for alignment. This is
  * primarily intended for use with other memory regions not allocated by
  * pci_epf_alloc_space(), such as peripheral register spaces or the trigger
  * address for a platform MSI controller.
+ *
+ * Since this function is only used when dynamically changing a BAR (i.e. when
+ * calling set_bar() twice, without ever calling clear_bar(), as calling
+ * clear_bar() would clear the BAR's PCI address assigned by the host), this
+ * function must align to the current BAR size, since we are not clearing the
+ * BAR configuration.
  */
-int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
-			      u64 addr, dma_addr_t *base, size_t *off)
+int pci_epf_align_inbound_addr(struct pci_epf *epf, size_t bar_size, u64 addr,
+			      dma_addr_t *base, size_t *off)
 {
-	const struct pci_epc_features *epc_features;
-	u64 align;
-
-	if (!base || !off)
+	if (!base || !off || !bar_size)
 		return -EINVAL;
 
-	epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
-	if (!epc_features) {
-		dev_err(&epf->dev, "epc_features not implemented\n");
-		return -EOPNOTSUPP;
-	}
-
-	align = epc_features->align;
-	align = align ? align : 128;
-	if (epc_features->bar[bar].type == BAR_FIXED)
-		align = max(epc_features->bar[bar].fixed_size, align);
-
-	*base = round_down(addr, align);
-	*off = addr & (align - 1);
+	*base = round_down(addr, bar_size);
+	*off = addr & (bar_size - 1);
 
 	return 0;
 }
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 0ca08f0d05d7..bcc8184325d2 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -242,8 +242,8 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
 
-int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
-			      u64 addr, dma_addr_t *base, size_t *off);
+int pci_epf_align_inbound_addr(struct pci_epf *epf, size_t bar_size, u64 addr,
+			      dma_addr_t *base, size_t *off);
 int pci_epf_bind(struct pci_epf *epf);
 void pci_epf_unbind(struct pci_epf *epf);
 int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);




However, the more I think about it, considering that this alignment requirement
is inherent to the DWC controller (other controllers might not have this
requirement), perhaps pci_epf_align_inbound_addr() should not be a function in
pci-epf-core.c, perhaps this function would be better suited to live in
drivers/pci/controller/dwc/pcie-designware-ep.c ?


Kind regards,
Niklas
Re: [PATCH v20 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
Posted by Frank Li 5 months, 1 week ago
On Thu, Jul 10, 2025 at 01:40:25PM +0200, Niklas Cassel wrote:
> Hello Frank,
>
> I tested v20 of your series, but unfortunately, it still doesn't work.
>
> When enabling the doorbell, the programming of the inbound iATU fails:
>
> ## pci_epf_test_enable_doorbell()
> ## keeps the BAR size, and BAR type of a BAR that has already been configured,
> ## but changes the address translation for this BAR to redirect to the GIC ITS
> ## rather than to the memory allocated by pci_epf_alloc_space()
> ## (does not free the memory allocated by pci_epf_alloc_space())
>
> [   39.347502] pci_epf_test_enable_doorbell: msg hi: 0x0 msg low: 0xfe670040
> [   39.348103] pci_epf_test_enable_doorbell: base: 0xfe670000 off: 0x40
> [   39.348658] dw_pcie_ep_inbound_atu index: 1 parent_bus_addr: 0xfe670000 bar: 1 size: 0x100000
> [   39.349403] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xfe670000 pci->region_align: 0x10000 IS_ALIGNED: 1
> [   39.350260] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xfe670000 size: 0x100000 IS_ALIGNED: 0
> [   39.351028] rockchip-dw-pcie a40000000.pcie-ep: Failed to program IB window
>
> ## pci_epf_test_disable_doorbell()
> ## changes the address translation for this BAR to redirect to the memory
> ## allocated by pci_epf_alloc_space() (which was never freed when enabling the
> ## doorbell)
>
> [   39.351656] dw_pcie_ep_inbound_atu index: 1 parent_bus_addr: 0xa2e00000 bar: 1 size: 0x100000
> [   39.352401] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xa2e00000 pci->region_align: 0x10000 IS_ALIGNED: 1
> [   39.353257] dw_pcie_prog_ep_inbound_atu parent_bus_addr: 0xa2e00000 size: 0x100000 IS_ALIGNED: 1
>
>
> The reason why pci_epf_test_enable_doorbell() fails is because of this check:
> https://github.com/torvalds/linux/blob/v6.16-rc5/drivers/pci/controller/dwc/pcie-designware.c#L663
>
> If you want to understand why this very important check is there, it is
> because the DWC controller requires that the physical address programmed in
> the iATU is aligned to the size of the BAR (BAR_MASK+1), see this commit:
> https://github.com/torvalds/linux/commit/129f6af747b2
>
>
> Applying the following patch on top of your v20 series makes things work as
> intended and makes the pcie_ep_doorbell.DOORBELL_TEST selftest pass for me:
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index dfdd25cfc003..7d356b0201ae 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -738,9 +738,9 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
>  	reg->doorbell_bar = cpu_to_le32(bar);
>
>  	msg = &epf->db_msg[0].msg;
> -	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
> +	ret = pci_epf_align_inbound_addr(epf, epf->bar[bar].size,
> +					((u64)msg->address_hi << 32) | msg->address_lo,
>  					&epf_test->db_bar.phys_addr, &offset);
> -
>  	if (ret)
>  		goto err_doorbell;
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index c21d8e786eb3..b3d4117182e2 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -478,44 +478,36 @@ struct pci_epf *pci_epf_create(const char *name)
>  EXPORT_SYMBOL_GPL(pci_epf_create);
>
>  /**
> - * pci_epf_align_inbound_addr() - Align the given address based on the BAR
> - *				 alignment requirement
> + * pci_epf_align_inbound_addr() - Align the given address based on the BAR size
> + *
>   * @epf: the EPF device
> + * @bar_size: the current BAR size
>   * @addr: inbound address to be aligned
> - * @bar: the BAR number corresponding to the given addr
> - * @base: base address matching the @bar alignment requirement.
> + * @base: base address matching the @bar_size alignment requirement.
>   * @off: offset to be added to the @base address.
>   *
> - * Helper function to align input 'addr' to base and offset, which match
> - * BAR's alignment requirement.
> + * Helper function to align input 'addr' to base and offset, when dynamically
> + * changing a BAR.
>   *
>   * The pci_epf_alloc_space() function already accounts for alignment. This is
>   * primarily intended for use with other memory regions not allocated by
>   * pci_epf_alloc_space(), such as peripheral register spaces or the trigger
>   * address for a platform MSI controller.
> + *
> + * Since this function is only used when dynamically changing a BAR (i.e. when
> + * calling set_bar() twice, without ever calling clear_bar(), as calling
> + * clear_bar() would clear the BAR's PCI address assigned by the host), this
> + * function must align to the current BAR size, since we are not clearing the
> + * BAR configuration.
>   */
> -int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
> -			      u64 addr, dma_addr_t *base, size_t *off)
> +int pci_epf_align_inbound_addr(struct pci_epf *epf, size_t bar_size, u64 addr,
> +			      dma_addr_t *base, size_t *off)
>  {
> -	const struct pci_epc_features *epc_features;
> -	u64 align;
> -
> -	if (!base || !off)
> +	if (!base || !off || !bar_size)
>  		return -EINVAL;
>
> -	epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
> -	if (!epc_features) {
> -		dev_err(&epf->dev, "epc_features not implemented\n");
> -		return -EOPNOTSUPP;
> -	}
> -
> -	align = epc_features->align;
> -	align = align ? align : 128;
> -	if (epc_features->bar[bar].type == BAR_FIXED)
> -		align = max(epc_features->bar[bar].fixed_size, align);
> -
> -	*base = round_down(addr, align);
> -	*off = addr & (align - 1);
> +	*base = round_down(addr, bar_size);
> +	*off = addr & (bar_size - 1);
>
>  	return 0;
>  }
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 0ca08f0d05d7..bcc8184325d2 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -242,8 +242,8 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>  void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
>  			enum pci_epc_interface_type type);
>
> -int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
> -			      u64 addr, dma_addr_t *base, size_t *off);
> +int pci_epf_align_inbound_addr(struct pci_epf *epf, size_t bar_size, u64 addr,
> +			      dma_addr_t *base, size_t *off);
>  int pci_epf_bind(struct pci_epf *epf);
>  void pci_epf_unbind(struct pci_epf *epf);
>  int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);
>
>
>
>
> However, the more I think about it, considering that this alignment requirement
> is inherent to the DWC controller (other controllers might not have this
> requirement), perhaps pci_epf_align_inbound_addr() should not be a function in
> pci-epf-core.c, perhaps this function would be better suited to live in
> drivers/pci/controller/dwc/pcie-designware-ep.c ?

I think align to bar size is quite make sense now. Even other controllers
have not such requirement, align to bar size still work.

Anyways, it need be align. We still pass down bar number, get bar size
in pci_epf_align_inbound_addr(). So we can fine tune algorithm in future,
such as add field in epc_features.

Frank
>
>
> Kind regards,
> Niklas