* [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
@ 2024-12-18 23:08 Frank Li
2024-12-18 23:08 ` [PATCH v13 1/9] genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI Frank Li
` (8 more replies)
0 siblings, 9 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
│ │ │ │ │ │
│ │ │ 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 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.
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Krzysztof Wilczyński <kw@linux.com>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>
To: Arnd Bergmann <arnd@arndb.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: Niklas Cassel <cassel@kernel.org>
Cc: dlemoal@kernel.org
Cc: jdmason@kudzu.us
To: Rafael J. Wysocki <rafael@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
To: Anup Patel <apatel@ventanamicro.com>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: Marc Zyngier <maz@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (9):
genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI
PCI: EP: Add helper function pci_epf_msi_domain_get_msi_rid()
irqchip/gic-v3-its: Add helper function its_pmsi_prepare_devid()
irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
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
tools: PCI: Add 'B' option for test doorbell
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 49 ++++++---
drivers/irqchip/irq-msi-lib.c | 4 +
drivers/irqchip/irq-msi-lib.h | 6 ++
drivers/misc/pci_endpoint_test.c | 80 +++++++++++++++
drivers/pci/endpoint/Makefile | 2 +-
drivers/pci/endpoint/functions/pci-epf-test.c | 132 ++++++++++++++++++++++++
drivers/pci/endpoint/pci-ep-msi.c | 140 ++++++++++++++++++++++++++
drivers/pci/endpoint/pci-epf-core.c | 44 ++++++++
include/linux/irqdomain_defs.h | 2 +
include/linux/pci-ep-msi.h | 26 +++++
include/linux/pci-epf.h | 19 ++++
include/uapi/linux/pcitest.h | 1 +
tools/pci/pcitest.c | 16 ++-
13 files changed, 507 insertions(+), 14 deletions(-)
---
base-commit: fb32deaabac5c9ac8e555fad420113348cea8bc2
change-id: 20241010-ep-msi-8b4cab33b1be
Best regards,
---
Frank Li <Frank.Li@nxp.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v13 1/9] genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 2/9] PCI: EP: Add helper function pci_epf_msi_domain_get_msi_rid() Frank Li
` (7 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
┌────────────────────────────────┐
│ │
│ PCI Endpoint Controller │
│ │
│ ┌─────┐ ┌─────┐ ┌─────┐ │
PCI Bus │ │ │ │ │ │ │ │
─────────► │ │Func1│ │Func2│ ... │Func │ │
Doorbell │ │ │ │ │ │<n> │ │
│ │ │ │ │ │ │ │
│ └──┬──┘ └──┬──┘ └──┬──┘ │
│ │ │ │ │
└──────┼────────┼───────────┼────┘
│ │ │
▼ ▼ ▼
┌────────────────────────┐
│ MSI Controller │
└────────────────────────┘
Add domain BUS_PLATFORM_PCI_EP_MSI to allocate MSI domain for Endpoint
function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
write MSI message to MSI controller to trigger doorbell IRQ for difference
EP functions.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v12 to v13
- new patch
---
drivers/irqchip/irq-msi-lib.c | 4 ++++
drivers/irqchip/irq-msi-lib.h | 6 ++++++
include/linux/irqdomain_defs.h | 2 ++
3 files changed, 12 insertions(+)
diff --git a/drivers/irqchip/irq-msi-lib.c b/drivers/irqchip/irq-msi-lib.c
index d8e29fc0d4068..cf39f2e481477 100644
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -57,6 +57,10 @@ bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
return false;
break;
+ case DOMAIN_BUS_DEVICE_PCI_EP_MSI:
+ if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_ENDPOINT)))
+ return false;
+ fallthrough;
case DOMAIN_BUS_DEVICE_MSI:
/*
* Per device MSI should never have any MSI feature bits
diff --git a/drivers/irqchip/irq-msi-lib.h b/drivers/irqchip/irq-msi-lib.h
index 681ceabb7bc74..5ccfdb14fce1d 100644
--- a/drivers/irqchip/irq-msi-lib.h
+++ b/drivers/irqchip/irq-msi-lib.h
@@ -17,6 +17,12 @@
#define MATCH_PLATFORM_MSI BIT(DOMAIN_BUS_PLATFORM_MSI)
+#ifdef CONFIG_PCI_ENDPOINT
+#define MATCH_PLATFORM_PCI_EP_MSI BIT(DOMAIN_BUS_PLATFORM_PCI_EP_MSI)
+#else
+#define MATCH_PLATFORM_PCI_EP_MSI (0)
+#endif
+
int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
enum irq_domain_bus_token bus_token);
diff --git a/include/linux/irqdomain_defs.h b/include/linux/irqdomain_defs.h
index 36653e2ee1c92..feecbc27c2575 100644
--- a/include/linux/irqdomain_defs.h
+++ b/include/linux/irqdomain_defs.h
@@ -15,6 +15,7 @@ enum irq_domain_bus_token {
DOMAIN_BUS_GENERIC_MSI,
DOMAIN_BUS_PCI_MSI,
DOMAIN_BUS_PLATFORM_MSI,
+ DOMAIN_BUS_PLATFORM_PCI_EP_MSI,
DOMAIN_BUS_NEXUS,
DOMAIN_BUS_IPI,
DOMAIN_BUS_FSL_MC_MSI,
@@ -27,6 +28,7 @@ enum irq_domain_bus_token {
DOMAIN_BUS_AMDVI,
DOMAIN_BUS_DEVICE_MSI,
DOMAIN_BUS_WIRED_TO_MSI,
+ DOMAIN_BUS_DEVICE_PCI_EP_MSI,
};
#endif /* _LINUX_IRQDOMAIN_DEFS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 2/9] PCI: EP: Add helper function pci_epf_msi_domain_get_msi_rid()
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
2024-12-18 23:08 ` [PATCH v13 1/9] genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 3/9] irqchip/gic-v3-its: Add helper function its_pmsi_prepare_devid() Frank Li
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
Add helper function pci_epf_msi_domain_get_msi_rid() to convert EP device
id to msi domain's request ID to pave the road to support RC-to-EP doorbell
with platform MSI controller.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v12 to v13
- new patch
---
drivers/pci/endpoint/Makefile | 2 +-
drivers/pci/endpoint/pci-ep-msi.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/pci-ep-msi.h | 21 +++++++++++++++++++++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
index 95b2fe47e3b06..a1ccce440c2c5 100644
--- a/drivers/pci/endpoint/Makefile
+++ b/drivers/pci/endpoint/Makefile
@@ -5,4 +5,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/
+ pci-epc-mem.o pci-ep-msi.o functions/
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
new file mode 100644
index 0000000000000..7aedc1cafbd14
--- /dev/null
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCI Endpoint *Controller* (EPC) MSI library
+ *
+ * Copyright (C) 2024 NXP
+ * Author: Frank Li <Frank.Li@nxp.com>
+ */
+
+#include <linux/device.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>
+
+int pci_epf_msi_domain_get_msi_rid(struct device *dev, u32 *rid)
+{
+ struct pci_epf *epf = to_pci_epf(dev);
+ struct pci_epc *epc = epf->epc;
+
+ if (!rid)
+ return -EINVAL;
+
+ /*
+ * PCI Endpoint device ID can't full reuse PCI's BDF, BUS number is
+ * Root Complex Bus number, which is no means for EP side. Move func_no
+ * as low 3 bits to partially match PCI's BDF.
+ */
+ *rid = of_msi_map_id(epc->dev.parent, NULL, (epf->func_no & 0x7) | (epf->vfunc_no << 3));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_msi_domain_get_msi_rid);
diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
new file mode 100644
index 0000000000000..75236867426a4
--- /dev/null
+++ b/include/linux/pci-ep-msi.h
@@ -0,0 +1,21 @@
+/* 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__
+
+#ifdef CONFIG_PCI_ENDPOINT
+int pci_epf_msi_domain_get_msi_rid(struct device *dev, u32 *rid);
+#else
+static inline int pci_epf_msi_domain_get_msi_rid(struct device *dev, u32 *rid)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* __PCI_EP_MSI__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 3/9] irqchip/gic-v3-its: Add helper function its_pmsi_prepare_devid()
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
2024-12-18 23:08 ` [PATCH v13 1/9] genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI Frank Li
2024-12-18 23:08 ` [PATCH v13 2/9] PCI: EP: Add helper function pci_epf_msi_domain_get_msi_rid() Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support Frank Li
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
Add helper function its_pmsi_prepare_devid() to pave road to support new
DOMAIN_BUS_DEVICE_PCI_EP_MSI. No function change.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v12 to v13
- new patch
---
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 30 ++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 75aa0d4bd1346..b2a4b67545b82 100644
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -132,19 +132,10 @@ int __weak iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
return -1;
}
-static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
- int nvec, msi_alloc_info_t *info)
+static int its_pmsi_prepare_devid(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info, u32 dev_id)
{
struct msi_domain_info *msi_info;
- u32 dev_id;
- int ret;
-
- if (dev->of_node)
- ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
- else
- ret = iort_pmsi_get_dev_id(dev, &dev_id);
- if (ret)
- return ret;
/* ITS specific DeviceID, as the core ITS ignores dev. */
info->scratchpad[0].ul = dev_id;
@@ -165,6 +156,23 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
dev, nvec, info);
}
+static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
+{
+ u32 dev_id;
+ int ret;
+
+ if (dev->of_node)
+ ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
+ else
+ ret = iort_pmsi_get_dev_id(dev, &dev_id);
+
+ if (ret)
+ return ret;
+
+ return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
+}
+
static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
struct irq_domain *real_parent, struct msi_domain_info *info)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (2 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 3/9] irqchip/gic-v3-its: Add helper function its_pmsi_prepare_devid() Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-19 10:52 ` Marc Zyngier
2024-12-18 23:08 ` [PATCH v13 5/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
` (4 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
┌────────────────────────────────┐
│ │
│ PCI Endpoint Controller │
│ │
│ ┌─────┐ ┌─────┐ ┌─────┐ │
PCI Bus │ │ │ │ │ │ │ │
─────────► │ │Func1│ │Func2│ ... │Func │ │
Doorbell │ │ │ │ │ │<n> │ │
│ │ │ │ │ │ │ │
│ └──┬──┘ └──┬──┘ └──┬──┘ │
│ │ │ │ │
└──────┼────────┼───────────┼────┘
│ │ │
▼ ▼ ▼
┌────────────────────────┐
│ MSI Controller │
└────────────────────────┘
Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
write MSI message to MSI controller to trigger doorbell IRQ for difference
EP functions.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v12 to v13
- new patch
---
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index b2a4b67545b82..16e7d53f0b133 100644
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -5,6 +5,7 @@
// Copyright (C) 2022 Intel
#include <linux/acpi_iort.h>
+#include <linux/pci-ep-msi.h>
#include <linux/pci.h>
#include "irq-gic-common.h"
@@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
}
+static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
+{
+ u32 dev_id;
+ int ret;
+
+ ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
+ if (ret)
+ return ret;
+
+ return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
+}
+
static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
struct irq_domain *real_parent, struct msi_domain_info *info)
{
@@ -205,6 +219,9 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
*/
info->ops->msi_prepare = its_pmsi_prepare;
break;
+ case DOMAIN_BUS_DEVICE_PCI_EP_MSI:
+ info->ops->msi_prepare = its_pci_ep_msi_prepare;
+ break;
default:
/* Confused. How did the lib return true? */
WARN_ON_ONCE(1);
@@ -218,7 +235,7 @@ const struct msi_parent_ops gic_v3_its_msi_parent_ops = {
.supported_flags = ITS_MSI_FLAGS_SUPPORTED,
.required_flags = ITS_MSI_FLAGS_REQUIRED,
.bus_select_token = DOMAIN_BUS_NEXUS,
- .bus_select_mask = MATCH_PCI_MSI | MATCH_PLATFORM_MSI,
+ .bus_select_mask = MATCH_PCI_MSI | MATCH_PLATFORM_MSI | MATCH_PLATFORM_PCI_EP_MSI,
.prefix = "ITS-",
.init_dev_msi_info = its_init_dev_msi_info,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 5/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (3 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
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 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/pci-ep-msi.c | 104 ++++++++++++++++++++++++++++++++++++++
include/linux/pci-ep-msi.h | 5 ++
include/linux/pci-epf.h | 16 ++++++
3 files changed, 125 insertions(+)
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
index 7aedc1cafbd14..33503d86cf2bc 100644
--- a/drivers/pci/endpoint/pci-ep-msi.c
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -34,3 +34,107 @@ int pci_epf_msi_domain_get_msi_rid(struct device *dev, u32 *rid)
return 0;
}
EXPORT_SYMBOL_GPL(pci_epf_msi_domain_get_msi_rid);
+
+static void pci_epf_write_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+ struct msi_desc *desc = irq_data_get_msi_desc(d);
+ 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));
+}
+
+static void pci_epf_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
+{
+ arg->desc = desc;
+ arg->hwirq = desc->msi_index;
+}
+
+static const struct msi_domain_template pci_epf_msi_template = {
+ .chip = {
+ .name = "EP-MSI",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_write_msi_msg = pci_epf_write_msi_msg,
+ /* The rest is filled in by the MSI parent */
+ },
+
+ .ops = {
+ .set_desc = pci_epf_msi_set_desc,
+ },
+
+ .info = {
+ .bus_token = DOMAIN_BUS_DEVICE_PCI_EP_MSI,
+ },
+};
+
+static int pci_epf_device_msi_init_and_alloc_irqs(struct device *dev, unsigned int nvec)
+{
+ struct irq_domain *domain = dev->msi.domain;
+
+ if (!domain)
+ return -EINVAL;
+
+ if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN,
+ &pci_epf_msi_template, nvec, NULL, NULL))
+ return -ENODEV;
+
+ return msi_domain_alloc_irqs_range(dev, MSI_DEFAULT_DOMAIN, 0, nvec - 1);
+}
+
+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 = (epf->func_no & 0x7) | (epf->vfunc_no << 3);
+ dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_PCI_EP_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 = pci_epf_device_msi_init_and_alloc_irqs(dev, num_db);
+ 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)
+{
+ msi_domain_free_irqs_all(&epf->dev, MSI_DEFAULT_DOMAIN);
+ msi_remove_device_irq_domain(&epf->dev, MSI_DEFAULT_DOMAIN);
+
+ 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
index 75236867426a4..26b1c86893ee4 100644
--- a/include/linux/pci-ep-msi.h
+++ b/include/linux/pci-ep-msi.h
@@ -18,4 +18,9 @@ static inline int pci_epf_msi_domain_get_msi_rid(struct device *dev, u32 *rid)
}
#endif
+struct pci_epf;
+
+int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums);
+void pci_epf_free_doorbell(struct pci_epf *epf);
+
#endif /* __PCI_EP_MSI__ */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 18a3aeb62ae4e..5374e6515ffa0 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;
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (4 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 5/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
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 v9 to v13
- 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 8fa2797d4169a..d7a80f9c1e661 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -464,6 +464,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, u64 *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 5374e6515ffa0..2847d195433bf 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -238,6 +238,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, u64 *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);
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (5 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
2024-12-18 23:08 ` [PATCH v13 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
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, 'pcitest -B' return success.
If EP side doesn't support MSI, the same to 'F'.
F: 'pcitest -B' return failure, other case as usual.
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v9 to v13
- none
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 ef6677f34116e..a0a0e86a081cb 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)
@@ -74,6 +83,9 @@ struct pci_epf_test_reg {
u32 irq_type;
u32 irq_number;
u32 flags;
+ u32 doorbell_bar;
+ u32 doorbell_offset;
+ u32 doorbell_data;
} __packed;
static struct pci_epf_header test_header = {
@@ -642,6 +654,117 @@ 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_epf_bar db_bar = {};
+ 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,
+ &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;
+
+ db_bar.barno = bar;
+ db_bar.size = epf->bar[bar].size;
+ db_bar.flags = epf->bar[bar].flags;
+
+ ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &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;
@@ -688,6 +811,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;
@@ -934,6 +1065,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);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 8/9] misc: pci_endpoint_test: Add doorbell test case
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (6 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
@ 2024-12-18 23:08 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
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, 'pcitest -B' return success.
If EP side doesn't support MSI, the same to 'F'.
F: 'pcitest -B' return failure, other case as usual.
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
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 | 80 ++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/pcitest.h | 1 +
2 files changed, 81 insertions(+)
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 3aaaf47fa4ee2..b3f36b6ba8ba2 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,6 +74,10 @@
#define PCI_ENDPOINT_TEST_IRQ_NUMBER 0x28
#define PCI_ENDPOINT_TEST_FLAGS 0x2c
+#define PCI_ENDPOINT_TEST_DB_BAR 0x30
+#define PCI_ENDPOINT_TEST_DB_OFFSET 0x34
+#define PCI_ENDPOINT_TEST_DB_DATA 0x38
+
#define FLAG_USE_DMA BIT(0)
#define PCI_DEVICE_ID_TI_AM654 0xb00c
@@ -108,6 +119,7 @@ enum pci_barno {
BAR_3,
BAR_4,
BAR_5,
+ NO_BAR = -1,
};
struct pci_endpoint_test {
@@ -746,6 +758,71 @@ static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
return false;
}
+static bool 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 false;
+ }
+
+ 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 false;
+ }
+
+ 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 false;
+ }
+
+ if (!(status & STATUS_DOORBELL_SUCCESS))
+ return false;
+
+ return true;
+}
+
static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -793,6 +870,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 94b46b043b536..b82e7f2ed937d 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -20,6 +20,7 @@
#define PCITEST_MSIX _IOW('P', 0x7, int)
#define PCITEST_SET_IRQTYPE _IOW('P', 0x8, int)
#define PCITEST_GET_IRQTYPE _IO('P', 0x9)
+#define PCITEST_DOORBELL _IO('P', 0xa)
#define PCITEST_CLEAR_IRQ _IO('P', 0x10)
#define PCITEST_FLAGS_USE_DMA 0x00000001
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v13 9/9] tools: PCI: Add 'B' option for test doorbell
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
` (7 preceding siblings ...)
2024-12-18 23:08 ` [PATCH v13 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
@ 2024-12-18 23:08 ` Frank Li
8 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-18 23:08 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, Marc Zyngier
Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
linux-arm-kernel, Frank Li
Add doorbell test support.
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v13
- none
---
tools/pci/pcitest.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 7b530d838d408..fcff0224a3381 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -34,6 +34,7 @@ struct pci_test {
bool copy;
unsigned long size;
bool use_dma;
+ bool doorbell;
};
static int run_test(struct pci_test *test)
@@ -147,6 +148,15 @@ static int run_test(struct pci_test *test)
fprintf(stdout, "%s\n", result[ret]);
}
+ if (test->doorbell) {
+ ret = ioctl(fd, PCITEST_DOORBELL, 0);
+ fprintf(stdout, "Ringing doorbell on the EP\t\t");
+ if (ret < 0)
+ fprintf(stdout, "TEST FAILED\n");
+ else
+ fprintf(stdout, "%s\n", result[ret]);
+ }
+
fflush(stdout);
close(fd);
return (ret < 0) ? ret : 1 - ret; /* return 0 if test succeeded */
@@ -172,7 +182,7 @@ int main(int argc, char **argv)
/* set default endpoint device */
test->device = "/dev/pci-endpoint-test.0";
- while ((c = getopt(argc, argv, "D:b:m:x:i:deIlhrwcs:")) != EOF)
+ while ((c = getopt(argc, argv, "D:b:m:x:i:BdeIlhrwcs:")) != EOF)
switch (c) {
case 'D':
test->device = optarg;
@@ -222,6 +232,9 @@ int main(int argc, char **argv)
case 'd':
test->use_dma = true;
continue;
+ case 'B':
+ test->doorbell = true;
+ continue;
case 'h':
default:
usage:
@@ -241,6 +254,7 @@ int main(int argc, char **argv)
"\t-w Write buffer test\n"
"\t-c Copy buffer test\n"
"\t-s <size> Size of buffer {default: 100KB}\n"
+ "\t-B Doorbell test\n"
"\t-h Print this help message\n",
argv[0]);
return -EINVAL;
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-18 23:08 ` [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support Frank Li
@ 2024-12-19 10:52 ` Marc Zyngier
2024-12-19 17:02 ` Frank Li
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2024-12-19 10:52 UTC (permalink / raw)
To: Frank Li
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Wed, 18 Dec 2024 23:08:39 +0000,
Frank Li <Frank.Li@nxp.com> wrote:
>
> ┌────────────────────────────────┐
> │ │
> │ PCI Endpoint Controller │
> │ │
> │ ┌─────┐ ┌─────┐ ┌─────┐ │
> PCI Bus │ │ │ │ │ │ │ │
> ─────────► │ │Func1│ │Func2│ ... │Func │ │
> Doorbell │ │ │ │ │ │<n> │ │
> │ │ │ │ │ │ │ │
> │ └──┬──┘ └──┬──┘ └──┬──┘ │
> │ │ │ │ │
> └──────┼────────┼───────────┼────┘
> │ │ │
> ▼ ▼ ▼
> ┌────────────────────────┐
> │ MSI Controller │
> └────────────────────────┘
>
> Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> write MSI message to MSI controller to trigger doorbell IRQ for difference
> EP functions.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v12 to v13
> - new patch
This might be v13, but after all this time, I have no idea what you
are trying to do. You keep pasting this non-ASCII drawing in commit
messages, but I still have no idea what this PCI Bus Doorbell
represents.
I appreciate the knowledge shortage is on my end, but it would
definitely help if someone would take the time to explain what this is
all about.
From what I gather, the ITS is actually on an end-point, and get
writes from the host, but that doesn't answer much.
> ---
> drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> index b2a4b67545b82..16e7d53f0b133 100644
> --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> @@ -5,6 +5,7 @@
> // Copyright (C) 2022 Intel
>
> #include <linux/acpi_iort.h>
> +#include <linux/pci-ep-msi.h>
> #include <linux/pci.h>
>
> #include "irq-gic-common.h"
> @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> }
>
> +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> + int nvec, msi_alloc_info_t *info)
> +{
> + u32 dev_id;
> + int ret;
> +
> + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
What this doesn't express is *how* are the writes conveyed to the ITS.
Specifically, the DevID is normally sampled as sideband information at
during the write transaction.
Obviously, you can't do that over PCI. So there is a lot of
undisclosed assumption about how the ITS is integrated, and how it
samples the DevID.
My conclusion is that this is not as generic as it seems to be. It is
definitely tied to implementation-specific behaviours, none of which
are explained.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 10:52 ` Marc Zyngier
@ 2024-12-19 17:02 ` Frank Li
2024-12-19 20:10 ` Niklas Cassel
2025-01-06 16:38 ` Frank Li
0 siblings, 2 replies; 21+ messages in thread
From: Frank Li @ 2024-12-19 17:02 UTC (permalink / raw)
To: Marc Zyngier
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> On Wed, 18 Dec 2024 23:08:39 +0000,
> Frank Li <Frank.Li@nxp.com> wrote:
> >
> > ┌────────────────────────────────┐
> > │ │
> > │ PCI Endpoint Controller │
> > │ │
> > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > PCI Bus │ │ │ │ │ │ │ │
> > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > Doorbell │ │ │ │ │ │<n> │ │
> > │ │ │ │ │ │ │ │
> > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > │ │ │ │ │
> > └──────┼────────┼───────────┼────┘
> > │ │ │
> > ▼ ▼ ▼
> > ┌────────────────────────┐
> > │ MSI Controller │
> > └────────────────────────┘
> >
> > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > EP functions.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change from v12 to v13
> > - new patch
>
> This might be v13, but after all this time, I have no idea what you
> are trying to do. You keep pasting this non-ASCII drawing in commit
> messages, but I still have no idea what this PCI Bus Doorbell
> represents.
PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
PCI Host, such as PC (x86).
i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
are not reverse direction. X86 try write some MMIO register ( mapped PCI
bar0). But i.MX95 don't know it have been modified. So currently solution
is create a polling thread to check every 10ms.
So this patches try resolve this problem at the platform, which have MSI
controller such as ITS.
after this patches, i.MX95 can create a PCI Bar1, which map to MSI
controller register space, when X86 write data to Bar1 (call as doorbell),
a irq will be triggered at i.MX95.
Doorbell in diagram means 'push doorbell' (write data to bar<n>).
>
> I appreciate the knowledge shortage is on my end, but it would
> definitely help if someone would take the time to explain what this is
> all about.
I am not sure if diagram in corver letter can help this, or above
descriptions is enough.
┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
│ │ │ │ │ │
│ │ │ 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 │ │ │
│ │ │ │ │ │
│ │ │ │ │ │
└────────────┘ └───────────────────────────────────┘ └────────────────┘
(* some detail have been changed and don't affect understand overall
picture)
>
> From what I gather, the ITS is actually on an end-point, and get
> writes from the host, but that doesn't answer much.
Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
AXI transaction -> ITS MMIO map register -> CPU IRQ.
The major problem how to distingiush from difference PCI Endpoint function
driver. There are have many EP functions as much as 8, which quite similar
standard PCI, one PCIe device can have 8 physical functions.
>
> > ---
> > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > index b2a4b67545b82..16e7d53f0b133 100644
> > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > @@ -5,6 +5,7 @@
> > // Copyright (C) 2022 Intel
> >
> > #include <linux/acpi_iort.h>
> > +#include <linux/pci-ep-msi.h>
> > #include <linux/pci.h>
> >
> > #include "irq-gic-common.h"
> > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > }
> >
> > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > + int nvec, msi_alloc_info_t *info)
> > +{
> > + u32 dev_id;
> > + int ret;
> > +
> > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
>
> What this doesn't express is *how* are the writes conveyed to the ITS.
> Specifically, the DevID is normally sampled as sideband information at
> during the write transaction.
Like PCI host, there msi-map in dts file, which descript how map PCI RID
to DevID, such as
msi-map = <0 $its 0x80 8>;
This informtion should be descripted in DTS or ACPI ...
>
> Obviously, you can't do that over PCI. So there is a lot of
> undisclosed assumption about how the ITS is integrated, and how it
> samples the DevID.
Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
driver should implement this type of covert. Such as i.MX95, there are
hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
here.
On going patch may help understand these
https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
If use latest ITS MSI64 should be simple, only need descript it at DTS
(I have not hardware to test this case yet).
pci-ep {
...
msi-map = <0 &its, 0x<8_0000, 0xff>;
^, ctrl ID.
msi-mask = <0xff>;
...
}
>
> My conclusion is that this is not as generic as it seems to be. It is
> definitely tied to implementation-specific behaviours, none of which
> are explained.
Compared to standard PCI MSI, which also have implementation-specific
behaviours, which convert PCI request ID to DevID Or stream ID.
https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
(I have struggle this for almost one year for this implementation-specific
part)
Well defined and mature PCI standard, MSI still need two parts, common part
and "implementation-specific" part.
Common part of standard PCI is at several place, such its driver/msi
libary/ kernel msi code ...
"implementation-specific" part is in PCI host bridge driver, such as
drivers/pci/controller/dwc/pcie-qcom.c
This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
who use another dwc controller, which they already implemented
"implementation-specific" by only update dts to provide hardware
information.(I guest he use ITS's MSI64)
Because it is new patches, I have not added Niklas's test-by tag. There
are not big functional change since Nikas test. The major change is make
msi part better align current MSI framework according to Thomas's
suggestion.
Frank
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 17:02 ` Frank Li
@ 2024-12-19 20:10 ` Niklas Cassel
2024-12-19 20:17 ` Frank Li
2025-01-06 16:38 ` Frank Li
1 sibling, 1 reply; 21+ messages in thread
From: Niklas Cassel @ 2024-12-19 20:10 UTC (permalink / raw)
To: Frank Li
Cc: Marc Zyngier, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, dlemoal, jdmason,
linux-arm-kernel
On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > On Wed, 18 Dec 2024 23:08:39 +0000,
[...]
> If use latest ITS MSI64 should be simple, only need descript it at DTS
> (I have not hardware to test this case yet).
> pci-ep {
> ...
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> ^, ctrl ID.
> msi-mask = <0xff>;
> ...
> }
[...]
> This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> who use another dwc controller, which they already implemented
> "implementation-specific" by only update dts to provide hardware
> information.(I guest he use ITS's MSI64)
>
> Because it is new patches, I have not added Niklas's test-by tag. There
> are not big functional change since Nikas test. The major change is make
> msi part better align current MSI framework according to Thomas's
> suggestion.
Frank, I tested this series (a few revisions back) on the rockchip rk3588,
which just like imx95, uses a DWC based PCIe EP controller, and ARM GIC ITS,
but unlike imx95, it does not require any additional look up table registers
to be configured.
While the rk3588 PCIe host controller node has:
msi-map = <0x0000 &its1 0x0000 0x1000>;
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi?h=v6.13-rc3#n164
The rk3588 PCIe endpoint controller node, which is the only one relevant
in this case, only has:
msi-parent = <&its1 0x0000>;
no msi-map.
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v6.14-armsoc/dts64&id=b6f09f497b07008aa65c31341138cecafa78222c
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 20:10 ` Niklas Cassel
@ 2024-12-19 20:17 ` Frank Li
2024-12-19 20:43 ` Niklas Cassel
0 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2024-12-19 20:17 UTC (permalink / raw)
To: Niklas Cassel
Cc: Marc Zyngier, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, dlemoal, jdmason,
linux-arm-kernel
On Thu, Dec 19, 2024 at 09:10:16PM +0100, Niklas Cassel wrote:
> On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > > On Wed, 18 Dec 2024 23:08:39 +0000,
>
> [...]
>
> > If use latest ITS MSI64 should be simple, only need descript it at DTS
> > (I have not hardware to test this case yet).
> > pci-ep {
> > ...
> > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > ^, ctrl ID.
> > msi-mask = <0xff>;
> > ...
> > }
>
> [...]
>
> > This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> > who use another dwc controller, which they already implemented
> > "implementation-specific" by only update dts to provide hardware
> > information.(I guest he use ITS's MSI64)
> >
> > Because it is new patches, I have not added Niklas's test-by tag. There
> > are not big functional change since Nikas test. The major change is make
> > msi part better align current MSI framework according to Thomas's
> > suggestion.
>
> Frank, I tested this series (a few revisions back) on the rockchip rk3588,
> which just like imx95, uses a DWC based PCIe EP controller, and ARM GIC ITS,
> but unlike imx95, it does not require any additional look up table registers
> to be configured.
>
>
> While the rk3588 PCIe host controller node has:
> msi-map = <0x0000 &its1 0x0000 0x1000>;
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi?h=v6.13-rc3#n164
>
> The rk3588 PCIe endpoint controller node, which is the only one relevant
> in this case, only has:
> msi-parent = <&its1 0x0000>;
> no msi-map.
> https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v6.14-armsoc/dts64&id=b6f09f497b07008aa65c31341138cecafa78222c
Thank you very much. I update msi part, so endpoint controller node align
host controller node.
It should be
msi-map = <0x0000 &its1 0x0000 0x1000>;
So if your hardware support multi physical function, your can create more
than one pci_test func. Previous version only support one EP func.
Frank
>
>
> Kind regards,
> Niklas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 20:17 ` Frank Li
@ 2024-12-19 20:43 ` Niklas Cassel
2024-12-19 20:53 ` Frank Li
0 siblings, 1 reply; 21+ messages in thread
From: Niklas Cassel @ 2024-12-19 20:43 UTC (permalink / raw)
To: Frank Li
Cc: Marc Zyngier, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, dlemoal, jdmason,
linux-arm-kernel
On Thu, Dec 19, 2024 at 03:17:15PM -0500, Frank Li wrote:
>
> Thank you very much. I update msi part, so endpoint controller node align
> host controller node.
>
> It should be
> msi-map = <0x0000 &its1 0x0000 0x1000>;
>
> So if your hardware support multi physical function, your can create more
> than one pci_test func. Previous version only support one EP func.
I see. That seems like an improvement.
I will need to ask Rockchip maintainer to drop my msi-parent patch for PCIe
EP node then. (Which is currently queued up in for-6.14)
However, for the PCIe host node, rk3588 has:
iommu-map = <0x0000 &mmu600_pcie 0x0000 0x1000>;
For the PCIe endpoint node, rk3588 has:
iommus = <&mmu600_pcie 0x0000>;
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v6.14-armsoc/dts64&id=da92d3dfc871e821a1bface3ba5afcf8cda19805
Is it fine that for the PCIe EP node, we specify iommu mapping using:
iommus = <&mmu600_pcie 0x0000>;
but the ITS/MSI map will be:
msi-map = <0x0000 &its1 0x0000 0x1000>;
isn't this a bit inconsistent?
The physical function is the "F" in the BDF.
Does this mean that:
iommus = <&mmu600_pcie 0x0000>;
the IOMMU will not be able to distinguish different PCI physical functions
from the same PCI device? So two different physical functions on the same
PCI device share the same IOMMU mappings?
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 20:43 ` Niklas Cassel
@ 2024-12-19 20:53 ` Frank Li
0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2024-12-19 20:53 UTC (permalink / raw)
To: Niklas Cassel
Cc: Marc Zyngier, Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, dlemoal, jdmason,
linux-arm-kernel
On Thu, Dec 19, 2024 at 09:43:15PM +0100, Niklas Cassel wrote:
> On Thu, Dec 19, 2024 at 03:17:15PM -0500, Frank Li wrote:
> >
> > Thank you very much. I update msi part, so endpoint controller node align
> > host controller node.
> >
> > It should be
> > msi-map = <0x0000 &its1 0x0000 0x1000>;
> >
> > So if your hardware support multi physical function, your can create more
> > than one pci_test func. Previous version only support one EP func.
>
> I see. That seems like an improvement.
> I will need to ask Rockchip maintainer to drop my msi-parent patch for PCIe
> EP node then. (Which is currently queued up in for-6.14)
>
> However, for the PCIe host node, rk3588 has:
> iommu-map = <0x0000 &mmu600_pcie 0x0000 0x1000>;
>
> For the PCIe endpoint node, rk3588 has:
> iommus = <&mmu600_pcie 0x0000>;
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v6.14-armsoc/dts64&id=da92d3dfc871e821a1bface3ba5afcf8cda19805
>
> Is it fine that for the PCIe EP node, we specify iommu mapping using:
> iommus = <&mmu600_pcie 0x0000>;
> but the ITS/MSI map will be:
> msi-map = <0x0000 &its1 0x0000 0x1000>;
For doorbell feature, it should be fine.
>
> isn't this a bit inconsistent?
Ideally, iommus need do similar map.
>
> The physical function is the "F" in the BDF.
> Does this mean that:
> iommus = <&mmu600_pcie 0x0000>;
> the IOMMU will not be able to distinguish different PCI physical functions
> from the same PCI device?
You are right. All physical functions will share one IOMMU space.
> So two different physical functions on the same
> PCI device share the same IOMMU mappings?
Yes,
Function should be okay. Only miss isolation protection. func1 and access
func2's dma memory address. At most system it should be fine.
Frank
>
>
> Kind regards,
> Niklas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2024-12-19 17:02 ` Frank Li
2024-12-19 20:10 ` Niklas Cassel
@ 2025-01-06 16:38 ` Frank Li
2025-01-06 17:13 ` Marc Zyngier
1 sibling, 1 reply; 21+ messages in thread
From: Frank Li @ 2025-01-06 16:38 UTC (permalink / raw)
To: Marc Zyngier
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > On Wed, 18 Dec 2024 23:08:39 +0000,
> > Frank Li <Frank.Li@nxp.com> wrote:
> > >
> > > ┌────────────────────────────────┐
> > > │ │
> > > │ PCI Endpoint Controller │
> > > │ │
> > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > PCI Bus │ │ │ │ │ │ │ │
> > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > Doorbell │ │ │ │ │ │<n> │ │
> > > │ │ │ │ │ │ │ │
> > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > │ │ │ │ │
> > > └──────┼────────┼───────────┼────┘
> > > │ │ │
> > > ▼ ▼ ▼
> > > ┌────────────────────────┐
> > > │ MSI Controller │
> > > └────────────────────────┘
> > >
> > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > EP functions.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > change from v12 to v13
> > > - new patch
> >
> > This might be v13, but after all this time, I have no idea what you
> > are trying to do. You keep pasting this non-ASCII drawing in commit
> > messages, but I still have no idea what this PCI Bus Doorbell
> > represents.
>
> PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> PCI Host, such as PC (x86).
>
> i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> are not reverse direction. X86 try write some MMIO register ( mapped PCI
> bar0). But i.MX95 don't know it have been modified. So currently solution
> is create a polling thread to check every 10ms.
>
> So this patches try resolve this problem at the platform, which have MSI
> controller such as ITS.
>
> after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> controller register space, when X86 write data to Bar1 (call as doorbell),
> a irq will be triggered at i.MX95.
>
> Doorbell in diagram means 'push doorbell' (write data to bar<n>).
>
> >
> > I appreciate the knowledge shortage is on my end, but it would
> > definitely help if someone would take the time to explain what this is
> > all about.
>
> I am not sure if diagram in corver letter can help this, or above
> descriptions is enough.
>
>
> ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> │ │ │ │ │ │
> │ │ │ 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 │ │ │
> │ │ │ │ │ │
> │ │ │ │ │ │
> └────────────┘ └───────────────────────────────────┘ └────────────────┘
> (* some detail have been changed and don't affect understand overall
> picture)
>
> >
> > From what I gather, the ITS is actually on an end-point, and get
> > writes from the host, but that doesn't answer much.
>
> Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> AXI transaction -> ITS MMIO map register -> CPU IRQ.
>
> The major problem how to distingiush from difference PCI Endpoint function
> driver. There are have many EP functions as much as 8, which quite similar
> standard PCI, one PCIe device can have 8 physical functions.
>
> >
> > > ---
> > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > index b2a4b67545b82..16e7d53f0b133 100644
> > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > @@ -5,6 +5,7 @@
> > > // Copyright (C) 2022 Intel
> > >
> > > #include <linux/acpi_iort.h>
> > > +#include <linux/pci-ep-msi.h>
> > > #include <linux/pci.h>
> > >
> > > #include "irq-gic-common.h"
> > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > }
> > >
> > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > + int nvec, msi_alloc_info_t *info)
> > > +{
> > > + u32 dev_id;
> > > + int ret;
> > > +
> > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> >
> > What this doesn't express is *how* are the writes conveyed to the ITS.
> > Specifically, the DevID is normally sampled as sideband information at
> > during the write transaction.
>
> Like PCI host, there msi-map in dts file, which descript how map PCI RID
> to DevID, such as
> msi-map = <0 $its 0x80 8>;
>
> This informtion should be descripted in DTS or ACPI ...
>
> >
> > Obviously, you can't do that over PCI. So there is a lot of
> > undisclosed assumption about how the ITS is integrated, and how it
> > samples the DevID.
>
> Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> driver should implement this type of covert. Such as i.MX95, there are
> hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> here.
>
> On going patch may help understand these
> https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
>
> If use latest ITS MSI64 should be simple, only need descript it at DTS
> (I have not hardware to test this case yet).
> pci-ep {
> ...
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> ^, ctrl ID.
> msi-mask = <0xff>;
> ...
> }
>
> >
> > My conclusion is that this is not as generic as it seems to be. It is
> > definitely tied to implementation-specific behaviours, none of which
> > are explained.
>
> Compared to standard PCI MSI, which also have implementation-specific
> behaviours, which convert PCI request ID to DevID Or stream ID.
> https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> (I have struggle this for almost one year for this implementation-specific
> part)
>
> Well defined and mature PCI standard, MSI still need two parts, common part
> and "implementation-specific" part.
>
> Common part of standard PCI is at several place, such its driver/msi
> libary/ kernel msi code ...
>
> "implementation-specific" part is in PCI host bridge driver, such as
> drivers/pci/controller/dwc/pcie-qcom.c
>
> This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> who use another dwc controller, which they already implemented
> "implementation-specific" by only update dts to provide hardware
> information.(I guest he use ITS's MSI64)
>
> Because it is new patches, I have not added Niklas's test-by tag. There
> are not big functional change since Nikas test. The major change is make
> msi part better align current MSI framework according to Thomas's
> suggestion.
Thomas Gleixner and Marc Zyngier:
Happy new year! Do you have additioinal comments for this?
Frank
>
> Frank
> >
> > Thanks,
> >
> > M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2025-01-06 16:38 ` Frank Li
@ 2025-01-06 17:13 ` Marc Zyngier
2025-01-06 18:20 ` Frank Li
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2025-01-06 17:13 UTC (permalink / raw)
To: Frank Li
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Mon, 06 Jan 2025 16:38:02 +0000,
Frank Li <Frank.li@nxp.com> wrote:
>
> On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > > On Wed, 18 Dec 2024 23:08:39 +0000,
> > > Frank Li <Frank.Li@nxp.com> wrote:
> > > >
> > > > ┌────────────────────────────────┐
> > > > │ │
> > > > │ PCI Endpoint Controller │
> > > > │ │
> > > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > > PCI Bus │ │ │ │ │ │ │ │
> > > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > > Doorbell │ │ │ │ │ │<n> │ │
> > > > │ │ │ │ │ │ │ │
> > > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > > │ │ │ │ │
> > > > └──────┼────────┼───────────┼────┘
> > > > │ │ │
> > > > ▼ ▼ ▼
> > > > ┌────────────────────────┐
> > > > │ MSI Controller │
> > > > └────────────────────────┘
> > > >
> > > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > > EP functions.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > > change from v12 to v13
> > > > - new patch
> > >
> > > This might be v13, but after all this time, I have no idea what you
> > > are trying to do. You keep pasting this non-ASCII drawing in commit
> > > messages, but I still have no idea what this PCI Bus Doorbell
> > > represents.
> >
> > PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> > as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> > PCI Host, such as PC (x86).
> >
> > i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> > are not reverse direction. X86 try write some MMIO register ( mapped PCI
> > bar0). But i.MX95 don't know it have been modified. So currently solution
> > is create a polling thread to check every 10ms.
> >
> > So this patches try resolve this problem at the platform, which have MSI
> > controller such as ITS.
> >
> > after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> > controller register space, when X86 write data to Bar1 (call as doorbell),
> > a irq will be triggered at i.MX95.
> >
> > Doorbell in diagram means 'push doorbell' (write data to bar<n>).
> >
> > >
> > > I appreciate the knowledge shortage is on my end, but it would
> > > definitely help if someone would take the time to explain what this is
> > > all about.
> >
> > I am not sure if diagram in corver letter can help this, or above
> > descriptions is enough.
> >
> >
> > ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> > │ │ │ │ │ │
> > │ │ │ 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 │ │ │
> > │ │ │ │ │ │
> > │ │ │ │ │ │
> > └────────────┘ └───────────────────────────────────┘ └────────────────┘
> > (* some detail have been changed and don't affect understand overall
> > picture)
> >
> > >
> > > From what I gather, the ITS is actually on an end-point, and get
> > > writes from the host, but that doesn't answer much.
> >
> > Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> > AXI transaction -> ITS MMIO map register -> CPU IRQ.
> >
> > The major problem how to distingiush from difference PCI Endpoint function
> > driver. There are have many EP functions as much as 8, which quite similar
> > standard PCI, one PCIe device can have 8 physical functions.
> >
> > >
> > > > ---
> > > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > index b2a4b67545b82..16e7d53f0b133 100644
> > > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > @@ -5,6 +5,7 @@
> > > > // Copyright (C) 2022 Intel
> > > >
> > > > #include <linux/acpi_iort.h>
> > > > +#include <linux/pci-ep-msi.h>
> > > > #include <linux/pci.h>
> > > >
> > > > #include "irq-gic-common.h"
> > > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > > }
> > > >
> > > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > > + int nvec, msi_alloc_info_t *info)
> > > > +{
> > > > + u32 dev_id;
> > > > + int ret;
> > > > +
> > > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> > >
> > > What this doesn't express is *how* are the writes conveyed to the ITS.
> > > Specifically, the DevID is normally sampled as sideband information at
> > > during the write transaction.
> >
> > Like PCI host, there msi-map in dts file, which descript how map PCI RID
> > to DevID, such as
> > msi-map = <0 $its 0x80 8>;
> >
> > This informtion should be descripted in DTS or ACPI ...
> >
> > >
> > > Obviously, you can't do that over PCI. So there is a lot of
> > > undisclosed assumption about how the ITS is integrated, and how it
> > > samples the DevID.
> >
> > Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> > driver should implement this type of covert. Such as i.MX95, there are
> > hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> > here.
> >
> > On going patch may help understand these
> > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> >
> > If use latest ITS MSI64 should be simple, only need descript it at DTS
> > (I have not hardware to test this case yet).
> > pci-ep {
> > ...
> > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > ^, ctrl ID.
> > msi-mask = <0xff>;
> > ...
> > }
> >
> > >
> > > My conclusion is that this is not as generic as it seems to be. It is
> > > definitely tied to implementation-specific behaviours, none of which
> > > are explained.
> >
> > Compared to standard PCI MSI, which also have implementation-specific
> > behaviours, which convert PCI request ID to DevID Or stream ID.
> > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > (I have struggle this for almost one year for this implementation-specific
> > part)
> >
> > Well defined and mature PCI standard, MSI still need two parts, common part
> > and "implementation-specific" part.
> >
> > Common part of standard PCI is at several place, such its driver/msi
> > libary/ kernel msi code ...
> >
> > "implementation-specific" part is in PCI host bridge driver, such as
> > drivers/pci/controller/dwc/pcie-qcom.c
> >
> > This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> > who use another dwc controller, which they already implemented
> > "implementation-specific" by only update dts to provide hardware
> > information.(I guest he use ITS's MSI64)
> >
> > Because it is new patches, I have not added Niklas's test-by tag. There
> > are not big functional change since Nikas test. The major change is make
> > msi part better align current MSI framework according to Thomas's
> > suggestion.
>
> Thomas Gleixner and Marc Zyngier:
>
> Happy new year! Do you have additioinal comments for this?
I think this is pretty pointless.
- DOMAIN_BUS_DEVICE_PCI_EP_MSI is just a reinvention of platform MSI,
and I don't see why we need to have *two* square wheels
- The whole thing relies on IMPDEF behaviours that are not described,
making it impossible to write a *host* driver that works
universally. Specifically, you completely ignored my comment about
*how* is the DevID sampled on the ITS side. How is that supposed to
work when the DevID is carried as AXI user bits instead of data? How
can the host provide that information?
- your "but it's been tested by..." argument doesn't carry much
weight, as the kernel has at least one critical bug per "Tested-by"
tag
Given that, I don't see how this series is fit for purpose.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2025-01-06 17:13 ` Marc Zyngier
@ 2025-01-06 18:20 ` Frank Li
2025-01-13 16:11 ` Frank Li
0 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2025-01-06 18:20 UTC (permalink / raw)
To: Marc Zyngier
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Mon, Jan 06, 2025 at 05:13:10PM +0000, Marc Zyngier wrote:
> On Mon, 06 Jan 2025 16:38:02 +0000,
> Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> > > On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > > > On Wed, 18 Dec 2024 23:08:39 +0000,
> > > > Frank Li <Frank.Li@nxp.com> wrote:
> > > > >
> > > > > ┌────────────────────────────────┐
> > > > > │ │
> > > > > │ PCI Endpoint Controller │
> > > > > │ │
> > > > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > > > PCI Bus │ │ │ │ │ │ │ │
> > > > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > > > Doorbell │ │ │ │ │ │<n> │ │
> > > > > │ │ │ │ │ │ │ │
> > > > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > > > │ │ │ │ │
> > > > > └──────┼────────┼───────────┼────┘
> > > > > │ │ │
> > > > > ▼ ▼ ▼
> > > > > ┌────────────────────────┐
> > > > > │ MSI Controller │
> > > > > └────────────────────────┘
> > > > >
> > > > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > > > EP functions.
> > > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > > change from v12 to v13
> > > > > - new patch
> > > >
> > > > This might be v13, but after all this time, I have no idea what you
> > > > are trying to do. You keep pasting this non-ASCII drawing in commit
> > > > messages, but I still have no idea what this PCI Bus Doorbell
> > > > represents.
> > >
> > > PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> > > as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> > > PCI Host, such as PC (x86).
> > >
> > > i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> > > are not reverse direction. X86 try write some MMIO register ( mapped PCI
> > > bar0). But i.MX95 don't know it have been modified. So currently solution
> > > is create a polling thread to check every 10ms.
> > >
> > > So this patches try resolve this problem at the platform, which have MSI
> > > controller such as ITS.
> > >
> > > after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> > > controller register space, when X86 write data to Bar1 (call as doorbell),
> > > a irq will be triggered at i.MX95.
> > >
> > > Doorbell in diagram means 'push doorbell' (write data to bar<n>).
> > >
> > > >
> > > > I appreciate the knowledge shortage is on my end, but it would
> > > > definitely help if someone would take the time to explain what this is
> > > > all about.
> > >
> > > I am not sure if diagram in corver letter can help this, or above
> > > descriptions is enough.
> > >
> > >
> > > ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> > > │ │ │ │ │ │
> > > │ │ │ 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 │ │ │
> > > │ │ │ │ │ │
> > > │ │ │ │ │ │
> > > └────────────┘ └───────────────────────────────────┘ └────────────────┘
> > > (* some detail have been changed and don't affect understand overall
> > > picture)
> > >
> > > >
> > > > From what I gather, the ITS is actually on an end-point, and get
> > > > writes from the host, but that doesn't answer much.
> > >
> > > Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> > > AXI transaction -> ITS MMIO map register -> CPU IRQ.
> > >
> > > The major problem how to distingiush from difference PCI Endpoint function
> > > driver. There are have many EP functions as much as 8, which quite similar
> > > standard PCI, one PCIe device can have 8 physical functions.
> > >
> > > >
> > > > > ---
> > > > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > index b2a4b67545b82..16e7d53f0b133 100644
> > > > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > @@ -5,6 +5,7 @@
> > > > > // Copyright (C) 2022 Intel
> > > > >
> > > > > #include <linux/acpi_iort.h>
> > > > > +#include <linux/pci-ep-msi.h>
> > > > > #include <linux/pci.h>
> > > > >
> > > > > #include "irq-gic-common.h"
> > > > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > > > }
> > > > >
> > > > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > + int nvec, msi_alloc_info_t *info)
> > > > > +{
> > > > > + u32 dev_id;
> > > > > + int ret;
> > > > > +
> > > > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> > > >
> > > > What this doesn't express is *how* are the writes conveyed to the ITS.
> > > > Specifically, the DevID is normally sampled as sideband information at
> > > > during the write transaction.
> > >
> > > Like PCI host, there msi-map in dts file, which descript how map PCI RID
> > > to DevID, such as
> > > msi-map = <0 $its 0x80 8>;
> > >
> > > This informtion should be descripted in DTS or ACPI ...
> > >
> > > >
> > > > Obviously, you can't do that over PCI. So there is a lot of
> > > > undisclosed assumption about how the ITS is integrated, and how it
> > > > samples the DevID.
> > >
> > > Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> > > driver should implement this type of covert. Such as i.MX95, there are
> > > hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> > > here.
> > >
> > > On going patch may help understand these
> > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > >
> > > If use latest ITS MSI64 should be simple, only need descript it at DTS
> > > (I have not hardware to test this case yet).
> > > pci-ep {
> > > ...
> > > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > > ^, ctrl ID.
> > > msi-mask = <0xff>;
> > > ...
> > > }
Bookmark 1
> > >
> > > >
> > > > My conclusion is that this is not as generic as it seems to be. It is
> > > > definitely tied to implementation-specific behaviours, none of which
> > > > are explained.
> > >
> > > Compared to standard PCI MSI, which also have implementation-specific
> > > behaviours, which convert PCI request ID to DevID Or stream ID.
> > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > > (I have struggle this for almost one year for this implementation-specific
> > > part)
> > >
> > > Well defined and mature PCI standard, MSI still need two parts, common part
> > > and "implementation-specific" part.
> > >
> > > Common part of standard PCI is at several place, such its driver/msi
> > > libary/ kernel msi code ...
> > >
> > > "implementation-specific" part is in PCI host bridge driver, such as
> > > drivers/pci/controller/dwc/pcie-qcom.c
> > >
> > > This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> > > who use another dwc controller, which they already implemented
> > > "implementation-specific" by only update dts to provide hardware
> > > information.(I guest he use ITS's MSI64)
> > >
> > > Because it is new patches, I have not added Niklas's test-by tag. There
> > > are not big functional change since Nikas test. The major change is make
> > > msi part better align current MSI framework according to Thomas's
> > > suggestion.
> >
> > Thomas Gleixner and Marc Zyngier:
> >
> > Happy new year! Do you have additioinal comments for this?
>
> I think this is pretty pointless.
>
> - DOMAIN_BUS_DEVICE_PCI_EP_MSI is just a reinvention of platform MSI,
> and I don't see why we need to have *two* square wheels
"DOMAIN_BUS_DEVICE_PCI_EP_MSI" was purposed by Thomas Gleixner.
https://lore.kernel.org/linux-pci/87o7197wbx.ffs@tglx/, the difference
is
- "platform MSI" only have one device id for each device. But
DOMAIN_BUS_DEVICE_PCI_EP_MSI have multi child devices, which need map to
difference devices id.
If you like, I can try to extend "platform msi" to support msi-map. But
The other problem "immutable MSI messages" need be resolve also.
PCIe EP require "immutable MSI messages". address/data pair can't be change
by set irq affinity.
>
> - The whole thing relies on IMPDEF behaviours that are not described,
> making it impossible to write a *host* driver that works
> universally.
Host side need NOT know EP's side IMPDEF behaviours. Host side just know
addr/data pair. *(BAR<n> + offset) = DATA (in RC side) will trigger
doorbell.
"AXI user bits" concern see below book mark axi.
> Specifically, you completely ignored my comment about
> *how* is the DevID sampled on the ITS side.
See above "book mark 1", let me change another words, descript again,
It is quite similar with PCI root complex case.
In PCI RC's dts, it looks:
pci {
...
msi-map = <0 &its, 0x<8_0000, 0xff>;
^, ctrl ID.
...
}
ITS call pci_msi_domain_get_msi_rid() to get device id.
static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
int nvec, msi_alloc_info_t *info)
{
...
info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain->parent, pdev);
...
}
PCI msi common code call __of_msi_map_id() to convert PCI rid to stream id
from dts file. It should have similar method if device have not use DT.
--- EP case (Run at EP side) ---
for my patches, it do similar thing, in dts, PCI EP controller
pci-ep {
msi-map = <0 &its, 0x<8_0000, 0xff>;
msi-mask = <0xff>;
}
static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
int nvec, msi_alloc_info_t *info)
{
....
ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
....
}
PCIe EP common part will convert EP function device ID to difference device
id according to msi-map in pci-ep node.
> How is that supposed to
> work when the DevID is carried as AXI user bits instead of data? How
> can the host provide that information?
book mark AXI:
Host driver needn't such information. Host write PCI TLP, such as
*ADDR = DATA.
PCI EP controller get such TLP, which convert to AXI write. PCI EP
controller will add AXI user bits, which was descripted in PCI EP
controller's dts file.
pci-ep {
msi-map = <0 &its, 0x<8_0000, 0xff>;
^ "8" is AXI user bits, which added when
convert TLP to axi write.
}
>
> - your "but it's been tested by..." argument doesn't carry much
> weight, as the kernel has at least one critical bug per "Tested-by"
> tag
My means is this solution can cross difference platform with only dts
change.
>
> Given that, I don't see how this series is fit for purpose.
sorry for add book mark to refer to difference place in the the mail.
let me know if need further description.
Frank
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2025-01-06 18:20 ` Frank Li
@ 2025-01-13 16:11 ` Frank Li
2025-01-20 21:20 ` Frank Li
0 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2025-01-13 16:11 UTC (permalink / raw)
To: Marc Zyngier
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Mon, Jan 06, 2025 at 01:20:17PM -0500, Frank Li wrote:
> On Mon, Jan 06, 2025 at 05:13:10PM +0000, Marc Zyngier wrote:
> > On Mon, 06 Jan 2025 16:38:02 +0000,
> > Frank Li <Frank.li@nxp.com> wrote:
> > >
> > > On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> > > > On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > > > > On Wed, 18 Dec 2024 23:08:39 +0000,
> > > > > Frank Li <Frank.Li@nxp.com> wrote:
> > > > > >
> > > > > > ┌────────────────────────────────┐
> > > > > > │ │
> > > > > > │ PCI Endpoint Controller │
> > > > > > │ │
> > > > > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > > > > PCI Bus │ │ │ │ │ │ │ │
> > > > > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > > > > Doorbell │ │ │ │ │ │<n> │ │
> > > > > > │ │ │ │ │ │ │ │
> > > > > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > > > > │ │ │ │ │
> > > > > > └──────┼────────┼───────────┼────┘
> > > > > > │ │ │
> > > > > > ▼ ▼ ▼
> > > > > > ┌────────────────────────┐
> > > > > > │ MSI Controller │
> > > > > > └────────────────────────┘
> > > > > >
> > > > > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > > > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > > > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > > > > EP functions.
> > > > > >
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > > change from v12 to v13
> > > > > > - new patch
> > > > >
> > > > > This might be v13, but after all this time, I have no idea what you
> > > > > are trying to do. You keep pasting this non-ASCII drawing in commit
> > > > > messages, but I still have no idea what this PCI Bus Doorbell
> > > > > represents.
> > > >
> > > > PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> > > > as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> > > > PCI Host, such as PC (x86).
> > > >
> > > > i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> > > > are not reverse direction. X86 try write some MMIO register ( mapped PCI
> > > > bar0). But i.MX95 don't know it have been modified. So currently solution
> > > > is create a polling thread to check every 10ms.
> > > >
> > > > So this patches try resolve this problem at the platform, which have MSI
> > > > controller such as ITS.
> > > >
> > > > after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> > > > controller register space, when X86 write data to Bar1 (call as doorbell),
> > > > a irq will be triggered at i.MX95.
> > > >
> > > > Doorbell in diagram means 'push doorbell' (write data to bar<n>).
> > > >
> > > > >
> > > > > I appreciate the knowledge shortage is on my end, but it would
> > > > > definitely help if someone would take the time to explain what this is
> > > > > all about.
> > > >
> > > > I am not sure if diagram in corver letter can help this, or above
> > > > descriptions is enough.
> > > >
> > > >
> > > > ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> > > > │ │ │ │ │ │
> > > > │ │ │ 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 │ │ │
> > > > │ │ │ │ │ │
> > > > │ │ │ │ │ │
> > > > └────────────┘ └───────────────────────────────────┘ └────────────────┘
> > > > (* some detail have been changed and don't affect understand overall
> > > > picture)
> > > >
> > > > >
> > > > > From what I gather, the ITS is actually on an end-point, and get
> > > > > writes from the host, but that doesn't answer much.
> > > >
> > > > Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> > > > AXI transaction -> ITS MMIO map register -> CPU IRQ.
> > > >
> > > > The major problem how to distingiush from difference PCI Endpoint function
> > > > driver. There are have many EP functions as much as 8, which quite similar
> > > > standard PCI, one PCIe device can have 8 physical functions.
> > > >
> > > > >
> > > > > > ---
> > > > > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > > > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > index b2a4b67545b82..16e7d53f0b133 100644
> > > > > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > @@ -5,6 +5,7 @@
> > > > > > // Copyright (C) 2022 Intel
> > > > > >
> > > > > > #include <linux/acpi_iort.h>
> > > > > > +#include <linux/pci-ep-msi.h>
> > > > > > #include <linux/pci.h>
> > > > > >
> > > > > > #include "irq-gic-common.h"
> > > > > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > > > > }
> > > > > >
> > > > > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > > + int nvec, msi_alloc_info_t *info)
> > > > > > +{
> > > > > > + u32 dev_id;
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> > > > >
> > > > > What this doesn't express is *how* are the writes conveyed to the ITS.
> > > > > Specifically, the DevID is normally sampled as sideband information at
> > > > > during the write transaction.
> > > >
> > > > Like PCI host, there msi-map in dts file, which descript how map PCI RID
> > > > to DevID, such as
> > > > msi-map = <0 $its 0x80 8>;
> > > >
> > > > This informtion should be descripted in DTS or ACPI ...
> > > >
> > > > >
> > > > > Obviously, you can't do that over PCI. So there is a lot of
> > > > > undisclosed assumption about how the ITS is integrated, and how it
> > > > > samples the DevID.
> > > >
> > > > Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> > > > driver should implement this type of covert. Such as i.MX95, there are
> > > > hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> > > > here.
> > > >
> > > > On going patch may help understand these
> > > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > > >
> > > > If use latest ITS MSI64 should be simple, only need descript it at DTS
> > > > (I have not hardware to test this case yet).
> > > > pci-ep {
> > > > ...
> > > > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > > > ^, ctrl ID.
> > > > msi-mask = <0xff>;
> > > > ...
> > > > }
>
> Bookmark 1
>
> > > >
> > > > >
> > > > > My conclusion is that this is not as generic as it seems to be. It is
> > > > > definitely tied to implementation-specific behaviours, none of which
> > > > > are explained.
> > > >
> > > > Compared to standard PCI MSI, which also have implementation-specific
> > > > behaviours, which convert PCI request ID to DevID Or stream ID.
> > > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > > > (I have struggle this for almost one year for this implementation-specific
> > > > part)
> > > >
> > > > Well defined and mature PCI standard, MSI still need two parts, common part
> > > > and "implementation-specific" part.
> > > >
> > > > Common part of standard PCI is at several place, such its driver/msi
> > > > libary/ kernel msi code ...
> > > >
> > > > "implementation-specific" part is in PCI host bridge driver, such as
> > > > drivers/pci/controller/dwc/pcie-qcom.c
> > > >
> > > > This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> > > > who use another dwc controller, which they already implemented
> > > > "implementation-specific" by only update dts to provide hardware
> > > > information.(I guest he use ITS's MSI64)
> > > >
> > > > Because it is new patches, I have not added Niklas's test-by tag. There
> > > > are not big functional change since Nikas test. The major change is make
> > > > msi part better align current MSI framework according to Thomas's
> > > > suggestion.
> > >
> > > Thomas Gleixner and Marc Zyngier:
> > >
> > > Happy new year! Do you have additioinal comments for this?
> >
> > I think this is pretty pointless.
> >
> > - DOMAIN_BUS_DEVICE_PCI_EP_MSI is just a reinvention of platform MSI,
> > and I don't see why we need to have *two* square wheels
>
> "DOMAIN_BUS_DEVICE_PCI_EP_MSI" was purposed by Thomas Gleixner.
> https://lore.kernel.org/linux-pci/87o7197wbx.ffs@tglx/, the difference
> is
> - "platform MSI" only have one device id for each device. But
> DOMAIN_BUS_DEVICE_PCI_EP_MSI have multi child devices, which need map to
> difference devices id.
>
> If you like, I can try to extend "platform msi" to support msi-map. But
> The other problem "immutable MSI messages" need be resolve also.
>
> PCIe EP require "immutable MSI messages". address/data pair can't be change
> by set irq affinity.
>
> >
> > - The whole thing relies on IMPDEF behaviours that are not described,
> > making it impossible to write a *host* driver that works
> > universally.
>
> Host side need NOT know EP's side IMPDEF behaviours. Host side just know
> addr/data pair. *(BAR<n> + offset) = DATA (in RC side) will trigger
> doorbell.
>
> "AXI user bits" concern see below book mark axi.
>
> > Specifically, you completely ignored my comment about
> > *how* is the DevID sampled on the ITS side.
>
> See above "book mark 1", let me change another words, descript again,
>
> It is quite similar with PCI root complex case.
>
> In PCI RC's dts, it looks:
>
> pci {
> ...
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> ^, ctrl ID.
> ...
> }
>
> ITS call pci_msi_domain_get_msi_rid() to get device id.
>
> static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
> int nvec, msi_alloc_info_t *info)
> {
> ...
> info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain->parent, pdev);
> ...
> }
>
> PCI msi common code call __of_msi_map_id() to convert PCI rid to stream id
> from dts file. It should have similar method if device have not use DT.
>
> --- EP case (Run at EP side) ---
>
> for my patches, it do similar thing, in dts, PCI EP controller
>
> pci-ep {
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> msi-mask = <0xff>;
> }
>
> static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> int nvec, msi_alloc_info_t *info)
> {
>
> ....
> ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> ....
> }
>
> PCIe EP common part will convert EP function device ID to difference device
> id according to msi-map in pci-ep node.
>
> > How is that supposed to
> > work when the DevID is carried as AXI user bits instead of data? How
> > can the host provide that information?
>
> book mark AXI:
>
> Host driver needn't such information. Host write PCI TLP, such as
> *ADDR = DATA.
>
> PCI EP controller get such TLP, which convert to AXI write. PCI EP
> controller will add AXI user bits, which was descripted in PCI EP
> controller's dts file.
>
> pci-ep {
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> ^ "8" is AXI user bits, which added when
> convert TLP to axi write.
>
> }
>
> >
> > - your "but it's been tested by..." argument doesn't carry much
> > weight, as the kernel has at least one critical bug per "Tested-by"
> > tag
>
> My means is this solution can cross difference platform with only dts
> change.
>
> >
> > Given that, I don't see how this series is fit for purpose.
>
> sorry for add book mark to refer to difference place in the the mail.
>
> let me know if need further description.
>
Marc Zyngier:
Do you have any chance to read my reply? Does it anwser your
question? anything missed or still unclear? How to move forward?
Frank
> Frank
>
> >
> > Thanks,
> >
> > M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support
2025-01-13 16:11 ` Frank Li
@ 2025-01-20 21:20 ` Frank Li
0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2025-01-20 21:20 UTC (permalink / raw)
To: Marc Zyngier
Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
Anup Patel, linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal,
jdmason, linux-arm-kernel
On Mon, Jan 13, 2025 at 11:11:22AM -0500, Frank Li wrote:
> On Mon, Jan 06, 2025 at 01:20:17PM -0500, Frank Li wrote:
> > On Mon, Jan 06, 2025 at 05:13:10PM +0000, Marc Zyngier wrote:
> > > On Mon, 06 Jan 2025 16:38:02 +0000,
> > > Frank Li <Frank.li@nxp.com> wrote:
> > > >
> > > > On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> > > > > On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > > > > > On Wed, 18 Dec 2024 23:08:39 +0000,
> > > > > > Frank Li <Frank.Li@nxp.com> wrote:
> > > > > > >
> > > > > > > ┌────────────────────────────────┐
> > > > > > > │ │
> > > > > > > │ PCI Endpoint Controller │
> > > > > > > │ │
> > > > > > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > > > > > PCI Bus │ │ │ │ │ │ │ │
> > > > > > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > > > > > Doorbell │ │ │ │ │ │<n> │ │
> > > > > > > │ │ │ │ │ │ │ │
> > > > > > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > > > > > │ │ │ │ │
> > > > > > > └──────┼────────┼───────────┼────┘
> > > > > > > │ │ │
> > > > > > > ▼ ▼ ▼
> > > > > > > ┌────────────────────────┐
> > > > > > > │ MSI Controller │
> > > > > > > └────────────────────────┘
> > > > > > >
> > > > > > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > > > > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > > > > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > > > > > EP functions.
> > > > > > >
> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > > ---
> > > > > > > change from v12 to v13
> > > > > > > - new patch
> > > > > >
> > > > > > This might be v13, but after all this time, I have no idea what you
> > > > > > are trying to do. You keep pasting this non-ASCII drawing in commit
> > > > > > messages, but I still have no idea what this PCI Bus Doorbell
> > > > > > represents.
> > > > >
> > > > > PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> > > > > as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> > > > > PCI Host, such as PC (x86).
> > > > >
> > > > > i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> > > > > are not reverse direction. X86 try write some MMIO register ( mapped PCI
> > > > > bar0). But i.MX95 don't know it have been modified. So currently solution
> > > > > is create a polling thread to check every 10ms.
> > > > >
> > > > > So this patches try resolve this problem at the platform, which have MSI
> > > > > controller such as ITS.
> > > > >
> > > > > after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> > > > > controller register space, when X86 write data to Bar1 (call as doorbell),
> > > > > a irq will be triggered at i.MX95.
> > > > >
> > > > > Doorbell in diagram means 'push doorbell' (write data to bar<n>).
> > > > >
> > > > > >
> > > > > > I appreciate the knowledge shortage is on my end, but it would
> > > > > > definitely help if someone would take the time to explain what this is
> > > > > > all about.
> > > > >
> > > > > I am not sure if diagram in corver letter can help this, or above
> > > > > descriptions is enough.
> > > > >
> > > > >
> > > > > ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> > > > > │ │ │ │ │ │
> > > > > │ │ │ 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 │ │ │
> > > > > │ │ │ │ │ │
> > > > > │ │ │ │ │ │
> > > > > └────────────┘ └───────────────────────────────────┘ └────────────────┘
> > > > > (* some detail have been changed and don't affect understand overall
> > > > > picture)
> > > > >
> > > > > >
> > > > > > From what I gather, the ITS is actually on an end-point, and get
> > > > > > writes from the host, but that doesn't answer much.
> > > > >
> > > > > Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> > > > > AXI transaction -> ITS MMIO map register -> CPU IRQ.
> > > > >
> > > > > The major problem how to distingiush from difference PCI Endpoint function
> > > > > driver. There are have many EP functions as much as 8, which quite similar
> > > > > standard PCI, one PCIe device can have 8 physical functions.
> > > > >
> > > > > >
> > > > > > > ---
> > > > > > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > > > > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > > index b2a4b67545b82..16e7d53f0b133 100644
> > > > > > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > > > > > @@ -5,6 +5,7 @@
> > > > > > > // Copyright (C) 2022 Intel
> > > > > > >
> > > > > > > #include <linux/acpi_iort.h>
> > > > > > > +#include <linux/pci-ep-msi.h>
> > > > > > > #include <linux/pci.h>
> > > > > > >
> > > > > > > #include "irq-gic-common.h"
> > > > > > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > > > > > }
> > > > > > >
> > > > > > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > > > > > + int nvec, msi_alloc_info_t *info)
> > > > > > > +{
> > > > > > > + u32 dev_id;
> > > > > > > + int ret;
> > > > > > > +
> > > > > > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> > > > > >
> > > > > > What this doesn't express is *how* are the writes conveyed to the ITS.
> > > > > > Specifically, the DevID is normally sampled as sideband information at
> > > > > > during the write transaction.
> > > > >
> > > > > Like PCI host, there msi-map in dts file, which descript how map PCI RID
> > > > > to DevID, such as
> > > > > msi-map = <0 $its 0x80 8>;
> > > > >
> > > > > This informtion should be descripted in DTS or ACPI ...
> > > > >
> > > > > >
> > > > > > Obviously, you can't do that over PCI. So there is a lot of
> > > > > > undisclosed assumption about how the ITS is integrated, and how it
> > > > > > samples the DevID.
> > > > >
> > > > > Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> > > > > driver should implement this type of covert. Such as i.MX95, there are
> > > > > hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> > > > > here.
> > > > >
> > > > > On going patch may help understand these
> > > > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > > > >
> > > > > If use latest ITS MSI64 should be simple, only need descript it at DTS
> > > > > (I have not hardware to test this case yet).
> > > > > pci-ep {
> > > > > ...
> > > > > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > > > > ^, ctrl ID.
> > > > > msi-mask = <0xff>;
> > > > > ...
> > > > > }
> >
> > Bookmark 1
> >
> > > > >
> > > > > >
> > > > > > My conclusion is that this is not as generic as it seems to be. It is
> > > > > > definitely tied to implementation-specific behaviours, none of which
> > > > > > are explained.
> > > > >
> > > > > Compared to standard PCI MSI, which also have implementation-specific
> > > > > behaviours, which convert PCI request ID to DevID Or stream ID.
> > > > > https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> > > > > (I have struggle this for almost one year for this implementation-specific
> > > > > part)
> > > > >
> > > > > Well defined and mature PCI standard, MSI still need two parts, common part
> > > > > and "implementation-specific" part.
> > > > >
> > > > > Common part of standard PCI is at several place, such its driver/msi
> > > > > libary/ kernel msi code ...
> > > > >
> > > > > "implementation-specific" part is in PCI host bridge driver, such as
> > > > > drivers/pci/controller/dwc/pcie-qcom.c
> > > > >
> > > > > This solution already test by Tested-by: Niklas Cassel <cassel@kernel.org>
> > > > > who use another dwc controller, which they already implemented
> > > > > "implementation-specific" by only update dts to provide hardware
> > > > > information.(I guest he use ITS's MSI64)
> > > > >
> > > > > Because it is new patches, I have not added Niklas's test-by tag. There
> > > > > are not big functional change since Nikas test. The major change is make
> > > > > msi part better align current MSI framework according to Thomas's
> > > > > suggestion.
> > > >
> > > > Thomas Gleixner and Marc Zyngier:
> > > >
> > > > Happy new year! Do you have additioinal comments for this?
> > >
> > > I think this is pretty pointless.
> > >
> > > - DOMAIN_BUS_DEVICE_PCI_EP_MSI is just a reinvention of platform MSI,
> > > and I don't see why we need to have *two* square wheels
> >
> > "DOMAIN_BUS_DEVICE_PCI_EP_MSI" was purposed by Thomas Gleixner.
> > https://lore.kernel.org/linux-pci/87o7197wbx.ffs@tglx/, the difference
> > is
> > - "platform MSI" only have one device id for each device. But
> > DOMAIN_BUS_DEVICE_PCI_EP_MSI have multi child devices, which need map to
> > difference devices id.
> >
> > If you like, I can try to extend "platform msi" to support msi-map. But
> > The other problem "immutable MSI messages" need be resolve also.
> >
> > PCIe EP require "immutable MSI messages". address/data pair can't be change
> > by set irq affinity.
> >
> > >
> > > - The whole thing relies on IMPDEF behaviours that are not described,
> > > making it impossible to write a *host* driver that works
> > > universally.
> >
> > Host side need NOT know EP's side IMPDEF behaviours. Host side just know
> > addr/data pair. *(BAR<n> + offset) = DATA (in RC side) will trigger
> > doorbell.
> >
> > "AXI user bits" concern see below book mark axi.
> >
> > > Specifically, you completely ignored my comment about
> > > *how* is the DevID sampled on the ITS side.
> >
> > See above "book mark 1", let me change another words, descript again,
> >
> > It is quite similar with PCI root complex case.
> >
> > In PCI RC's dts, it looks:
> >
> > pci {
> > ...
> > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > ^, ctrl ID.
> > ...
> > }
> >
> > ITS call pci_msi_domain_get_msi_rid() to get device id.
> >
> > static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
> > int nvec, msi_alloc_info_t *info)
> > {
> > ...
> > info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain->parent, pdev);
> > ...
> > }
> >
> > PCI msi common code call __of_msi_map_id() to convert PCI rid to stream id
> > from dts file. It should have similar method if device have not use DT.
> >
> > --- EP case (Run at EP side) ---
> >
> > for my patches, it do similar thing, in dts, PCI EP controller
> >
> > pci-ep {
> > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > msi-mask = <0xff>;
> > }
> >
> > static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > int nvec, msi_alloc_info_t *info)
> > {
> >
> > ....
> > ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> > ....
> > }
> >
> > PCIe EP common part will convert EP function device ID to difference device
> > id according to msi-map in pci-ep node.
> >
> > > How is that supposed to
> > > work when the DevID is carried as AXI user bits instead of data? How
> > > can the host provide that information?
> >
> > book mark AXI:
> >
> > Host driver needn't such information. Host write PCI TLP, such as
> > *ADDR = DATA.
> >
> > PCI EP controller get such TLP, which convert to AXI write. PCI EP
> > controller will add AXI user bits, which was descripted in PCI EP
> > controller's dts file.
> >
> > pci-ep {
> > msi-map = <0 &its, 0x<8_0000, 0xff>;
> > ^ "8" is AXI user bits, which added when
> > convert TLP to axi write.
> >
> > }
> >
> > >
> > > - your "but it's been tested by..." argument doesn't carry much
> > > weight, as the kernel has at least one critical bug per "Tested-by"
> > > tag
> >
> > My means is this solution can cross difference platform with only dts
> > change.
> >
> > >
> > > Given that, I don't see how this series is fit for purpose.
> >
> > sorry for add book mark to refer to difference place in the the mail.
> >
> > let me know if need further description.
> >
>
> Marc Zyngier:
>
> Do you have any chance to read my reply? Does it anwser your
> question? anything missed or still unclear? How to move forward?
Marc:
Do you have any chance to read my reply?
Frank
>
> Frank
>
> > Frank
> >
> > >
> > > Thanks,
> > >
> > > M.
> > >
> > > --
> > > Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-01-20 21:22 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 23:08 [PATCH v13 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
2024-12-18 23:08 ` [PATCH v13 1/9] genirq/msi: Provide DOMAIN_BUS_PLATFORM_PCI_EP_MSI Frank Li
2024-12-18 23:08 ` [PATCH v13 2/9] PCI: EP: Add helper function pci_epf_msi_domain_get_msi_rid() Frank Li
2024-12-18 23:08 ` [PATCH v13 3/9] irqchip/gic-v3-its: Add helper function its_pmsi_prepare_devid() Frank Li
2024-12-18 23:08 ` [PATCH v13 4/9] irqchip/gic-v3-its: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI support Frank Li
2024-12-19 10:52 ` Marc Zyngier
2024-12-19 17:02 ` Frank Li
2024-12-19 20:10 ` Niklas Cassel
2024-12-19 20:17 ` Frank Li
2024-12-19 20:43 ` Niklas Cassel
2024-12-19 20:53 ` Frank Li
2025-01-06 16:38 ` Frank Li
2025-01-06 17:13 ` Marc Zyngier
2025-01-06 18:20 ` Frank Li
2025-01-13 16:11 ` Frank Li
2025-01-20 21:20 ` Frank Li
2024-12-18 23:08 ` [PATCH v13 5/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
2024-12-18 23:08 ` [PATCH v13 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
2024-12-18 23:08 ` [PATCH v13 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
2024-12-18 23:08 ` [PATCH v13 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
2024-12-18 23:08 ` [PATCH v13 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).