┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐ │ │ │ │ │ │ │ │ │ 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 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 (15): platform-msi: Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all() irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable() irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS irqchip/gic-v3-its: Add support for device tree msi-map and msi-mask PCI: endpoint: Set ID and of_node for function driver 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 pci: imx6: Add helper function imx_pcie_add_lut_by_rid() pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode arm64: dts: imx95: Add msi-map for pci-ep device arm64: dts: imx95-19x19-evk: Add PCIe1 endpoint function overlay file
arch/arm64/boot/dts/freescale/Makefile | 3 + .../dts/freescale/imx95-19x19-evk-pcie1-ep.dtso | 21 ++++ arch/arm64/boot/dts/freescale/imx95.dtsi | 1 + drivers/base/platform-msi.c | 1 + drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++ drivers/irqchip/irq-gic-v3-its.c | 2 +- drivers/misc/pci_endpoint_test.c | 81 +++++++++++++ drivers/pci/controller/dwc/pci-imx6.c | 25 ++-- drivers/pci/endpoint/Makefile | 1 + drivers/pci/endpoint/functions/pci-epf-test.c | 132 +++++++++++++++++++++ drivers/pci/endpoint/pci-ep-msi.c | 90 ++++++++++++++ drivers/pci/endpoint/pci-epf-core.c | 48 ++++++++ include/linux/irqdomain.h | 7 ++ include/linux/pci-ep-msi.h | 28 +++++ include/linux/pci-epf.h | 21 ++++ include/uapi/linux/pcitest.h | 1 + .../selftests/pci_endpoint/pci_endpoint_test.c | 25 ++++ 17 files changed, 486 insertions(+), 9 deletions(-) --- base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20241010-ep-msi-8b4cab33b1be
Best regards, --- Frank Li Frank.Li@nxp.com
The follow steps trigger kernel dump warning and platform_device_msi_init_and_alloc_irqs() return false.
1: platform_device_msi_init_and_alloc_irqs(); 2: platform_device_msi_free_irqs_all(); 3: platform_device_msi_init_and_alloc_irqs();
[ 76.713677] WARNING: CPU: 3 PID: 134 at kernel/irq/msi.c:1028 msi_create_device_irq_domain+0x1bc/0x22c [ 76.723010] Modules linked in: [ 76.726082] CPU: 3 UID: 0 PID: 134 Comm: kworker/3:1H Not tainted 6.13.0-rc1-00015-gd60b98003b43-dirty #57 [ 76.735741] Hardware name: NXP i.MX95 19X19 board (DT) [ 76.740883] Workqueue: kpcitest pci_epf_test_cmd_handler [ 76.746212] pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 76.753172] pc : msi_create_device_irq_domain+0x1bc/0x22c [ 76.758586] lr : msi_create_device_irq_domain+0x104/0x22c [ 76.763988] sp : ffff800083f43be0 [ 76.767313] x29: ffff800083f43be0 x28: 0000000000000000 x27: ffff8000827a7000 [ 76.774466] x26: ffff00008085f400 x25: ffff00008000b180 x24: ffff000080fc6410 [ 76.781624] x23: ffff000085704cc0 x22: ffff8000811c8828 x21: ffff000085704cc0 [ 76.788774] x20: ffff000082814000 x19: 0000000000000000 x18: ffffffffffffffff [ 76.795933] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 [ 76.803083] x14: 0000000000000000 x13: 0000000f00000000 x12: 0000000000000000 [ 76.810233] x11: 0000000000000000 x10: 000000000000002d x9 : ffff800083f43ba0 [ 76.817383] x8 : 00000000ffffffff x7 : 0000000000000019 x6 : ffff0000857e443a [ 76.824533] x5 : 0000000000000000 x4 : ffffffffffffffff x3 : ffff000085704ce8 [ 76.831683] x2 : ffff000080835640 x1 : 0000000000000213 x0 : ffff0000877189c0 [ 76.838840] Call trace: [ 76.841287] msi_create_device_irq_domain+0x1bc/0x22c (P) [ 76.846701] msi_create_device_irq_domain+0x104/0x22c (L) [ 76.852118] platform_device_msi_init_and_alloc_irqs+0x6c/0xb8
Do below two things in platform_device_msi_init_and_alloc_irqs(). - msi_create_device_irq_domain() - msi_domain_alloc_irqs_range()
But only call msi_domain_free_irqs_all() in platform_device_msi_free_irqs_all(), which missed call msi_remove_device_irq_domain(). This cause above kernel dump when call platform_device_msi_init_and_alloc_irqs() again.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 5o v15 - none
change from v13 to v14 - bring back from v9 and remove fixup and cc stable before it is new use case. --- drivers/base/platform-msi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c index 0e60dd650b5e0..70db08f3ac6fa 100644 --- a/drivers/base/platform-msi.c +++ b/drivers/base/platform-msi.c @@ -95,5 +95,6 @@ EXPORT_SYMBOL_GPL(platform_device_msi_init_and_alloc_irqs); void platform_device_msi_free_irqs_all(struct device *dev) { msi_domain_free_irqs_all(dev, MSI_DEFAULT_DOMAIN); + msi_remove_device_irq_domain(dev, MSI_DEFAULT_DOMAIN); } EXPORT_SYMBOL_GPL(platform_device_msi_free_irqs_all);
On Tue, 11 Feb 2025 19:21:54 +0000, Frank Li Frank.Li@nxp.com wrote:
The follow steps trigger kernel dump warning and platform_device_msi_init_and_alloc_irqs() return false.
1: platform_device_msi_init_and_alloc_irqs(); 2: platform_device_msi_free_irqs_all(); 3: platform_device_msi_init_and_alloc_irqs();
[ 76.713677] WARNING: CPU: 3 PID: 134 at kernel/irq/msi.c:1028 msi_create_device_irq_domain+0x1bc/0x22c [ 76.723010] Modules linked in: [ 76.726082] CPU: 3 UID: 0 PID: 134 Comm: kworker/3:1H Not tainted 6.13.0-rc1-00015-gd60b98003b43-dirty #57 [ 76.735741] Hardware name: NXP i.MX95 19X19 board (DT) [ 76.740883] Workqueue: kpcitest pci_epf_test_cmd_handler [ 76.746212] pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 76.753172] pc : msi_create_device_irq_domain+0x1bc/0x22c [ 76.758586] lr : msi_create_device_irq_domain+0x104/0x22c [ 76.763988] sp : ffff800083f43be0 [ 76.767313] x29: ffff800083f43be0 x28: 0000000000000000 x27: ffff8000827a7000 [ 76.774466] x26: ffff00008085f400 x25: ffff00008000b180 x24: ffff000080fc6410 [ 76.781624] x23: ffff000085704cc0 x22: ffff8000811c8828 x21: ffff000085704cc0 [ 76.788774] x20: ffff000082814000 x19: 0000000000000000 x18: ffffffffffffffff [ 76.795933] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 [ 76.803083] x14: 0000000000000000 x13: 0000000f00000000 x12: 0000000000000000 [ 76.810233] x11: 0000000000000000 x10: 000000000000002d x9 : ffff800083f43ba0 [ 76.817383] x8 : 00000000ffffffff x7 : 0000000000000019 x6 : ffff0000857e443a [ 76.824533] x5 : 0000000000000000 x4 : ffffffffffffffff x3 : ffff000085704ce8 [ 76.831683] x2 : ffff000080835640 x1 : 0000000000000213 x0 : ffff0000877189c0 [ 76.838840] Call trace: [ 76.841287] msi_create_device_irq_domain+0x1bc/0x22c (P) [ 76.846701] msi_create_device_irq_domain+0x104/0x22c (L) [ 76.852118] platform_device_msi_init_and_alloc_irqs+0x6c/0xb8
Do below two things in platform_device_msi_init_and_alloc_irqs().
- msi_create_device_irq_domain()
- msi_domain_alloc_irqs_range()
But only call msi_domain_free_irqs_all() in platform_device_msi_free_irqs_all(), which missed call msi_remove_device_irq_domain(). This cause above kernel dump when call platform_device_msi_init_and_alloc_irqs() again.
I don't think this commit message makes much sense, and doesn't explain the essential problem, which is the lack of symmetry. I'd suggest something like:
"platform_device_msi_init_and_alloc_irqs() performs two tasks: allocating the MSI domain for a platform device, and allocate a number of MSIs in that domain.
platform_device_msi_free_irqs_all() only frees the MSIs, and leaves the MSI domain alive.
Given that platform_device_msi_init_and_alloc_irqs() is the sole tool a platform device has to allocate platform MSIs, it would make sense for platform_device_msi_free_irqs_all() to teardown the MSI domain at the same time as the MSIs.
This also avoids warnings and unexpected behaviours when a driver repeatedly allocates and frees MSIs."
With that:
Acked-by: Marc Zyngier maz@kernel.org
M.
Add the flag IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and the API function irq_domain_is_msi_immutable() to check if the MSI controller retains an immutable address/data pair during irq_set_affinity().
Ensure compatibility with MSI users like PCIe Endpoint Doorbell, which require the address/data pair to remain unchanged after setup. Use this function to verify if the MSI controller is immutable.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - Roll back to v12 version because Marc Zyngier have concern about add DOMAIN_BUS_DEVICE_PCI_EP_MSI. https://lore.kernel.org/imx/861pxfq315.wl-maz@kernel.org/
Change from v11 to v12 - change to IRQ_DOMAIN_FLAG_MSI_IMMUTABLE to minimized the code change. --- include/linux/irqdomain.h | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index e432b6a12a32f..3dbe05d8740e6 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -231,6 +231,9 @@ enum { /* Irq domain must destroy generic chips when removed */ IRQ_DOMAIN_FLAG_DESTROY_GC = (1 << 10),
+ /* Address and data pair is mutable when irq_set_affinity() */ + IRQ_DOMAIN_FLAG_MSI_IMMUTABLE = (1 << 11), + /* * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved * for implementation specific purposes and ignored by the @@ -692,6 +695,10 @@ static inline bool irq_domain_is_msi_device(struct irq_domain *domain) return domain->flags & IRQ_DOMAIN_FLAG_MSI_DEVICE; }
+static inline bool irq_domain_is_msi_immutable(struct irq_domain *domain) +{ + return domain->flags & IRQ_DOMAIN_FLAG_MSI_IMMUTABLE; +} #else /* CONFIG_IRQ_DOMAIN_HIERARCHY */ static inline int irq_domain_alloc_irqs(struct irq_domain *domain, unsigned int nr_irqs, int node, void *arg)
On Tue, 11 Feb 2025 19:21:55 +0000, Frank Li Frank.Li@nxp.com wrote:
Add the flag IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and the API function irq_domain_is_msi_immutable() to check if the MSI controller retains an immutable address/data pair during irq_set_affinity().
Ensure compatibility with MSI users like PCIe Endpoint Doorbell, which require the address/data pair to remain unchanged after setup. Use this function to verify if the MSI controller is immutable.
Why is that a requirement? Why should a driver even care?
M.
On Sat, Mar 01, 2025 at 11:10:35AM +0000, Marc Zyngier wrote:
On Tue, 11 Feb 2025 19:21:55 +0000, Frank Li Frank.Li@nxp.com wrote:
Add the flag IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and the API function irq_domain_is_msi_immutable() to check if the MSI controller retains an immutable address/data pair during irq_set_affinity().
Ensure compatibility with MSI users like PCIe Endpoint Doorbell, which require the address/data pair to remain unchanged after setup. Use this function to verify if the MSI controller is immutable.
Why is that a requirement? Why should a driver even care?
At v9, there were detail discussion about this https://lore.kernel.org/all/87v7w0s9a8.ffs@tglx/
let me summary:
Host driver workflow like:
1. read address/data from shared memory (PC bar<n>) 2. write data to address to trigger doorbell.
1 and 2 is not atomic. So EP side may call set_affinity function during 1 and 2, address/data may be changed in some MSI provider, so 2 write to previous address/data pair, which may not existed or map to other place and cause write to unexpected place.
Frank
M.
-- Without deviation from the norm, progress is not possible.
Set the IRQ_DOMAIN_FLAG_MSI_IMMUTABLE flag for ITS, as it does not change the address/data pair after setup.
Ensure compatibility with MSI users, such as PCIe Endpoint Doorbell, which require the address/data pair to remain unchanged. Enable PCIe endpoints to use ITS for triggering doorbells from the PCIe Root Complex (RC) side.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v12 - roll back to v12 version because Marc Zyngier have concern about add DOMAIN_BUS_DEVICE_PCI_EP_MSI. https://lore.kernel.org/imx/861pxfq315.wl-maz@kernel.org/
change from v11 to v12 - new patch --- drivers/irqchip/irq-gic-v3-its.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 8c3ec5734f1ef..7c0a97a1bf8be 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -5126,7 +5126,7 @@ static int its_init_domain(struct its_node *its) irq_domain_update_bus_token(inner_domain, DOMAIN_BUS_NEXUS);
inner_domain->msi_parent_ops = &gic_v3_its_msi_parent_ops; - inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; + inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT | IRQ_DOMAIN_FLAG_MSI_IMMUTABLE;
return 0; }
Some platform devices create child devices dynamically and require the parent device's msi-map to map device IDs to actual sideband information.
A typical use case is using ITS as a PCIe Endpoint Controller(EPC)'s doorbell function, where PCI hosts send TLP memory writes to the EP controller. The EP controller converts these writes to AXI transactions and appends platform-specific sideband information. See below figure.
┌────────────────────────────────┐ │ │ │ PCI Endpoint Controller │ │ │ │ ┌─────┐ ┌─────┐ ┌─────┐ │ PCI Bus │ │ │ │ │ │ │ │ ─────────► │ │Func1│ │Func2│ ... │Func │ │ TLP Memory │ │ │ │ │ │<n> │ │ Write Push │ │ │ │ │ │ │ │ DoorBell │ └─┬─┬─┘ └──┬──┘ └──┬──┘ │ │ │ │ │ │ │ └────┼─┼────────┼───────────┼────┘ sideband │ │ Address│ │ information ▼ ▼ /Data ▼ ▼ ┌────────────────────────┐ │ MSI Controller │ └────────────────────────┘
EPC's DTS will provide such information by msi-map and msi-mask. A simplified dts as
pcie-ep@10000000 { ... msi-map = <0 &its 0xc 8>; ^^^ 0xc is implement defined sideband information, which append to AXI write transaction. ^ 0 is function index.
msi-mask = <0x7> }
Check msi-map if msi-parent missed to keep compatility with existed system.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 new patch --- drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c index e150365fbe892..6c7389bb84a4a 100644 --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c @@ -118,6 +118,14 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev, index++; } while (!ret);
+ if (ret) { + struct device_node *np = NULL; + + ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id); + if (np) + of_node_put(np); + } + return ret; }
On Tue, 11 Feb 2025 19:21:57 +0000, Frank Li Frank.Li@nxp.com wrote:
Some platform devices create child devices dynamically and require the parent device's msi-map to map device IDs to actual sideband information.
A typical use case is using ITS as a PCIe Endpoint Controller(EPC)'s doorbell function, where PCI hosts send TLP memory writes to the EP controller. The EP controller converts these writes to AXI transactions and appends platform-specific sideband information. See below figure.
┌────────────────────────────────┐ │ │ │ PCI Endpoint Controller │ │ │ │ ┌─────┐ ┌─────┐ ┌─────┐ │ PCI Bus │ │ │ │ │ │ │ │ ─────────► │ │Func1│ │Func2│ ... │Func │ │ TLP Memory │ │ │ │ │ │<n> │ │ Write Push │ │ │ │ │ │ │ │ DoorBell │ └─┬─┬─┘ └──┬──┘ └──┬──┘ │ │ │ │ │ │ │ └────┼─┼────────┼───────────┼────┘ sideband │ │ Address│ │ information ▼ ▼ /Data ▼ ▼ ┌────────────────────────┐ │ MSI Controller │ └────────────────────────┘
Please stop using these figures in commit messages. I don't think they help, and they are not in consistent with the way the commit messages are managed.
EPC's DTS will provide such information by msi-map and msi-mask. A simplified dts as
pcie-ep@10000000 { ... msi-map = <0 &its 0xc 8>; ^^^ 0xc is implement defined sideband information, which append to AXI write transaction. ^ 0 is function index.
What does this sideband field represent? How is the ITS driver supposed to use that data? Is it the full devid as presented to the ITS? Something combined with the "function index"? Is the "function index" a full RID, as defined in the documentation?
Also, msi-map is so far reserved to a PCIe RC, not this sort of wonky contraption. This needs to be documented.
msi-mask = <0x7> }
Check msi-map if msi-parent missed to keep compatility with existed system.
Signed-off-by: Frank Li Frank.Li@nxp.com
change from v14 to v15
- none
change from v13 to v14 new patch
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c index e150365fbe892..6c7389bb84a4a 100644 --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c @@ -118,6 +118,14 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev, index++; } while (!ret);
- if (ret) {
struct device_node *np = NULL;
ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
if (np)
of_node_put(np);
- }
- return ret;
}
Thanks,
M.
On Sat, Mar 01, 2025 at 11:37:14AM +0000, Marc Zyngier wrote:
On Tue, 11 Feb 2025 19:21:57 +0000, Frank Li Frank.Li@nxp.com wrote:
Some platform devices create child devices dynamically and require the parent device's msi-map to map device IDs to actual sideband information.
A typical use case is using ITS as a PCIe Endpoint Controller(EPC)'s doorbell function, where PCI hosts send TLP memory writes to the EP controller. The EP controller converts these writes to AXI transactions and appends platform-specific sideband information. See below figure.
┌────────────────────────────────┐ │ │ │ PCI Endpoint Controller │ │ │ │ ┌─────┐ ┌─────┐ ┌─────┐ │ PCI Bus │ │ │ │ │ │ │ │ ─────────► │ │Func1│ │Func2│ ... │Func │ │ TLP Memory │ │ │ │ │ │<n> │ │ Write Push │ │ │ │ │ │ │ │ DoorBell │ └─┬─┬─┘ └──┬──┘ └──┬──┘ │ │ │ │ │ │ │ └────┼─┼────────┼───────────┼────┘ sideband │ │ Address│ │ information ▼ ▼ /Data ▼ ▼ ┌────────────────────────┐ │ MSI Controller │ └────────────────────────┘
Please stop using these figures in commit messages. I don't think they help, and they are not in consistent with the way the commit messages are managed.
Okay
EPC's DTS will provide such information by msi-map and msi-mask. A simplified dts as
pcie-ep@10000000 { ... msi-map = <0 &its 0xc 8>; ^^^ 0xc is implement defined sideband information, which append to AXI write transaction. ^ 0 is function index.
What does this sideband field represent?
ARM ITS use term "streamid" for this sideband field, which indicate which MSI consumer write to address/data pair on bus. Such as PCI1 or PCI2.
How is the ITS driver supposed to use that data?
ITS use a as devid, or info->scratchpad[0].ul = dev_id; msi-map actually given a start dev_id (it is 0xc in example) for fuction0. function1 will use dev_id + 1 ...
Is it the full devid as presented to the ITS?
Yes,
Something combined with the "function index"? Is the "function index" a full RID, as defined in the documentation?
Not a full RID. RID is related with host PCIe's topology. The EP function's RID may 1:00:01 at PC1, 3:00:01 at the another PC.
So Endpoint driver can't use RID directly. It should related function and virtual function number only.
PCI define 8 physicla funciton, and 64000 virutal function. Define device id as vfunc[31:3], pfunc[2:0] as msi-map's input. DTS provide information how map it to real streamid.
Also, msi-map is so far reserved to a PCIe RC, not this sort of wonky contraption. This needs to be documented.
Okay, I can update document.
Frank
msi-mask = <0x7> }
Check msi-map if msi-parent missed to keep compatility with existed system.
Signed-off-by: Frank Li Frank.Li@nxp.com
change from v14 to v15
- none
change from v13 to v14 new patch
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c index e150365fbe892..6c7389bb84a4a 100644 --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c @@ -118,6 +118,14 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev, index++; } while (!ret);
- if (ret) {
struct device_node *np = NULL;
ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
if (np)
of_node_put(np);
- }
- return ret;
}
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
Set device ID as 'vfunc_no << 3 | func_no' and use 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent device.
Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node' settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation functions use the EPC's device node, but they should use the EPF one. For multiple function drivers, IOMMU/MSI should be different for each function driver.
If multiple function devices share the same EPC device, there will be no isolation between them. Setting the ID and 'of_node' prepares for proper support.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 new patch --- drivers/pci/endpoint/pci-epf-core.c | 4 ++++ include/linux/pci-epf.h | 2 ++ 2 files changed, 6 insertions(+)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index 50bc2892a36c5..f728ee2660a4e 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -120,12 +120,16 @@ int pci_epf_bind(struct pci_epf *epf) epf_vf->sec_epc_func_no = epf->sec_epc_func_no; epf_vf->epc = epf->epc; epf_vf->sec_epc = epf->sec_epc; + epf_vf->dev.id = PCI_EPF_DEVID(epf->func_no, vfunc_no); + device_set_of_node_from_dev(&epf_vf->dev, epc->dev.parent); ret = epf_vf->driver->ops->bind(epf_vf); if (ret) goto ret; epf_vf->is_bound = true; }
+ epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0); + device_set_of_node_from_dev(&epf->dev, epc->dev.parent); ret = epf->driver->ops->bind(epf); if (ret) goto ret; diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h index ee6156bcbbd05..d2790b8b29394 100644 --- a/include/linux/pci-epf.h +++ b/include/linux/pci-epf.h @@ -212,6 +212,8 @@ static inline void *epf_get_drvdata(struct pci_epf *epf) return dev_get_drvdata(&epf->dev); }
+#define PCI_EPF_DEVID(func_no, vfunc_no) ((vfunc_no) << 3 | (func_no)) + struct pci_epf *pci_epf_create(const char *name); void pci_epf_destroy(struct pci_epf *epf); int __pci_epf_register_driver(struct pci_epf_driver *driver,
Doorbell feature is implemented by mapping the EP's MSI interrupt controller message address to a dedicated BAR in the EPC core. It is the responsibility of the EPF driver to pass the actual message data to be written by the host to the doorbell BAR region through its own logic.
Tested-by: Niklas Cassel cassel@kernel.org Signed-off-by: Frank Li Frank.Li@nxp.com --- Change from v14 to v15 - check CONFIG_GENERIC_MSI
Fix below build error | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202502082204.6PRR3cfG-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
drivers/pci/endpoint/pci-ep-msi.c:53:15: error: implicit declaration of function 'platform_device_msi_init_and_alloc_irqs' [-Werror=implicit-function-declaration]
53 | ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);
| Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202502082242.pOq1hB1d-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
drivers/pci/endpoint/pci-ep-msi.c:49:14: error: implicit declaration of function 'irq_domain_is_msi_immutable'; did you mean 'irq_domain_is_msi_device'? [-Werror=implicit-function-declaration]
49 | if (!irq_domain_is_msi_immutable(dom)) {
Change from v13 to v14 - basic roll back to v9 - use device:id as msi-map input, its will handle it - use existed platform_device_msi_init_and_alloc_irqs() - pass down epf->dev point, because epf->dev of-node will be the same as epc's parent.
Change from v12 to v13 - Use DOMAIN_BUS_DEVICE_PCI_EP_MSI
Change from v10 to v12 - none
Change from v9 to v10 - Create msi domain for each function device. - Remove only function support limiation. My hardware only support one function, so not test more than one case. - use "msi-map" descript msi information
msi-map = <func_no << 8 | vfunc_no, &its, start_stream_id, size>;
Chagne from v8 to v9 - sort header file - use pci_epc_get(dev_name(msi_desc_to_dev(desc))); - check epf number at pci_epf_alloc_doorbell - Add comments for miss msi-parent case
change from v5 to v8 -none
Change from v4 to v5 - Remove request_irq() in pci_epc_alloc_doorbell() and leave to EP function driver, so ep function driver can register differece call back function for difference doorbell events and set irq affinity to differece CPU core. - Improve error message when MSI allocate failure.
Change from v3 to v4 - msi change to use msi_get_virq() avoid use msi_for_each_desc(). - add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name. - move mutex lock to epc function - initialize variable at declear place. - passdown epf to epc*() function to simplify code. --- drivers/pci/endpoint/Makefile | 1 + drivers/pci/endpoint/pci-ep-msi.c | 82 +++++++++++++++++++++++++++++++++++++++ include/linux/pci-ep-msi.h | 28 +++++++++++++ include/linux/pci-epf.h | 16 ++++++++ 4 files changed, 127 insertions(+)
diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile index 95b2fe47e3b06..c502ea7ef367c 100644 --- a/drivers/pci/endpoint/Makefile +++ b/drivers/pci/endpoint/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS) += pci-ep-cfs.o obj-$(CONFIG_PCI_ENDPOINT) += pci-epc-core.o pci-epf-core.o\ pci-epc-mem.o functions/ +obj-$(CONFIG_GENERIC_MSI_IRQ) += pci-ep-msi.o diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c new file mode 100644 index 0000000000000..549b55b864d0e --- /dev/null +++ b/drivers/pci/endpoint/pci-ep-msi.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * PCI Endpoint *Controller* (EPC) MSI library + * + * Copyright (C) 2025 NXP + * Author: Frank Li Frank.Li@nxp.com + */ + +#include <linux/device.h> +#include <linux/irqdomain.h> +#include <linux/module.h> +#include <linux/msi.h> +#include <linux/of_irq.h> +#include <linux/pci-epc.h> +#include <linux/pci-epf.h> +#include <linux/pci-ep-cfs.h> +#include <linux/pci-ep-msi.h> +#include <linux/slab.h> + +static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg) +{ + struct pci_epf *epf = to_pci_epf(desc->dev); + + if (epf && epf->db_msg && desc->msi_index < epf->num_db) + memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg)); +} + +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db) +{ + struct pci_epc *epc = epf->epc; + struct device *dev = &epf->dev; + struct irq_domain *dom; + void *msg; + u32 rid; + int ret; + int i; + + rid = PCI_EPF_DEVID(epf->func_no, epf->vfunc_no); + dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_MSI); + if (!dom) { + dev_err(dev, "Can't find msi domain\n"); + return -EINVAL; + } + + dev_set_msi_domain(dev, dom); + + msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + epf->num_db = num_db; + epf->db_msg = msg; + + ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg); + if (ret) { + /* + * The pcie_ep DT node has to specify 'msi-parent' for EP + * doorbell support to work. Right now only GIC ITS is + * supported. If you have GIC ITS and reached this print, + * perhaps you are missing 'msi-map' in DT. + */ + dev_err(dev, "Failed to allocate MSI\n"); + kfree(msg); + return -ENOMEM; + } + + for (i = 0; i < num_db; i++) + epf->db_msg[i].virq = msi_get_virq(dev, i); + + return ret; +} +EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell); + +void pci_epf_free_doorbell(struct pci_epf *epf) +{ + platform_device_msi_free_irqs_all(&epf->dev); + + kfree(epf->db_msg); + epf->db_msg = NULL; + epf->num_db = 0; +} +EXPORT_SYMBOL_GPL(pci_epf_free_doorbell); diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h new file mode 100644 index 0000000000000..6dfbe9353f0d8 --- /dev/null +++ b/include/linux/pci-ep-msi.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * PCI Endpoint *Function* side MSI header file + * + * Copyright (C) 2024 NXP + * Author: Frank Li Frank.Li@nxp.com + */ + +#ifndef __PCI_EP_MSI__ +#define __PCI_EP_MSI__ + +struct pci_epf; + +#ifdef CONFIG_GENERIC_MSI_IRQ +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums); +void pci_epf_free_doorbell(struct pci_epf *epf); +#else +static inline int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums) +{ + return -EINVAL; +} + +static inline void pci_epf_free_doorbell(struct pci_epf *epf) +{ +} +#endif /* CONFIG_GENERIC_MSI_IRQ */ + +#endif /* __PCI_EP_MSI__ */ diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h index d2790b8b29394..518bc4171285e 100644 --- a/include/linux/pci-epf.h +++ b/include/linux/pci-epf.h @@ -12,6 +12,7 @@ #include <linux/configfs.h> #include <linux/device.h> #include <linux/mod_devicetable.h> +#include <linux/msi.h> #include <linux/pci.h>
struct pci_epf; @@ -125,6 +126,17 @@ struct pci_epf_bar { int flags; };
+/** + * struct pci_epf_doorbell_msg - represents doorbell message + * @msi_msg: MSI message + * @virq: irq number of this doorbell MSI message + * @name: irq name for doorbell interrupt + */ +struct pci_epf_doorbell_msg { + struct msi_msg msg; + int virq; +}; + /** * struct pci_epf - represents the PCI EPF device * @dev: the PCI EPF device @@ -152,6 +164,8 @@ struct pci_epf_bar { * @vfunction_num_map: bitmap to manage virtual function number * @pci_vepf: list of virtual endpoint functions associated with this function * @event_ops: Callbacks for capturing the EPC events + * @db_msg: data for MSI from RC side + * @num_db: number of doorbells */ struct pci_epf { struct device dev; @@ -182,6 +196,8 @@ struct pci_epf { unsigned long vfunction_num_map; struct list_head pci_vepf; const struct pci_epc_event_ops *event_ops; + struct pci_epf_doorbell_msg *db_msg; + u16 num_db; };
/**
Some MSI controller change address/data pair when irq_set_affinity(). Current PCI endpoint can't support this type MSI controller. So add flag MSI_FLAG_MUTABLE in include/linux/msi.h and check it when allocate doorbell.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - bring v10 back
Change from v9 to v10 - new patch --- drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c index 549b55b864d0e..c0e2d806ee658 100644 --- a/drivers/pci/endpoint/pci-ep-msi.c +++ b/drivers/pci/endpoint/pci-ep-msi.c @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
dev_set_msi_domain(dev, dom);
+ if (!irq_domain_is_msi_parent(dom)) + return -EINVAL; + + if (!irq_domain_is_msi_immutable(dom)) { + dev_err(dev, "Can't support mutable address/data pair MSI controller\n"); + return -EINVAL; + } + msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL); if (!msg) return -ENOMEM;
On Tue, 11 Feb 2025 19:22:00 +0000, Frank Li Frank.Li@nxp.com wrote:
Some MSI controller change address/data pair when irq_set_affinity(). Current PCI endpoint can't support this type MSI controller. So add flag MSI_FLAG_MUTABLE in include/linux/msi.h and check it when allocate doorbell.
Signed-off-by: Frank Li Frank.Li@nxp.com
change from v14 to v15
- none
change from v13 to v14
- bring v10 back
Change from v9 to v10
- new patch
drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c index 549b55b864d0e..c0e2d806ee658 100644 --- a/drivers/pci/endpoint/pci-ep-msi.c +++ b/drivers/pci/endpoint/pci-ep-msi.c @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db) dev_set_msi_domain(dev, dom);
- if (!irq_domain_is_msi_parent(dom))
return -EINVAL;
- if (!irq_domain_is_msi_immutable(dom)) {
dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
return -EINVAL;
- }
- msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL); if (!msg) return -ENOMEM;
I really don't think this brings much to the table. These systems are not built by picking up random bits that can be put together by hand. They are integrated in an SoC, and I can't imagine that the MSI controller is picked randomly.
So my conclusion is that this is either *designed* to work, or it doesn't exist, and that this code is just dead code.
Thanks,
M.
On Sat, Mar 01, 2025 at 11:44:50AM +0000, Marc Zyngier wrote:
On Tue, 11 Feb 2025 19:22:00 +0000, Frank Li Frank.Li@nxp.com wrote:
Some MSI controller change address/data pair when irq_set_affinity(). Current PCI endpoint can't support this type MSI controller. So add flag MSI_FLAG_MUTABLE in include/linux/msi.h and check it when allocate doorbell.
Signed-off-by: Frank Li Frank.Li@nxp.com
change from v14 to v15
- none
change from v13 to v14
- bring v10 back
Change from v9 to v10
- new patch
drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c index 549b55b864d0e..c0e2d806ee658 100644 --- a/drivers/pci/endpoint/pci-ep-msi.c +++ b/drivers/pci/endpoint/pci-ep-msi.c @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
dev_set_msi_domain(dev, dom);
- if (!irq_domain_is_msi_parent(dom))
return -EINVAL;
- if (!irq_domain_is_msi_immutable(dom)) {
dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
return -EINVAL;
- }
- msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL); if (!msg) return -ENOMEM;
I really don't think this brings much to the table. These systems are not built by picking up random bits that can be put together by hand. They are integrated in an SoC, and I can't imagine that the MSI controller is picked randomly.
So my conclusion is that this is either *designed* to work, or it doesn't exist, and that this code is just dead code.
I think Thomas answer this question. This code is actually safty check MSI provider's capablity. If MSI provider can't address/pair is not immutable, there are big risk when irq change affinity.
https://lore.kernel.org/imx/87senwd3mo.ffs@tglx/
"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. "
Let me know if I miss understand your means.
Frank
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
Introduce the helper function pci_epf_align_inbound_addr() to adjust addresses according to PCI BAR alignment requirements, converting addresses into base and offset values.
Signed-off-by: Frank Li Frank.Li@nxp.com --- Change form v14 to v15 - change out address type to dma_addr_t to fix below build issue
| Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202502082311.G1hWGggF-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/endpoint/functions/pci-epf-test.c: In function 'pci_epf_test_enable_doorbell':
drivers/pci/endpoint/functions/pci-epf-test.c:726:42: error: passing argument 4 of 'pci_epf_align_inbound_addr' from incompatible pointer type [-Werror=incompatible-pointer-types]
726 | &epf_test->db_bar.phys_addr, &offset); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | dma_addr_t * {aka unsigned int *} In file included from include/linux/pci-epc.h:12,
Change form v9 to v14 - none
change from v8 to v9 - pci_epf_align_inbound_addr(), base and off must be not NULL - rm pci_epf_align_inbound_addr_lo_hi()
change from v7 to v8 - change name to pci_epf_align_inbound_addr() - update comment said only need for memory, which not allocated by pci_epf_alloc_space().
change from v6 to v7 - new patch --- drivers/pci/endpoint/pci-epf-core.c | 44 +++++++++++++++++++++++++++++++++++++ include/linux/pci-epf.h | 3 +++ 2 files changed, 47 insertions(+)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index f728ee2660a4e..b93984889f736 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -469,6 +469,50 @@ struct pci_epf *pci_epf_create(const char *name) } EXPORT_SYMBOL_GPL(pci_epf_create);
+/** + * pci_epf_align_inbound_addr() - Get base address and offset that match BAR's + * alignment requirement + * @epf: the EPF device + * @addr: the address of the memory + * @bar: the BAR number corresponding to map addr + * @base: return base address, which match BAR's alignment requirement. + * @off: return offset. + * + * Helper function to convert input 'addr' to base and offset, which match + * BAR's alignment requirement. + * + * 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. + */ +int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar, + u64 addr, dma_addr_t *base, size_t *off) +{ + const struct pci_epc_features *epc_features; + u64 align; + + if (!base || !off) + 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); + + return 0; +} +EXPORT_SYMBOL_GPL(pci_epf_align_inbound_addr); + static void pci_epf_dev_release(struct device *dev) { struct pci_epf *epf = to_pci_epf(dev); diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h index 518bc4171285e..a1a1bde1bc2ff 100644 --- a/include/linux/pci-epf.h +++ b/include/linux/pci-epf.h @@ -240,6 +240,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar, enum pci_epc_interface_type type); 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_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);
Add three registers: doorbell_bar, doorbell_addr, and doorbell_data. Use pci_epf_alloc_doorbell() to allocate a doorbell address space.
Enable the Root Complex (RC) side driver to trigger pci-epc-test's doorbell callback handler by writing doorbell_data to the mapped doorbell_bar's address space.
Set STATUS_DOORBELL_SUCCESS in the doorbell callback to indicate completion.
Avoid breaking compatibility between host and endpoint, add new command COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL. Host side need send COMMAND_ENABLE_DOORBELL to map one bar's inbound address to MSI space. the command COMMAND_DISABLE_DOORBELL to recovery original inbound address mapping.
Host side new driver Host side old driver
EP: new driver S F EP: old driver F F
S: If EP side support MSI, 'pci_endpoint_test -f pcie_ep_doorbell' return success. If EP side doesn't support MSI, the same to 'F'.
F: 'pci_endpoint_test -f pcie_ep_doorbell' return failure, other case as usual.
Tested-by: Niklas Cassel cassel@kernel.org Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
Change from v9 to v14 - update commit message by use pci_endpoint_test -f pcie_ep_doorbell
Change from v8 to v9 - move pci_epf_alloc_doorbell() into pci_epf_{enable/disable}_doorbell(). - remove doorbell_done in commit message. - rename pci_epf_{enable/disable}_doorbell() to pci_epf_test_{enable/disable}_doorbell() to align corrent code style.
Change from v7 to v8 - rename to pci_epf_align_inbound_addr_lo_hi()
Change from v6 to v7 - use help function pci_epf_align_addr_lo_hi()
Change from v5 to v6 - rename doorbell_addr to doorbell_offset
Chagne from v4 to v5 - Add doorbell free at unbind function. - Move msi irq handler to here to more complex user case, such as differece doorbell can use difference handler function. - Add Niklas's code to handle fixed bar's case. If need add your signed-off tag or co-developer tag, please let me know.
change from v3 to v4 - remove revid requirement - Add command COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL. - call pci_epc_set_bar() to map inbound address to MSI space only at COMMAND_ENABLE_DOORBELL. --- drivers/pci/endpoint/functions/pci-epf-test.c | 132 ++++++++++++++++++++++++++ 1 file changed, 132 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index b94e205ae10b9..bba1229c46f14 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -11,12 +11,14 @@ #include <linux/dmaengine.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/msi.h> #include <linux/slab.h> #include <linux/pci_ids.h> #include <linux/random.h>
#include <linux/pci-epc.h> #include <linux/pci-epf.h> +#include <linux/pci-ep-msi.h> #include <linux/pci_regs.h>
#define IRQ_TYPE_INTX 0 @@ -29,6 +31,8 @@ #define COMMAND_READ BIT(3) #define COMMAND_WRITE BIT(4) #define COMMAND_COPY BIT(5) +#define COMMAND_ENABLE_DOORBELL BIT(6) +#define COMMAND_DISABLE_DOORBELL BIT(7)
#define STATUS_READ_SUCCESS BIT(0) #define STATUS_READ_FAIL BIT(1) @@ -39,6 +43,11 @@ #define STATUS_IRQ_RAISED BIT(6) #define STATUS_SRC_ADDR_INVALID BIT(7) #define STATUS_DST_ADDR_INVALID BIT(8) +#define STATUS_DOORBELL_SUCCESS BIT(9) +#define STATUS_DOORBELL_ENABLE_SUCCESS BIT(10) +#define STATUS_DOORBELL_ENABLE_FAIL BIT(11) +#define STATUS_DOORBELL_DISABLE_SUCCESS BIT(12) +#define STATUS_DOORBELL_DISABLE_FAIL BIT(13)
#define FLAG_USE_DMA BIT(0)
@@ -63,6 +72,7 @@ struct pci_epf_test { bool dma_supported; bool dma_private; const struct pci_epc_features *epc_features; + struct pci_epf_bar db_bar; };
struct pci_epf_test_reg { @@ -77,6 +87,9 @@ struct pci_epf_test_reg { u32 irq_number; u32 flags; u32 caps; + u32 doorbell_bar; + u32 doorbell_offset; + u32 doorbell_data; } __packed;
static struct pci_epf_header test_header = { @@ -645,6 +658,116 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, } }
+static irqreturn_t pci_epf_test_doorbell_handler(int irq, void *data) +{ + struct pci_epf_test *epf_test = data; + enum pci_barno test_reg_bar = epf_test->test_reg_bar; + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; + + reg->status |= STATUS_DOORBELL_SUCCESS; + pci_epf_test_raise_irq(epf_test, reg); + + return IRQ_HANDLED; +} + +static void pci_epf_test_doorbell_cleanup(struct pci_epf_test *epf_test) +{ + struct pci_epf_test_reg *reg = epf_test->reg[epf_test->test_reg_bar]; + struct pci_epf *epf = epf_test->epf; + + if (reg->doorbell_bar > 0) { + free_irq(epf->db_msg[0].virq, epf_test); + reg->doorbell_bar = NO_BAR; + } + + if (epf->db_msg) + pci_epf_free_doorbell(epf); +} + +static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test, + struct pci_epf_test_reg *reg) +{ + struct pci_epf *epf = epf_test->epf; + struct pci_epc *epc = epf->epc; + struct msi_msg *msg; + enum pci_barno bar; + size_t offset; + int ret; + + ret = pci_epf_alloc_doorbell(epf, 1); + if (ret) { + reg->status |= STATUS_DOORBELL_ENABLE_FAIL; + return; + } + + msg = &epf->db_msg[0].msg; + bar = pci_epc_get_next_free_bar(epf_test->epc_features, epf_test->test_reg_bar + 1); + if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) { + reg->status |= STATUS_DOORBELL_ENABLE_FAIL; + return; + } + + ret = request_irq(epf->db_msg[0].virq, pci_epf_test_doorbell_handler, 0, + "pci-test-doorbell", epf_test); + if (ret) { + dev_err(&epf->dev, + "Failed to request irq %d, doorbell feature is not supported\n", + epf->db_msg[0].virq); + reg->status |= STATUS_DOORBELL_ENABLE_FAIL; + pci_epf_test_doorbell_cleanup(epf_test); + return; + } + + reg->doorbell_data = msg->data; + reg->doorbell_bar = bar; + + msg = &epf->db_msg[0].msg; + ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo, + &epf_test->db_bar.phys_addr, &offset); + + if (ret) { + reg->status |= STATUS_DOORBELL_ENABLE_FAIL; + pci_epf_test_doorbell_cleanup(epf_test); + return; + } + + reg->doorbell_offset = offset; + + epf_test->db_bar.barno = bar; + epf_test->db_bar.size = epf->bar[bar].size; + epf_test->db_bar.flags = epf->bar[bar].flags; + + ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar); + if (ret) { + reg->status |= STATUS_DOORBELL_ENABLE_FAIL; + pci_epf_test_doorbell_cleanup(epf_test); + } else { + reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS; + } +} + +static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test, + struct pci_epf_test_reg *reg) +{ + enum pci_barno bar = reg->doorbell_bar; + struct pci_epf *epf = epf_test->epf; + struct pci_epc *epc = epf->epc; + int ret; + + if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) { + reg->status |= STATUS_DOORBELL_DISABLE_FAIL; + return; + } + + ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]); + if (ret) + reg->status |= STATUS_DOORBELL_DISABLE_FAIL; + else + reg->status |= STATUS_DOORBELL_DISABLE_SUCCESS; + + pci_epf_test_doorbell_cleanup(epf_test); +} + static void pci_epf_test_cmd_handler(struct work_struct *work) { u32 command; @@ -691,6 +814,14 @@ static void pci_epf_test_cmd_handler(struct work_struct *work) pci_epf_test_copy(epf_test, reg); pci_epf_test_raise_irq(epf_test, reg); break; + case COMMAND_ENABLE_DOORBELL: + pci_epf_test_enable_doorbell(epf_test, reg); + pci_epf_test_raise_irq(epf_test, reg); + break; + case COMMAND_DISABLE_DOORBELL: + pci_epf_test_disable_doorbell(epf_test, reg); + pci_epf_test_raise_irq(epf_test, reg); + break; default: dev_err(dev, "Invalid command 0x%x\n", command); break; @@ -953,6 +1084,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf) pci_epf_test_clean_dma_chan(epf_test); pci_epf_test_clear_bar(epf); } + pci_epf_test_doorbell_cleanup(epf_test); pci_epf_test_free_space(epf); }
Add three registers: PCIE_ENDPOINT_TEST_DB_BAR, PCIE_ENDPOINT_TEST_DB_ADDR, and PCIE_ENDPOINT_TEST_DB_DATA.
Trigger the doorbell by writing data from PCI_ENDPOINT_TEST_DB_DATA to the address provided by PCI_ENDPOINT_TEST_DB_OFFSET and wait for endpoint feedback.
Add two command to COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL to enable EP side's doorbell support and avoid compatible problem, which host side driver miss-match with endpoint side function driver. See below table:
Host side new driver Host side old driver EP: new driver S F EP: old driver F F
S: If EP side support MSI, 'pci_endpoint_test -f pcie_ep_doorbell' return success. If EP side doesn't support MSI, the same to 'F'.
F: 'pci_endpoint_test -f pcie_ep_doorbell' return failure, other case as usual.
Tested-by: Niklas Cassel cassel@kernel.org Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
Change from v13 to v14 - update to use pci_endpoint_test -f pcie_ep_doorbell - change ioctrl id to fix conflict
Change from v9 to v13 - none
Change from v8 to v9 - change PCITEST_DOORBELL to 0xa
Change form v6 to v8 - none
Change from v5 to v6 - %s/PCI_ENDPOINT_TEST_DB_ADDR/PCI_ENDPOINT_TEST_DB_OFFSET/g
Change from v4 to v5 - remove unused varible - add irq_type at pci_endpoint_test_doorbell();
change from v3 to v4 - Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL. - Remove new DID requirement. --- drivers/misc/pci_endpoint_test.c | 81 ++++++++++++++++++++++++++++++++++++++++ include/uapi/linux/pcitest.h | 1 + 2 files changed, 82 insertions(+)
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c index d5ac71a493865..7ac021bcc7152 100644 --- a/drivers/misc/pci_endpoint_test.c +++ b/drivers/misc/pci_endpoint_test.c @@ -42,6 +42,8 @@ #define COMMAND_READ BIT(3) #define COMMAND_WRITE BIT(4) #define COMMAND_COPY BIT(5) +#define COMMAND_ENABLE_DOORBELL BIT(6) +#define COMMAND_DISABLE_DOORBELL BIT(7)
#define PCI_ENDPOINT_TEST_STATUS 0x8 #define STATUS_READ_SUCCESS BIT(0) @@ -53,6 +55,11 @@ #define STATUS_IRQ_RAISED BIT(6) #define STATUS_SRC_ADDR_INVALID BIT(7) #define STATUS_DST_ADDR_INVALID BIT(8) +#define STATUS_DOORBELL_SUCCESS BIT(9) +#define STATUS_DOORBELL_ENABLE_SUCCESS BIT(10) +#define STATUS_DOORBELL_ENABLE_FAIL BIT(11) +#define STATUS_DOORBELL_DISABLE_SUCCESS BIT(12) +#define STATUS_DOORBELL_DISABLE_FAIL BIT(13)
#define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR 0x0c #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR 0x10 @@ -67,11 +74,16 @@ #define PCI_ENDPOINT_TEST_IRQ_NUMBER 0x28
#define PCI_ENDPOINT_TEST_FLAGS 0x2c + #define FLAG_USE_DMA BIT(0)
#define PCI_ENDPOINT_TEST_CAPS 0x30 #define CAP_UNALIGNED_ACCESS BIT(0)
+#define PCI_ENDPOINT_TEST_DB_BAR 0x34 +#define PCI_ENDPOINT_TEST_DB_OFFSET 0x38 +#define PCI_ENDPOINT_TEST_DB_DATA 0x3c + #define PCI_DEVICE_ID_TI_AM654 0xb00c #define PCI_DEVICE_ID_TI_J7200 0xb00f #define PCI_DEVICE_ID_TI_AM64 0xb010 @@ -111,6 +123,7 @@ enum pci_barno { BAR_3, BAR_4, BAR_5, + NO_BAR = -1, };
struct pci_endpoint_test { @@ -829,6 +842,71 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test, return 0; }
+static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test) +{ + struct pci_dev *pdev = test->pdev; + struct device *dev = &pdev->dev; + int irq_type = test->irq_type; + enum pci_barno bar; + u32 data, status; + u32 addr; + + if (irq_type < IRQ_TYPE_INTX || irq_type > IRQ_TYPE_MSIX) { + dev_err(dev, "Invalid IRQ type option\n"); + return -EINVAL; + } + + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type); + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1); + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, + COMMAND_ENABLE_DOORBELL); + + wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000)); + + status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS); + if (status & STATUS_DOORBELL_ENABLE_FAIL) { + dev_err(dev, "Failed to enable doorbell\n"); + return -EINVAL; + } + + data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA); + addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET); + bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR); + + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type); + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1); + + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0); + + bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR); + + writel(data, test->bar[bar] + addr); + + wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000)); + + status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS); + + if (!(status & STATUS_DOORBELL_SUCCESS)) + dev_err(dev, "Endpoint have not received Doorbell\n"); + + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, + COMMAND_DISABLE_DOORBELL); + + wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000)); + + status |= pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS); + + if (status & STATUS_DOORBELL_DISABLE_FAIL) { + dev_err(dev, "Failed to disable doorbell\n"); + return -EINVAL; + } + + if (!(status & STATUS_DOORBELL_SUCCESS)) + return -EINVAL; + + return 0; +} + static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -879,6 +957,9 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, case PCITEST_CLEAR_IRQ: ret = pci_endpoint_test_clear_irq(test); break; + case PCITEST_DOORBELL: + ret = pci_endpoint_test_doorbell(test); + break; }
ret: diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h index acd261f498666..80f4c0e05679b 100644 --- a/include/uapi/linux/pcitest.h +++ b/include/uapi/linux/pcitest.h @@ -21,6 +21,7 @@ #define PCITEST_SET_IRQTYPE _IOW('P', 0x8, int) #define PCITEST_GET_IRQTYPE _IO('P', 0x9) #define PCITEST_BARS _IO('P', 0xa) +#define PCITEST_DOORBELL _IO('P', 0xb) #define PCITEST_CLEAR_IRQ _IO('P', 0x10)
#define PCITEST_FLAGS_USE_DMA 0x00000001
Add doorbell test case.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - merge to selftests framework --- .../selftests/pci_endpoint/pci_endpoint_test.c | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c index c267b822c1081..ed14c9fa073f9 100644 --- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c +++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c @@ -218,4 +218,29 @@ TEST_F(pci_ep_data_transfer, COPY_TEST) test_size[i]); } } + +FIXTURE(pcie_ep_doorbell) +{ + int fd; +}; + +FIXTURE_SETUP(pcie_ep_doorbell) +{ + self->fd = open(test_device, O_RDWR); + + ASSERT_NE(-1, self->fd) TH_LOG("Can't open PCI Endpoint Test device"); +}; + +FIXTURE_TEARDOWN(pcie_ep_doorbell) +{ + close(self->fd); +}; + +TEST_F(pcie_ep_doorbell, DOORBELL_TEST) +{ + int ret; + + pci_ep_ioctl(PCITEST_DOORBELL, 0); + EXPECT_FALSE(ret) TH_LOG("Test failed for Doorbell\n"); +} TEST_HARNESS_MAIN
Add helper function imx_pcie_add_lut_by_rid(), which will be used for Endpoint mode in the future. No functional change.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - new patch --- drivers/pci/controller/dwc/pci-imx6.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 90ace941090f9..e20d91988c718 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -1028,18 +1028,14 @@ static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid) } }
-static int imx_pcie_enable_device(struct pci_host_bridge *bridge, - struct pci_dev *pdev) +static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid) { - struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata)); - u32 sid_i, sid_m, rid = pci_dev_id(pdev); + struct device *dev = imx_pcie->pci->dev; struct device_node *target; - struct device *dev; + u32 sid_i, sid_m; int err_i, err_m; u32 sid = 0;
- dev = imx_pcie->pci->dev; - target = NULL; err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask", &target, &sid_i); @@ -1114,6 +1110,13 @@ static int imx_pcie_enable_device(struct pci_host_bridge *bridge, return imx_pcie_add_lut(imx_pcie, rid, sid); }
+static int imx_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev) +{ + struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata)); + + return imx_pcie_add_lut_by_rid(imx_pcie, pci_dev_id(pdev)); +} + static void imx_pcie_disable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev) {
Support only one physical function, so call imx_pcie_add_lut_by_rid(0) to add a single LUT entry when operating in EP mode.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - new patch --- drivers/pci/controller/dwc/pci-imx6.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index e20d91988c718..adcca3e644332 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -995,7 +995,10 @@ static int imx_pcie_add_lut(struct imx_pcie *imx_pcie, u16 rid, u8 sid) data1 |= IMX95_PE0_LUT_VLD; regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, data1);
- data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */ + if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) + data2 = 0x7; /* EP side's RID from RC, only 'D' is meansful */ + else + data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */ data2 |= FIELD_PREP(IMX95_PE0_LUT_REQID, rid); regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, data2);
@@ -1652,6 +1655,9 @@ static int imx_pcie_probe(struct platform_device *pdev) ret = imx_add_pcie_ep(imx_pcie, pdev); if (ret < 0) return ret; + + /* Only support one physical function */ + imx_pcie_add_lut_by_rid(imx_pcie, 0); } else { pci->pp.use_atu_msg = true; ret = dw_pcie_host_init(&pci->pp);
Add msi-map for pci-ep device.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - new patch --- arch/arm64/boot/dts/freescale/imx95.dtsi | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi index 6b8470cb3461a..cdc06dea5982b 100644 --- a/arch/arm64/boot/dts/freescale/imx95.dtsi +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi @@ -1667,6 +1667,7 @@ pcie1_ep: pcie-ep@4c380000 { assigned-clock-rates = <3600000000>, <100000000>, <10000000>; assigned-clock-parents = <0>, <0>, <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>; + msi-map = <0x0 &its 0x98 0x1>; power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>; status = "disabled"; };
Add an overlay file to configure PCIe1 to function as an endpoint. Enable PCIe1 to work as endpoint mode on the imx95-19x19-evk platform.
Signed-off-by: Frank Li Frank.Li@nxp.com --- change from v14 to v15 - none
change from v13 to v14 - new patch --- arch/arm64/boot/dts/freescale/Makefile | 3 +++ .../dts/freescale/imx95-19x19-evk-pcie1-ep.dtso | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile index 839432153cc7a..d892303fa7eab 100644 --- a/arch/arm64/boot/dts/freescale/Makefile +++ b/arch/arm64/boot/dts/freescale/Makefile @@ -287,6 +287,9 @@ imx8mm-kontron-dl-dtbs := imx8mm-kontron-bl.dtb imx8mm-kontron-dl.dtbo
dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-dl.dtb
+imx95-19x19-evk-pcie1-ep-dtbs += imx95-19x19-evk.dtb imx95-19x19-evk-pcie1-ep.dtbo +dtb-$(CONFIG_ARCH_MXC) += imx95-19x19-evk-pcie1-ep.dtb + imx8mm-venice-gw72xx-0x-imx219-dtbs := imx8mm-venice-gw72xx-0x.dtb imx8mm-venice-gw72xx-0x-imx219.dtbo imx8mm-venice-gw72xx-0x-rpidsi-dtbs := imx8mm-venice-gw72xx-0x.dtb imx8mm-venice-gw72xx-0x-rpidsi.dtbo imx8mm-venice-gw72xx-0x-rs232-rts-dtbs := imx8mm-venice-gw72xx-0x.dtb imx8mm-venice-gw72xx-0x-rs232-rts.dtbo diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk-pcie1-ep.dtso b/arch/arm64/boot/dts/freescale/imx95-19x19-evk-pcie1-ep.dtso new file mode 100644 index 0000000000000..696588e768e61 --- /dev/null +++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk-pcie1-ep.dtso @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright 2024 NXP + */ + +#include <dt-bindings/gpio/gpio.h> + +/dts-v1/; +/plugin/; + +&pcie1 { + status = "disabled"; +}; + +&pcie1_ep { + pinctrl-0 = <&pinctrl_pcie0>; + pinctrl-names = "default"; + reset-gpio = <&i2c7_pcal6524 16 GPIO_ACTIVE_LOW>; + vpcie-supply = <®_slot_pwr>; + status = "okay"; +};
On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote:
Thomas Gleixner and Marc Zyngier:
Do you have any comments about irq/msi part?
Frank
┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐ │ │ │ │ │ │ │ │ │ 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 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.
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.
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)
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 (15): platform-msi: Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all() irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable() irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS irqchip/gic-v3-its: Add support for device tree msi-map and msi-mask PCI: endpoint: Set ID and of_node for function driver 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 pci: imx6: Add helper function imx_pcie_add_lut_by_rid() pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode arm64: dts: imx95: Add msi-map for pci-ep device arm64: dts: imx95-19x19-evk: Add PCIe1 endpoint function overlay file
arch/arm64/boot/dts/freescale/Makefile | 3 + .../dts/freescale/imx95-19x19-evk-pcie1-ep.dtso | 21 ++++ arch/arm64/boot/dts/freescale/imx95.dtsi | 1 + drivers/base/platform-msi.c | 1 + drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++ drivers/irqchip/irq-gic-v3-its.c | 2 +- drivers/misc/pci_endpoint_test.c | 81 +++++++++++++ drivers/pci/controller/dwc/pci-imx6.c | 25 ++-- drivers/pci/endpoint/Makefile | 1 + drivers/pci/endpoint/functions/pci-epf-test.c | 132 +++++++++++++++++++++ drivers/pci/endpoint/pci-ep-msi.c | 90 ++++++++++++++ drivers/pci/endpoint/pci-epf-core.c | 48 ++++++++ include/linux/irqdomain.h | 7 ++ include/linux/pci-ep-msi.h | 28 +++++ include/linux/pci-epf.h | 21 ++++ include/uapi/linux/pcitest.h | 1 + .../selftests/pci_endpoint/pci_endpoint_test.c | 25 ++++ 17 files changed, 486 insertions(+), 9 deletions(-)
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20241010-ep-msi-8b4cab33b1be
Best regards,
Frank Li Frank.Li@nxp.com
On Thu, Feb 20 2025 at 15:01, Frank Li wrote:
On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote:
Thomas Gleixner and Marc Zyngier:
Do you have any comments about irq/msi part?
I'm not having objections, but this needs to be acked by Marc as he had pretty strong opinions on the overall approach.
Thanks,
tglx
On Thu, Feb 20, 2025 at 09:42:57PM +0100, Thomas Gleixner wrote:
On Thu, Feb 20 2025 at 15:01, Frank Li wrote:
On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote:
Thomas Gleixner and Marc Zyngier:
Do you have any comments about irq/msi part?
I'm not having objections, but this needs to be acked by Marc as he had pretty strong opinions on the overall approach.
Marc Zyngier:
Do you have any concern about irq/msi part? Is it clean enough to answer what your conern at previous version?
How can we move forward? This help PCI EP side avoid software polling status.
Frank
Thanks,
tglx
On Thu, 20 Feb 2025 20:01:10 +0000, Frank Li Frank.li@nxp.com wrote:
On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote:
Thomas Gleixner and Marc Zyngier:
Do you have any comments about irq/msi part?
It certainly looks better and less invasive than the previous incarnations. Things to fix:
- Documentation: the msi-map property usage is undefined outside of a PCIe RC, and the way you describe its use is so vague I read anything in it. Please update Documentation/devicetree/bindings/pci/pci-msi.txt to reflect the new use case.
- 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.
Thanks,
M.
On Sat, Mar 01 2025 at 12:02, Marc Zyngier wrote:
- 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
linux-kselftest-mirror@lists.linaro.org