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

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

[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 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: cassel@kernel.org
Cc: dlemoal@kernel.org
Cc: maz@kernel.org
Cc: tglx@linutronix.de
Cc: jdmason@kudzu.us

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (5):
      PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering
      PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
      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/misc/pci_endpoint_test.c              |  63 +++++++++++++
 drivers/pci/endpoint/Makefile                 |   2 +-
 drivers/pci/endpoint/functions/pci-epf-test.c | 104 +++++++++++++++++++++
 drivers/pci/endpoint/pci-ep-msi.c             | 128 ++++++++++++++++++++++++++
 drivers/pci/endpoint/pci-epc-core.c           |  23 ++++-
 include/linux/pci-ep-msi.h                    |  15 +++
 include/linux/pci-epc.h                       |   2 +
 include/linux/pci-epf.h                       |  19 ++++
 include/uapi/linux/pcitest.h                  |   1 +
 tools/pci/pcitest.c                           |  16 +++-
 10 files changed, 369 insertions(+), 4 deletions(-)
---
base-commit: f231847d7f5a171be4566099f654521606b3ec37
change-id: 20241010-ep-msi-8b4cab33b1be

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

[PATCH v4 1/5] PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering

[Frank Li]

Introduce the pci_epc_get_fn() API, allowing a custom filter callback
function to be passed. Reimplement pci_epc_get() using pci_epc_get_fn()
with a match name callback. Prepare the codebase for adding RC-to-EP
doorbell support.

Add DEFINE_FREE(pci_epc_put) to implement scope based auto cleanup.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- none
---
 drivers/pci/endpoint/pci-epc-core.c | 23 +++++++++++++++++++++--
 include/linux/pci-epc.h             |  2 ++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 17f0071092550..14e6011df4b2c 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -48,6 +48,11 @@ void pci_epc_put(struct pci_epc *epc)
 }
 EXPORT_SYMBOL_GPL(pci_epc_put);
 
+static bool pci_epc_match_name(struct device *dev, void *name)
+{
+	return strcmp(dev_name(dev), name) == 0;
+}
+
 /**
  * pci_epc_get() - get the PCI endpoint controller
  * @epc_name: device name of the endpoint controller
@@ -56,6 +61,20 @@ EXPORT_SYMBOL_GPL(pci_epc_put);
  * endpoint controller
  */
 struct pci_epc *pci_epc_get(const char *epc_name)
+{
+	return pci_epc_get_fn(pci_epc_match_name, (void *)epc_name);
+}
+EXPORT_SYMBOL_GPL(pci_epc_get);
+
+/**
+ * pci_epc_get_fn() - get the PCI endpoint controller
+ * @fn: callback match filter function, return true when matched
+ * @param: parameter for callback function fn
+ *
+ * Invoke to get struct pci_epc * corresponding to the device name of the
+ * endpoint controller
+ */
+struct pci_epc *pci_epc_get_fn(bool (*fn)(struct device *dev, void *param), void *param)
 {
 	int ret = -EINVAL;
 	struct pci_epc *epc;
@@ -64,7 +83,7 @@ struct pci_epc *pci_epc_get(const char *epc_name)
 
 	class_dev_iter_init(&iter, &pci_epc_class, NULL, NULL);
 	while ((dev = class_dev_iter_next(&iter))) {
-		if (strcmp(epc_name, dev_name(dev)))
+		if (!fn(dev, param))
 			continue;
 
 		epc = to_pci_epc(dev);
@@ -82,7 +101,7 @@ struct pci_epc *pci_epc_get(const char *epc_name)
 	class_dev_iter_exit(&iter);
 	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(pci_epc_get);
+EXPORT_SYMBOL_GPL(pci_epc_get_fn);
 
 /**
  * pci_epc_get_first_free_bar() - helper to get first unreserved BAR
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 42ef06136bd1a..682808f4510be 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -266,7 +266,9 @@ pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
 enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
 					 *epc_features, enum pci_barno bar);
 struct pci_epc *pci_epc_get(const char *epc_name);
+struct pci_epc *pci_epc_get_fn(bool (*fn)(struct device *dev, void *param), void *param);
 void pci_epc_put(struct pci_epc *epc);
+DEFINE_FREE(pci_epc_put, void *, if (!(_T)) pci_epc_put(_T))
 
 int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
 		     size_t size, size_t page_size);

-- 
2.34.1

[PATCH v4 2/5] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[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.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- msi change to use msi_get_virq() avoid use msi_for_each_desc().
- add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
- move mutex lock to epc function
- initialize variable at declear place.
- passdown epf to epc*() function to simplify code.
---
 drivers/pci/endpoint/Makefile     |   2 +-
 drivers/pci/endpoint/pci-ep-msi.c | 128 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci-ep-msi.h        |  15 +++++
 include/linux/pci-epf.h           |  19 ++++++
 4 files changed, 163 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..91207fb66db32
--- /dev/null
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -0,0 +1,128 @@
+// 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/cleanup.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
+#include <linux/pci-ep-cfs.h>
+#include <linux/pci-ep-msi.h>
+
+static irqreturn_t pci_epf_doorbell_handler(int irq, void *data)
+{
+	struct pci_epf *epf = data;
+	struct msi_desc *desc = irq_get_msi_desc(irq);
+
+	if (desc && epf->event_ops && epf->event_ops->doorbell)
+		epf->event_ops->doorbell(epf, desc->msi_index);
+
+	return IRQ_HANDLED;
+}
+
+static bool pci_epc_match_parent(struct device *dev, void *param)
+{
+	return dev->parent == param;
+}
+
+static void pci_epc_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct pci_epc *epc __free(pci_epc_put) = NULL;
+	struct pci_epf *epf;
+
+	epc = pci_epc_get_fn(pci_epc_match_parent, desc->dev);
+	if (!epc)
+		return;
+
+	/* Only support one EPF for doorbell */
+	epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list);
+
+	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_epc_free_doorbell(struct pci_epc *epc, struct pci_epf *epf)
+{
+	int i;
+
+	guard(mutex)(&epc->lock);
+
+	for (i = 0; i < epf->num_db && epf->db_msg[i].virq; i++) {
+		free_irq(epf->db_msg[i].virq, epf);
+		epf->db_msg[i].virq = 0;
+		kfree(epf->db_msg[i].name);
+		epf->db_msg[i].name = NULL;
+	}
+
+	platform_device_msi_free_irqs_all(epc->dev.parent);
+}
+
+static int pci_epc_alloc_doorbell(struct pci_epc *epc, struct pci_epf *epf)
+{
+	struct device *dev = epc->dev.parent;
+	u16 num_db = epf->num_db;
+	int i = 0;
+	int ret;
+
+	guard(mutex)(&epc->lock);
+
+	ret = platform_device_msi_init_and_alloc_irqs(dev, num_db, pci_epc_write_msi_msg);
+	if (ret) {
+		dev_err(dev, "Failed to allocate MSI\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_db; i++) {
+		epf->db_msg[i].virq = msi_get_virq(dev, i);
+		epf->db_msg[i].name = kasprintf(GFP_KERNEL, "pci-epc-doorbell%d", i);
+		ret = request_irq(epf->db_msg[i].virq, pci_epf_doorbell_handler, 0,
+				  epf->db_msg[i].name, epf);
+		if (ret) {
+			dev_err(dev, "Failed to request doorbell\n");
+			pci_epc_free_doorbell(epc, epf);
+			return ret;
+		}
+	}
+
+	return 0;
+};
+
+int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
+{
+	struct pci_epc *epc = epf->epc;
+	void *msg;
+	int ret;
+
+	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_epc_alloc_doorbell(epc, epf);
+	if (ret)
+		kfree(msg);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
+
+void pci_epf_free_doorbell(struct pci_epf *epf)
+{
+	struct pci_epc *epc = epf->epc;
+
+	pci_epc_free_doorbell(epc, epf);
+
+	kfree(epf->db_msg);
+	epf->db_msg = NULL;
+	epf->num_db = 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
new file mode 100644
index 0000000000000..f0cfecf491199
--- /dev/null
+++ b/include/linux/pci-ep-msi.h
@@ -0,0 +1,15 @@
+/* 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__
+
+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..50c0f877f2efb 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;
@@ -75,6 +76,7 @@ struct pci_epf_ops {
  * @link_up: Callback for the EPC link up event
  * @link_down: Callback for the EPC link down event
  * @bus_master_enable: Callback for the EPC Bus Master Enable event
+ * @doorbell: Callback for EPC receive MSI from RC side
  */
 struct pci_epc_event_ops {
 	int (*epc_init)(struct pci_epf *epf);
@@ -82,6 +84,7 @@ struct pci_epc_event_ops {
 	int (*link_up)(struct pci_epf *epf);
 	int (*link_down)(struct pci_epf *epf);
 	int (*bus_master_enable)(struct pci_epf *epf);
+	int (*doorbell)(struct pci_epf *epf, int index);
 };
 
 /**
@@ -125,6 +128,18 @@ 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;
+	char *name;
+};
+
 /**
  * struct pci_epf - represents the PCI EPF device
  * @dev: the PCI EPF device
@@ -152,6 +167,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 +199,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

[PATCH v4 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support

[Frank Li]

Add three registers: doorbell_bar, doorbell_addr, and doorbell_data,
along with doorbell_done. 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 doorbell_done in the doorbell callback to indicate completion.

To avoid broken compatibility, 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.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
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 | 104 ++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 7c2ed6eae53ad..dcb69921497fd 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_addr;
+	u32	doorbell_data;
 } __packed;
 
 static struct pci_epf_header test_header = {
@@ -630,6 +642,60 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 	}
 }
 
+static void pci_epf_enable_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;
+	struct pci_epf_bar db_bar;
+	struct msi_msg *msg;
+	u64 doorbell_addr;
+	u32 align;
+	int ret;
+
+	align = epf_test->epc_features->align;
+	align = align ? align : 128;
+
+	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		return;
+	}
+
+	msg = &epf->db_msg[0].msg;
+	doorbell_addr = msg->address_hi;
+	doorbell_addr <<= 32;
+	doorbell_addr |= msg->address_lo;
+
+	db_bar.phys_addr = round_down(doorbell_addr, align);
+	db_bar.barno = bar;
+	db_bar.size = epf->bar[bar].size;
+	db_bar.flags = epf->bar[bar].flags;
+	db_bar.addr = NULL;
+
+	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &db_bar);
+	if (!ret)
+		reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS;
+}
+
+static void pci_epf_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;
+}
+
 static void pci_epf_test_cmd_handler(struct work_struct *work)
 {
 	u32 command;
@@ -676,6 +742,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_enable_doorbell(epf_test, reg);
+		pci_epf_test_raise_irq(epf_test, reg);
+		break;
+	case COMMAND_DISABLE_DOORBELL:
+		pci_epf_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;
@@ -810,11 +884,24 @@ static int pci_epf_test_link_down(struct pci_epf *epf)
 	return 0;
 }
 
+static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
+{
+	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+	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 0;
+}
+
 static const struct pci_epc_event_ops pci_epf_test_event_ops = {
 	.epc_init = pci_epf_test_epc_init,
 	.epc_deinit = pci_epf_test_epc_deinit,
 	.link_up = pci_epf_test_link_up,
 	.link_down = pci_epf_test_link_down,
+	.doorbell = pci_epf_test_doorbell,
 };
 
 static int pci_epf_test_alloc_space(struct pci_epf *epf)
@@ -909,6 +996,23 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 	if (ret)
 		return ret;
 
+	ret = pci_epf_alloc_doorbell(epf, 1);
+	if (!ret) {
+		struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+		struct msi_msg *msg = &epf->db_msg[0].msg;
+		u32 align = epc_features->align;
+		u64 doorbell_addr;
+
+		align = align ? align : 128;
+		doorbell_addr = msg->address_hi;
+		doorbell_addr <<= 32;
+		doorbell_addr |= msg->address_lo;
+
+		reg->doorbell_addr = doorbell_addr & (align - 1);
+		reg->doorbell_data = msg->data;
+		reg->doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
+	}
+
 	return 0;
 }
 

-- 
2.34.1

[PATCH v4 4/5] misc: pci_endpoint_test: Add doorbell test case

[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_ADDR 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.

		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.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v3 to v4
- Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
- Remove new DID requirement.
---
 drivers/misc/pci_endpoint_test.c | 63 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/pcitest.h     |  1 +
 2 files changed, 64 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 3aaaf47fa4ee2..d8193626c8965 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,7 +74,12 @@
 #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_ADDR		0x34
+#define PCI_ENDPOINT_TEST_DB_DATA		0x38
+
 #define FLAG_USE_DMA				BIT(0)
+#define FLAG_SUPPORT_DOORBELL			BIT(1)
 
 #define PCI_DEVICE_ID_TI_AM654			0xb00c
 #define PCI_DEVICE_ID_TI_J7200			0xb00f
@@ -75,6 +87,7 @@
 #define PCI_DEVICE_ID_TI_J721S2		0xb013
 #define PCI_DEVICE_ID_LS1088A			0x80c0
 #define PCI_DEVICE_ID_IMX8			0x0808
+#define PCI_DEVICE_ID_IMX8_DB			0x080c
 
 #define is_am654_pci_dev(pdev)		\
 		((pdev)->device == PCI_DEVICE_ID_TI_AM654)
@@ -108,6 +121,7 @@ enum pci_barno {
 	BAR_3,
 	BAR_4,
 	BAR_5,
+	NO_BAR = -1,
 };
 
 struct pci_endpoint_test {
@@ -746,6 +760,52 @@ 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)
+{
+	enum pci_barno bar;
+	u32 data, status;
+	u32 addr;
+
+	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)
+		return false;
+
+	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
+	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_ADDR);
+	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);
+
+	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_SUCCESS) &&
+	    (status & STATUS_DOORBELL_DISABLE_SUCCESS))
+		return true;
+
+	return false;
+}
+
 static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 				    unsigned long arg)
 {
@@ -793,6 +853,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..06d9f548b510e 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -21,6 +21,7 @@
 #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
 #define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
 #define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
+#define PCITEST_DOORBELL	_IO('P', 0x11)
 
 #define PCITEST_FLAGS_USE_DMA	0x00000001
 

-- 
2.34.1

[PATCH v4 5/5] tools: PCI: Add 'B' option for test doorbell

[Frank Li]

Add doorbell test support.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- 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 470258009ddc2..bbe26ebbfd945 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

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Niklas Cassel]

Hello Frank,

On Thu, Oct 31, 2024 at 12:26:59PM -0400, Frank Li wrote:
> ┌────────────┐   ┌───────────────────────────────────┐   ┌────────────────┐
> │            │   │                                   │   │                │
> │            │   │ 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 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

Perhaps drop these steps, to not confuse the reader.
They are no longer relevant with the latest revision anyway.

> 
> - 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.

I wanted to try this series on rk3588, which has a MSI controller as part of the GIC
using Interrupt Translation Services (ITS), for the Root Complex DT node:
https://github.com/torvalds/linux/blob/v6.12-rc6/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi#L164
https://github.com/torvalds/linux/blob/v6.12-rc6/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi#L1981-L1999

There isn't any reference to a MSI controller in the Endpoint DT node:
https://github.com/torvalds/linux/blob/v6.12-rc6/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi#L189

When testing your series, I get an error in
platform_device_msi_init_and_alloc_irqs(), because no domain is found.

I assume that is it "msi-parent" that we should add here.

However, looking at:
$ git grep -p msi-parent arch/arm64/boot/dts/freescale/

I do not see any freescale/iMX platform specifying a msi-parent for the EP node.

Is there any additional DTS changes needed for iMX that is not part of this series,
or what am I missing?

When adding "msi-parent = <&its1 0x0000>;
to the EP node
(this is just a guess since RC node has: msi-map = <0x0000 &its1 0x0000 0x1000>;)

I do get a domain, but I do not get any IRQ on the EP side when the RC side is
writing the doorbell (as part of pcitest -B),

[    7.978984] pci_epc_alloc_doorbell: num_db: 1
[    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
[    7.979978] pci_epf_test_bind: doorbell_data: 0x0
[    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
[   21.114613] pci_epf_enable_doorbell db_bar: 1
[   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
[   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
[   21.115994] pci_epf_enable_doorbell: success

# cat /proc/interrupts | grep epc
117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0

Even if I try to write the doorbell manually from EP side using devmem:

# devmem 0xfe670040 32 1

it still doesn't trigger a IRQ on the EP side:
# cat /proc/interrupts | grep epc
117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0

Any ideas?


Kind regards,
Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Niklas Cassel]

On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> 
> I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> writing the doorbell (as part of pcitest -B),
> 
> [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> [   21.114613] pci_epf_enable_doorbell db_bar: 1
> [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> [   21.115994] pci_epf_enable_doorbell: success
> 
> # cat /proc/interrupts | grep epc
> 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> 
> Even if I try to write the doorbell manually from EP side using devmem:
> 
> # devmem 0xfe670040 32 1

Sorry, this should of course have been:
# devmem 0xfe650040 32 1

But the result is the name, no IRQ triggered on the EP side.

(My command above was from testing "msi-parent = <&its0 0x0000>",
rather than &its1, but that also didn't work when writing the
corresponding "doorbell_addr" using e.g. devmem.)

Considering that the RC node is using &its1, that is probably
also what should be used in the EP node when running the controller
in EP mode instead of RC mode.


Kind regards,
Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Frank Li]

On Wed, Nov 06, 2024 at 10:36:42AM +0100, Niklas Cassel wrote:
> On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> >
> > I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> > writing the doorbell (as part of pcitest -B),
> >
> > [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> > [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> > [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> > [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> > [   21.114613] pci_epf_enable_doorbell db_bar: 1
> > [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> > [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> > [   21.115994] pci_epf_enable_doorbell: success
> >
> > # cat /proc/interrupts | grep epc
> > 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> >
> > Even if I try to write the doorbell manually from EP side using devmem:
> >
> > # devmem 0xfe670040 32 1
>
> Sorry, this should of course have been:
> # devmem 0xfe650040 32 1

Thank you test it. You can't write it at EP side. ITS identify the bus
master. master ID (streamid) of CPU is the diffference with PCI master's
ID (streamid). You set msi-parent = <&its0 0x0000>, not sure if 0x0000 is
validate stream.

You have to run at RC side, "devmem (Bar1+0x40) 32 0".  So PCIe EP
controller can use EP streamid.

some system need special register to config stream id, you can refer host
side's settings.

Frank

>
> But the result is the name, no IRQ triggered on the EP side.
>
> (My command above was from testing "msi-parent = <&its0 0x0000>",
> rather than &its1, but that also didn't work when writing the
> corresponding "doorbell_addr" using e.g. devmem.)

<&its0 0x0000>,  second argument is your PCIe controller's stream ID. You
can ref RC side.

>
> Considering that the RC node is using &its1, that is probably
> also what should be used in the EP node when running the controller
> in EP mode instead of RC mode.

Generally,  RC node should use smmu-map, instead &its1. Or your PCI
controller direct use 16bit RID as streamid.

>
>
> Kind regards,
> Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Niklas Cassel]

On Wed, Nov 06, 2024 at 12:13:09PM -0500, Frank Li wrote:
> On Wed, Nov 06, 2024 at 10:36:42AM +0100, Niklas Cassel wrote:
> > On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> > >
> > > I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> > > writing the doorbell (as part of pcitest -B),
> > >
> > > [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> > > [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> > > [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> > > [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> > > [   21.114613] pci_epf_enable_doorbell db_bar: 1
> > > [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> > > [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> > > [   21.115994] pci_epf_enable_doorbell: success
> > >
> > > # cat /proc/interrupts | grep epc
> > > 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> > >
> > > Even if I try to write the doorbell manually from EP side using devmem:
> > >
> > > # devmem 0xfe670040 32 1
> >
> > Sorry, this should of course have been:
> > # devmem 0xfe650040 32 1
> 
> Thank you test it. You can't write it at EP side. ITS identify the bus
> master. master ID (streamid) of CPU is the diffference with PCI master's
> ID (streamid). You set msi-parent = <&its0 0x0000>, not sure if 0x0000 is
> validate stream.

I see, this makes sense since the ITS converts BDF to an MSI specifier.


> 
> You have to run at RC side, "devmem (Bar1+0x40) 32 0".  So PCIe EP
> controller can use EP streamid.
> 
> some system need special register to config stream id, you can refer host
> side's settings.

> <&its0 0x0000>,  second argument is your PCIe controller's stream ID. You
> can ref RC side.

The RC node looks like this:
msi-map = <0x0000 &its1 0x0000 0x1000>;
So it does indeed use 0x0 as the MSI specifier.


> 
> >
> > Considering that the RC node is using &its1, that is probably
> > also what should be used in the EP node when running the controller
> > in EP mode instead of RC mode.
> 
> Generally,  RC node should use smmu-map, instead &its1. Or your PCI
> controller direct use 16bit RID as streamid.

smmu-map? Do you mean iommu-map?

I don't see why we would need to have the SMMU enabled to use ITS.
The iommu is currently disabled on my platform.

I did enable the iommu, and all BAR tests, read tests, write tests,
and copy tests pass. However I get an iommu error when the RC is
writing the doorbell. Perhaps you need to do dma_map_single() on
the address that you are setting the inbound address translation to?



Without the IOMMU, if I modify pci_endpoint_test.c to not send the
DISABLE_DOORBELL command on error (so that EP side still has DB enabled),
I can read all BARs except BAR1 (which was used for the doorbell):
[   21.077417] pci 0000:01:00.0: BAR 0 [mem 0xf0300000-0xf03fffff]: assigned
[   21.078029] pci 0000:01:00.0: BAR 1 [mem 0xf0400000-0xf04fffff]: assigned
[   21.078640] pci 0000:01:00.0: BAR 2 [mem 0xf0500000-0xf05fffff]: assigned
[   21.079250] pci 0000:01:00.0: BAR 3 [mem 0xf0600000-0xf06fffff]: assigned
[   21.079860] pci 0000:01:00.0: BAR 5 [mem 0xf0700000-0xf07fffff]: assigned
# pcitest -B
[   25.156623] COMMAND_ENABLE_DOORBELL complete - status: 0x440
[   25.157131] db_bar: 1 addr: 0x40 data: 0x0
[   25.157501] setting irq_type to: 1
[   25.157802] writing: 0x0 to offset: 0x40 in BAR: 1
[   35.300676] we wrote to the BAR, status is now: 0x0

status is not updated after writing to DB,
and /proc/interrupts on EP side is not incrementing.

# devmem 0xf0300000
0x00000000
# devmem 0xf0400040
0xFFFFFFFF
# devmem 0xf0500000
0x00000000
# devmem 0xf0600000
0x00000000
# devmem 0xf0700000
0x00000000

So there does seem to be something wrong with the inbound translation,
at least when testing on rk3588 which only uses 1MB fixed size BARs:
https://github.com/torvalds/linux/blob/v6.12-rc6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L276-L281



You also didn't answer which imx platform that you are using to test this,
I don't see a single imx platform that specifies "msi-parent".


Kind regards,
Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Frank Li]

On Thu, Nov 07, 2024 at 12:57:16AM +0100, Niklas Cassel wrote:
> On Wed, Nov 06, 2024 at 12:13:09PM -0500, Frank Li wrote:
> > On Wed, Nov 06, 2024 at 10:36:42AM +0100, Niklas Cassel wrote:
> > > On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> > > >
> > > > I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> > > > writing the doorbell (as part of pcitest -B),
> > > >
> > > > [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> > > > [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> > > > [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> > > > [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> > > > [   21.114613] pci_epf_enable_doorbell db_bar: 1
> > > > [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> > > > [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> > > > [   21.115994] pci_epf_enable_doorbell: success
> > > >
> > > > # cat /proc/interrupts | grep epc
> > > > 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> > > >
> > > > Even if I try to write the doorbell manually from EP side using devmem:
> > > >
> > > > # devmem 0xfe670040 32 1
> > >
> > > Sorry, this should of course have been:
> > > # devmem 0xfe650040 32 1
> >
> > Thank you test it. You can't write it at EP side. ITS identify the bus
> > master. master ID (streamid) of CPU is the diffference with PCI master's
> > ID (streamid). You set msi-parent = <&its0 0x0000>, not sure if 0x0000 is
> > validate stream.
>
> I see, this makes sense since the ITS converts BDF to an MSI specifier.
>
>
> >
> > You have to run at RC side, "devmem (Bar1+0x40) 32 0".  So PCIe EP
> > controller can use EP streamid.
> >
> > some system need special register to config stream id, you can refer host
> > side's settings.
>
> > <&its0 0x0000>,  second argument is your PCIe controller's stream ID. You
> > can ref RC side.
>
> The RC node looks like this:
> msi-map = <0x0000 &its1 0x0000 0x1000>;
> So it does indeed use 0x0 as the MSI specifier.
>
>
> >
> > >
> > > Considering that the RC node is using &its1, that is probably
> > > also what should be used in the EP node when running the controller
> > > in EP mode instead of RC mode.
> >
> > Generally,  RC node should use smmu-map, instead &its1. Or your PCI
> > controller direct use 16bit RID as streamid.
>
> smmu-map? Do you mean iommu-map?

Sorry, typo, my means msi-map.

>
> I don't see why we would need to have the SMMU enabled to use ITS.
> The iommu is currently disabled on my platform.
>
> I did enable the iommu, and all BAR tests, read tests, write tests,
> and copy tests pass. However I get an iommu error when the RC is
> writing the doorbell. Perhaps you need to do dma_map_single() on
> the address that you are setting the inbound address translation to?
>
>
>
> Without the IOMMU, if I modify pci_endpoint_test.c to not send the
> DISABLE_DOORBELL command on error (so that EP side still has DB enabled),
> I can read all BARs except BAR1 (which was used for the doorbell):
> [   21.077417] pci 0000:01:00.0: BAR 0 [mem 0xf0300000-0xf03fffff]: assigned
> [   21.078029] pci 0000:01:00.0: BAR 1 [mem 0xf0400000-0xf04fffff]: assigned
> [   21.078640] pci 0000:01:00.0: BAR 2 [mem 0xf0500000-0xf05fffff]: assigned
> [   21.079250] pci 0000:01:00.0: BAR 3 [mem 0xf0600000-0xf06fffff]: assigned
> [   21.079860] pci 0000:01:00.0: BAR 5 [mem 0xf0700000-0xf07fffff]: assigned
> # pcitest -B
> [   25.156623] COMMAND_ENABLE_DOORBELL complete - status: 0x440
> [   25.157131] db_bar: 1 addr: 0x40 data: 0x0
> [   25.157501] setting irq_type to: 1
> [   25.157802] writing: 0x0 to offset: 0x40 in BAR: 1
> [   35.300676] we wrote to the BAR, status is now: 0x0
>
> status is not updated after writing to DB,
> and /proc/interrupts on EP side is not incrementing.
>
> # devmem 0xf0300000
> 0x00000000
> # devmem 0xf0400040
> 0xFFFFFFFF
> # devmem 0xf0500000
> 0x00000000
> # devmem 0xf0600000
> 0x00000000
> # devmem 0xf0700000
> 0x00000000
>
> So there does seem to be something wrong with the inbound translation,
> at least when testing on rk3588 which only uses 1MB fixed size BARs:
> https://github.com/torvalds/linux/blob/v6.12-rc6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L276-L281
>

It should be fine.  Some hardware many append some stream id bits before
send to ITS.

>
>
> You also didn't answer which imx platform that you are using to test this,
> I don't see a single imx platform that specifies "msi-parent".

Only imx95 support its now. LUT part code is still under review. EP support
part should after lut merged. Preivous use layerscape, which uboot set it.

Frank

>
>
> Kind regards,
> Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Frank Li]

On Thu, Nov 07, 2024 at 12:57:16AM +0100, Niklas Cassel wrote:
> On Wed, Nov 06, 2024 at 12:13:09PM -0500, Frank Li wrote:
> > On Wed, Nov 06, 2024 at 10:36:42AM +0100, Niklas Cassel wrote:
> > > On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> > > >
> > > > I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> > > > writing the doorbell (as part of pcitest -B),
> > > >
> > > > [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> > > > [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> > > > [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> > > > [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> > > > [   21.114613] pci_epf_enable_doorbell db_bar: 1
> > > > [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> > > > [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> > > > [   21.115994] pci_epf_enable_doorbell: success
> > > >
> > > > # cat /proc/interrupts | grep epc
> > > > 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> > > >
> > > > Even if I try to write the doorbell manually from EP side using devmem:
> > > >
> > > > # devmem 0xfe670040 32 1
> > >
> > > Sorry, this should of course have been:
> > > # devmem 0xfe650040 32 1
> >
> > Thank you test it. You can't write it at EP side. ITS identify the bus
> > master. master ID (streamid) of CPU is the diffference with PCI master's
> > ID (streamid). You set msi-parent = <&its0 0x0000>, not sure if 0x0000 is
> > validate stream.
>
> I see, this makes sense since the ITS converts BDF to an MSI specifier.
>
>
> >
> > You have to run at RC side, "devmem (Bar1+0x40) 32 0".  So PCIe EP
> > controller can use EP streamid.
> >
> > some system need special register to config stream id, you can refer host
> > side's settings.
>
> > <&its0 0x0000>,  second argument is your PCIe controller's stream ID. You
> > can ref RC side.
>
> The RC node looks like this:
> msi-map = <0x0000 &its1 0x0000 0x1000>;
> So it does indeed use 0x0 as the MSI specifier.
>
>
> >
> > >
> > > Considering that the RC node is using &its1, that is probably
> > > also what should be used in the EP node when running the controller
> > > in EP mode instead of RC mode.
> >
> > Generally,  RC node should use smmu-map, instead &its1. Or your PCI
> > controller direct use 16bit RID as streamid.
>
> smmu-map? Do you mean iommu-map?
>
> I don't see why we would need to have the SMMU enabled to use ITS.
> The iommu is currently disabled on my platform.
>
> I did enable the iommu, and all BAR tests, read tests, write tests,
> and copy tests pass. However I get an iommu error when the RC is
> writing the doorbell. Perhaps you need to do dma_map_single() on
> the address that you are setting the inbound address translation to?
>
>
>
> Without the IOMMU, if I modify pci_endpoint_test.c to not send the
> DISABLE_DOORBELL command on error (so that EP side still has DB enabled),
> I can read all BARs except BAR1 (which was used for the doorbell):

If you enable IOMMU, please double check pci_epc_write_msi_msg() pass
down iommu mapped address. I have not test iommu enabled's case yet.

Frank

> [   21.077417] pci 0000:01:00.0: BAR 0 [mem 0xf0300000-0xf03fffff]: assigned
> [   21.078029] pci 0000:01:00.0: BAR 1 [mem 0xf0400000-0xf04fffff]: assigned
> [   21.078640] pci 0000:01:00.0: BAR 2 [mem 0xf0500000-0xf05fffff]: assigned
> [   21.079250] pci 0000:01:00.0: BAR 3 [mem 0xf0600000-0xf06fffff]: assigned
> [   21.079860] pci 0000:01:00.0: BAR 5 [mem 0xf0700000-0xf07fffff]: assigned
> # pcitest -B
> [   25.156623] COMMAND_ENABLE_DOORBELL complete - status: 0x440
> [   25.157131] db_bar: 1 addr: 0x40 data: 0x0
> [   25.157501] setting irq_type to: 1
> [   25.157802] writing: 0x0 to offset: 0x40 in BAR: 1
> [   35.300676] we wrote to the BAR, status is now: 0x0
>
> status is not updated after writing to DB,
> and /proc/interrupts on EP side is not incrementing.
>
> # devmem 0xf0300000
> 0x00000000
> # devmem 0xf0400040
> 0xFFFFFFFF
> # devmem 0xf0500000
> 0x00000000
> # devmem 0xf0600000
> 0x00000000
> # devmem 0xf0700000
> 0x00000000
>
> So there does seem to be something wrong with the inbound translation,
> at least when testing on rk3588 which only uses 1MB fixed size BARs:
> https://github.com/torvalds/linux/blob/v6.12-rc6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L276-L281
>
>
>
> You also didn't answer which imx platform that you are using to test this,
> I don't see a single imx platform that specifies "msi-parent".
>
>
> Kind regards,
> Niklas

Re: [PATCH v4 4/5] misc: pci_endpoint_test: Add doorbell test case

[Niklas Cassel]

On Thu, Oct 31, 2024 at 12:27:03PM -0400, Frank Li wrote:
> 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_ADDR 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.
> 
> 		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.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v3 to v4
> - Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
> - Remove new DID requirement.
> ---
>  drivers/misc/pci_endpoint_test.c | 63 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/pcitest.h     |  1 +
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 3aaaf47fa4ee2..d8193626c8965 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,7 +74,12 @@
>  #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_ADDR		0x34
> +#define PCI_ENDPOINT_TEST_DB_DATA		0x38
> +
>  #define FLAG_USE_DMA				BIT(0)
> +#define FLAG_SUPPORT_DOORBELL			BIT(1)

Unused.


>  
>  #define PCI_DEVICE_ID_TI_AM654			0xb00c
>  #define PCI_DEVICE_ID_TI_J7200			0xb00f
> @@ -75,6 +87,7 @@
>  #define PCI_DEVICE_ID_TI_J721S2		0xb013
>  #define PCI_DEVICE_ID_LS1088A			0x80c0
>  #define PCI_DEVICE_ID_IMX8			0x0808
> +#define PCI_DEVICE_ID_IMX8_DB			0x080c

Unused.


>  
>  #define is_am654_pci_dev(pdev)		\
>  		((pdev)->device == PCI_DEVICE_ID_TI_AM654)
> @@ -108,6 +121,7 @@ enum pci_barno {
>  	BAR_3,
>  	BAR_4,
>  	BAR_5,
> +	NO_BAR = -1,
>  };
>  
>  struct pci_endpoint_test {
> @@ -746,6 +760,52 @@ 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)
> +{
> +	enum pci_barno bar;
> +	u32 data, status;
> +	u32 addr;


You need to do:
pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);

Before sending the COMMAND_ENABLE_DOORBELL command.

Otherwise, when EP sends an IRQ in response to COMMAND_ENABLE_DOORBELL
being done, it will send it using type reg->irq_type, which will be 0,
since you haven't initialized it. Thus the EP will trigger a INTx IRQ.


You probably also want to have a variable:
int irq_type = test->irq_type;

So that you initialize irq_type to test->irq_type, instead of using the
global variable irq_type.

(This matches how it is done in pci_endpoint_test_write(),
pci_endpoint_test_read(), and pci_endpoint_test_write().)


> +
> +	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)
> +		return false;
> +
> +	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
> +	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_ADDR);
> +	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);
> +
> +	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_SUCCESS) &&
> +	    (status & STATUS_DOORBELL_DISABLE_SUCCESS))
> +		return true;
> +
> +	return false;
> +}
> +
>  static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
>  				    unsigned long arg)
>  {
> @@ -793,6 +853,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..06d9f548b510e 100644
> --- a/include/uapi/linux/pcitest.h
> +++ b/include/uapi/linux/pcitest.h
> @@ -21,6 +21,7 @@
>  #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
>  #define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
>  #define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
> +#define PCITEST_DOORBELL	_IO('P', 0x11)
>  
>  #define PCITEST_FLAGS_USE_DMA	0x00000001
>  
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support

[Niklas Cassel]

On Thu, Oct 31, 2024 at 12:27:02PM -0400, Frank Li wrote:
> Add three registers: doorbell_bar, doorbell_addr, and doorbell_data,
> along with doorbell_done. 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 doorbell_done in the doorbell callback to indicate completion.
> 
> To avoid broken compatibility, 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.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 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 | 104 ++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 7c2ed6eae53ad..dcb69921497fd 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_addr;
> +	u32	doorbell_data;
>  } __packed;
>  
>  static struct pci_epf_header test_header = {
> @@ -630,6 +642,60 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
>  	}
>  }
>  
> +static void pci_epf_enable_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;
> +	struct pci_epf_bar db_bar;
> +	struct msi_msg *msg;
> +	u64 doorbell_addr;
> +	u32 align;
> +	int ret;
> +
> +	align = epf_test->epc_features->align;
> +	align = align ? align : 128;
> +
> +	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
> +		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		return;
> +	}
> +
> +	msg = &epf->db_msg[0].msg;
> +	doorbell_addr = msg->address_hi;
> +	doorbell_addr <<= 32;
> +	doorbell_addr |= msg->address_lo;
> +
> +	db_bar.phys_addr = round_down(doorbell_addr, align);
> +	db_bar.barno = bar;
> +	db_bar.size = epf->bar[bar].size;
> +	db_bar.flags = epf->bar[bar].flags;
> +	db_bar.addr = NULL;
> +
> +	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &db_bar);
> +	if (!ret)
> +		reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS;
> +}
> +
> +static void pci_epf_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;
> +}
> +
>  static void pci_epf_test_cmd_handler(struct work_struct *work)
>  {
>  	u32 command;
> @@ -676,6 +742,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_enable_doorbell(epf_test, reg);
> +		pci_epf_test_raise_irq(epf_test, reg);
> +		break;
> +	case COMMAND_DISABLE_DOORBELL:
> +		pci_epf_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;
> @@ -810,11 +884,24 @@ static int pci_epf_test_link_down(struct pci_epf *epf)
>  	return 0;
>  }
>  
> +static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
> +{
> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +	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 0;
> +}
> +
>  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
>  	.epc_init = pci_epf_test_epc_init,
>  	.epc_deinit = pci_epf_test_epc_deinit,
>  	.link_up = pci_epf_test_link_up,
>  	.link_down = pci_epf_test_link_down,
> +	.doorbell = pci_epf_test_doorbell,
>  };
>  
>  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> @@ -909,6 +996,23 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	if (ret)
>  		return ret;
>  
> +	ret = pci_epf_alloc_doorbell(epf, 1);

Calling pci_epf_alloc_doorbell() unconditionally from bind will lead to the
following print for all platforms that have not configured a msi-parent:
[   64.543388] a40000000.pcie-ep: Failed to allocate MSI

In ealier discussions, I thought that you wanted to call
pci_epf_alloc_doorbell() in pci_epf_enable_doorbell(), and then let
pci_epf_enable_doorbell() return STATUS_DOORBELL_ENABLE_FAIL
if pci_epf_enable_doorbell() failed.


Perhaps you could modify pci_epf_enable_doorbell() to also check if
dev->msi.domain is NULL, before calling pci_epc_alloc_doorbell(),
and if dev->msi.domain is NULL, perhaps print a clearer error,
e.g. "no msi domain found, is 'msi-parent' device tree property missing?"
Or put the text in a comment next to the error check, if you think that a
print seems too silly.


> +	if (!ret) {
> +		struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +		struct msi_msg *msg = &epf->db_msg[0].msg;
> +		u32 align = epc_features->align;
> +		u64 doorbell_addr;
> +
> +		align = align ? align : 128;
> +		doorbell_addr = msg->address_hi;
> +		doorbell_addr <<= 32;
> +		doorbell_addr |= msg->address_lo;
> +
> +		reg->doorbell_addr = doorbell_addr & (align - 1);
> +		reg->doorbell_data = msg->data;
> +		reg->doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
> +	}
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support

[Frank Li]

On Thu, Nov 07, 2024 at 10:52:24PM +0100, Niklas Cassel wrote:
> On Thu, Oct 31, 2024 at 12:27:02PM -0400, Frank Li wrote:
> > Add three registers: doorbell_bar, doorbell_addr, and doorbell_data,
> > along with doorbell_done. 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 doorbell_done in the doorbell callback to indicate completion.
> >
> > To avoid broken compatibility, 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.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 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 | 104 ++++++++++++++++++++++++++
> >  1 file changed, 104 insertions(+)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 7c2ed6eae53ad..dcb69921497fd 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_addr;
> > +	u32	doorbell_data;
> >  } __packed;
> >
> >  static struct pci_epf_header test_header = {
> > @@ -630,6 +642,60 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
> >  	}
> >  }
> >
> > +static void pci_epf_enable_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;
> > +	struct pci_epf_bar db_bar;
> > +	struct msi_msg *msg;
> > +	u64 doorbell_addr;
> > +	u32 align;
> > +	int ret;
> > +
> > +	align = epf_test->epc_features->align;
> > +	align = align ? align : 128;
> > +
> > +	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
> > +		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
> > +		return;
> > +	}
> > +
> > +	msg = &epf->db_msg[0].msg;
> > +	doorbell_addr = msg->address_hi;
> > +	doorbell_addr <<= 32;
> > +	doorbell_addr |= msg->address_lo;
> > +
> > +	db_bar.phys_addr = round_down(doorbell_addr, align);
> > +	db_bar.barno = bar;
> > +	db_bar.size = epf->bar[bar].size;
> > +	db_bar.flags = epf->bar[bar].flags;
> > +	db_bar.addr = NULL;
> > +
> > +	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &db_bar);
> > +	if (!ret)
> > +		reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS;
> > +}
> > +
> > +static void pci_epf_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;
> > +}
> > +
> >  static void pci_epf_test_cmd_handler(struct work_struct *work)
> >  {
> >  	u32 command;
> > @@ -676,6 +742,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_enable_doorbell(epf_test, reg);
> > +		pci_epf_test_raise_irq(epf_test, reg);
> > +		break;
> > +	case COMMAND_DISABLE_DOORBELL:
> > +		pci_epf_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;
> > @@ -810,11 +884,24 @@ static int pci_epf_test_link_down(struct pci_epf *epf)
> >  	return 0;
> >  }
> >
> > +static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
> > +{
> > +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > +	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 0;
> > +}
> > +
> >  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> >  	.epc_init = pci_epf_test_epc_init,
> >  	.epc_deinit = pci_epf_test_epc_deinit,
> >  	.link_up = pci_epf_test_link_up,
> >  	.link_down = pci_epf_test_link_down,
> > +	.doorbell = pci_epf_test_doorbell,
> >  };
> >
> >  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > @@ -909,6 +996,23 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> >  	if (ret)
> >  		return ret;
> >
> > +	ret = pci_epf_alloc_doorbell(epf, 1);
>
> Calling pci_epf_alloc_doorbell() unconditionally from bind will lead to the
> following print for all platforms that have not configured a msi-parent:
> [   64.543388] a40000000.pcie-ep: Failed to allocate MSI
>
> In ealier discussions, I thought that you wanted to call
> pci_epf_alloc_doorbell() in pci_epf_enable_doorbell(), and then let
> pci_epf_enable_doorbell() return STATUS_DOORBELL_ENABLE_FAIL
> if pci_epf_enable_doorbell() failed.
>
>
> Perhaps you could modify pci_epf_enable_doorbell() to also check if
> dev->msi.domain is NULL, before calling pci_epc_alloc_doorbell(),
> and if dev->msi.domain is NULL, perhaps print a clearer error,
> e.g. "no msi domain found, is 'msi-parent' device tree property missing?"
> Or put the text in a comment next to the error check, if you think that a
> print seems too silly.

I think resource should be allocated in bind. it may be too frequent to
allocate and free msi resources when call pci_epf_enable_doorbell()/
pci_epf_disable_doorbell().

If you think "a40000000.pcie-ep: Failed to allocate MSI" a noise, I think
we can add a msi_domain check in pci_epc_alloc_doorbell() and print a nice
message at pci_epc_alloc_doorbell().

It should be similar as eDMA detect. It'd better show captiblity
information at beginning instead of defer to when use it.

Frank
>
>
> > +	if (!ret) {
> > +		struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > +		struct msi_msg *msg = &epf->db_msg[0].msg;
> > +		u32 align = epc_features->align;
> > +		u64 doorbell_addr;
> > +
> > +		align = align ? align : 128;
> > +		doorbell_addr = msg->address_hi;
> > +		doorbell_addr <<= 32;
> > +		doorbell_addr |= msg->address_lo;
> > +
> > +		reg->doorbell_addr = doorbell_addr & (align - 1);
> > +		reg->doorbell_data = msg->data;
> > +		reg->doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
> > +	}
> > +
> >  	return 0;
> >  }
> >
> >
> > --
> > 2.34.1
> >

Re: [PATCH v4 4/5] misc: pci_endpoint_test: Add doorbell test case

[Frank Li]

On Thu, Nov 07, 2024 at 10:42:28PM +0100, Niklas Cassel wrote:
> On Thu, Oct 31, 2024 at 12:27:03PM -0400, Frank Li wrote:
> > 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_ADDR 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.
> >
> > 		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.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change from v3 to v4
> > - Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
> > - Remove new DID requirement.
> > ---
> >  drivers/misc/pci_endpoint_test.c | 63 ++++++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/pcitest.h     |  1 +
> >  2 files changed, 64 insertions(+)
> >
> > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > index 3aaaf47fa4ee2..d8193626c8965 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,7 +74,12 @@
> >  #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_ADDR		0x34
> > +#define PCI_ENDPOINT_TEST_DB_DATA		0x38
> > +
> >  #define FLAG_USE_DMA				BIT(0)
> > +#define FLAG_SUPPORT_DOORBELL			BIT(1)
>
> Unused.
>
>
> >
> >  #define PCI_DEVICE_ID_TI_AM654			0xb00c
> >  #define PCI_DEVICE_ID_TI_J7200			0xb00f
> > @@ -75,6 +87,7 @@
> >  #define PCI_DEVICE_ID_TI_J721S2		0xb013
> >  #define PCI_DEVICE_ID_LS1088A			0x80c0
> >  #define PCI_DEVICE_ID_IMX8			0x0808
> > +#define PCI_DEVICE_ID_IMX8_DB			0x080c
>
> Unused.
>
>
> >
> >  #define is_am654_pci_dev(pdev)		\
> >  		((pdev)->device == PCI_DEVICE_ID_TI_AM654)
> > @@ -108,6 +121,7 @@ enum pci_barno {
> >  	BAR_3,
> >  	BAR_4,
> >  	BAR_5,
> > +	NO_BAR = -1,
> >  };
> >
> >  struct pci_endpoint_test {
> > @@ -746,6 +760,52 @@ 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)
> > +{
> > +	enum pci_barno bar;
> > +	u32 data, status;
> > +	u32 addr;
>
>
> You need to do:
> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
>
> Before sending the COMMAND_ENABLE_DOORBELL command.
>
> Otherwise, when EP sends an IRQ in response to COMMAND_ENABLE_DOORBELL
> being done, it will send it using type reg->irq_type, which will be 0,
> since you haven't initialized it. Thus the EP will trigger a INTx IRQ.
>
>
> You probably also want to have a variable:
> int irq_type = test->irq_type;
>
> So that you initialize irq_type to test->irq_type, instead of using the
> global variable irq_type.
>
> (This matches how it is done in pci_endpoint_test_write(),
> pci_endpoint_test_read(), and pci_endpoint_test_write().)

Thanks, did you trigger EP's MSI at your platform?

Frank
>
>
> > +
> > +	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)
> > +		return false;
> > +
> > +	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
> > +	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_ADDR);
> > +	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);
> > +
> > +	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_SUCCESS) &&
> > +	    (status & STATUS_DOORBELL_DISABLE_SUCCESS))
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
> >  				    unsigned long arg)
> >  {
> > @@ -793,6 +853,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..06d9f548b510e 100644
> > --- a/include/uapi/linux/pcitest.h
> > +++ b/include/uapi/linux/pcitest.h
> > @@ -21,6 +21,7 @@
> >  #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
> >  #define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
> >  #define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
> > +#define PCITEST_DOORBELL	_IO('P', 0x11)
> >
> >  #define PCITEST_FLAGS_USE_DMA	0x00000001
> >
> >
> > --
> > 2.34.1
> >

Re: [PATCH v4 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support

[Niklas Cassel]

On Thu, Nov 07, 2024 at 05:44:22PM -0500, Frank Li wrote:
> On Thu, Nov 07, 2024 at 10:52:24PM +0100, Niklas Cassel wrote:
> > On Thu, Oct 31, 2024 at 12:27:02PM -0400, Frank Li wrote:
> > > Add three registers: doorbell_bar, doorbell_addr, and doorbell_data,
> > > along with doorbell_done. 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 doorbell_done in the doorbell callback to indicate completion.
> > >
> > > To avoid broken compatibility, 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.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > 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 | 104 ++++++++++++++++++++++++++
> > >  1 file changed, 104 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index 7c2ed6eae53ad..dcb69921497fd 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_addr;
> > > +	u32	doorbell_data;
> > >  } __packed;
> > >
> > >  static struct pci_epf_header test_header = {
> > > @@ -630,6 +642,60 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
> > >  	}
> > >  }
> > >
> > > +static void pci_epf_enable_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;
> > > +	struct pci_epf_bar db_bar;
> > > +	struct msi_msg *msg;
> > > +	u64 doorbell_addr;
> > > +	u32 align;
> > > +	int ret;
> > > +
> > > +	align = epf_test->epc_features->align;
> > > +	align = align ? align : 128;
> > > +
> > > +	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
> > > +		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
> > > +		return;
> > > +	}
> > > +
> > > +	msg = &epf->db_msg[0].msg;
> > > +	doorbell_addr = msg->address_hi;
> > > +	doorbell_addr <<= 32;
> > > +	doorbell_addr |= msg->address_lo;
> > > +
> > > +	db_bar.phys_addr = round_down(doorbell_addr, align);
> > > +	db_bar.barno = bar;
> > > +	db_bar.size = epf->bar[bar].size;
> > > +	db_bar.flags = epf->bar[bar].flags;
> > > +	db_bar.addr = NULL;
> > > +
> > > +	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &db_bar);
> > > +	if (!ret)
> > > +		reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS;
> > > +}
> > > +
> > > +static void pci_epf_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;
> > > +}
> > > +
> > >  static void pci_epf_test_cmd_handler(struct work_struct *work)
> > >  {
> > >  	u32 command;
> > > @@ -676,6 +742,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_enable_doorbell(epf_test, reg);
> > > +		pci_epf_test_raise_irq(epf_test, reg);
> > > +		break;
> > > +	case COMMAND_DISABLE_DOORBELL:
> > > +		pci_epf_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;
> > > @@ -810,11 +884,24 @@ static int pci_epf_test_link_down(struct pci_epf *epf)
> > >  	return 0;
> > >  }
> > >
> > > +static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
> > > +{
> > > +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > > +	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 0;
> > > +}
> > > +
> > >  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> > >  	.epc_init = pci_epf_test_epc_init,
> > >  	.epc_deinit = pci_epf_test_epc_deinit,
> > >  	.link_up = pci_epf_test_link_up,
> > >  	.link_down = pci_epf_test_link_down,
> > > +	.doorbell = pci_epf_test_doorbell,
> > >  };
> > >
> > >  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > > @@ -909,6 +996,23 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> > >  	if (ret)
> > >  		return ret;
> > >
> > > +	ret = pci_epf_alloc_doorbell(epf, 1);
> >
> > Calling pci_epf_alloc_doorbell() unconditionally from bind will lead to the
> > following print for all platforms that have not configured a msi-parent:
> > [   64.543388] a40000000.pcie-ep: Failed to allocate MSI
> >
> > In ealier discussions, I thought that you wanted to call
> > pci_epf_alloc_doorbell() in pci_epf_enable_doorbell(), and then let
> > pci_epf_enable_doorbell() return STATUS_DOORBELL_ENABLE_FAIL
> > if pci_epf_enable_doorbell() failed.
> >
> >
> > Perhaps you could modify pci_epf_enable_doorbell() to also check if
> > dev->msi.domain is NULL, before calling pci_epc_alloc_doorbell(),
> > and if dev->msi.domain is NULL, perhaps print a clearer error,
> > e.g. "no msi domain found, is 'msi-parent' device tree property missing?"
> > Or put the text in a comment next to the error check, if you think that a
> > print seems too silly.
> 
> I think resource should be allocated in bind. it may be too frequent to
> allocate and free msi resources when call pci_epf_enable_doorbell()/
> pci_epf_disable_doorbell().
> 
> If you think "a40000000.pcie-ep: Failed to allocate MSI" a noise, I think
> we can add a msi_domain check in pci_epc_alloc_doorbell() and print a nice
> message at pci_epc_alloc_doorbell().
> 
> It should be similar as eDMA detect. It'd better show captiblity
> information at beginning instead of defer to when use it.

Sounds good to me.


Kind regards,
Niklas

Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Niklas Cassel]

On Wed, Nov 06, 2024 at 07:41:42PM -0500, Frank Li wrote:
> >
> > So there does seem to be something wrong with the inbound translation,
> > at least when testing on rk3588 which only uses 1MB fixed size BARs:
> > https://github.com/torvalds/linux/blob/v6.12-rc6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L276-L281
> >
> 
> It should be fine.  Some hardware many append some stream id bits before
> send to ITS.

Some more debugging with the IOMMU on.

EP side start:
[   14.601081] pci_epc_alloc_doorbell: num_db: 1
[   14.601588] pci_epf_test_bind: doorbell_addr: 0xf040 (align: 0x10000)
[   14.602162] pci_epf_test_bind: doorbell_data: 0x0
[   14.602573] pci_epf_test_bind: doorbell_bar: 0x1


When RC side does:
pcitest -B
[  109.252900] COMMAND_ENABLE_DOORBELL complete - status: 0x440
[  109.253406] db_bar: 1 addr: 0xf040 data: 0x0
[  109.254094] writing: 0x0 to offset: 0xf040 in BAR: 1
[  119.268887] we wrote to the BAR, status is now: 0x0

EP side results in:
[  117.894997] pci_epf_enable_doorbell db_bar: 1
[  117.895399] pci_epf_enable_doorbell: using doorbell_addr: 0xffffff9ff040
[  117.896517] pci_epf_enable_doorbell: phys_addr: 0xffffff9f0000
[  117.897037] dw_pcie_ep_set_bar: set_bar: bar: 1 phys_addr: ffffff9f0000 flags: 0x0 size: 0x100000
[  117.898504] pci_epf_enable_doorbell: success
[  118.912433] arm-smmu-v3 fc900000.iommu: event 0x10 received:
[  118.912965] arm-smmu-v3 fc900000.iommu:      0x0000000000000010
[  118.913508] arm-smmu-v3 fc900000.iommu:      0x0000020000000000
[  118.914018] arm-smmu-v3 fc900000.iommu:      0x0000ffffff90f040
[  118.914534] arm-smmu-v3 fc900000.iommu:      0x0000000000000000

Looking at the doorbell_addr, it seems to be a IOMMU address already.

If we look at the ARM SMMU-v3 specification, event 0x10 is:
Translation fault: The address provided to a stage of translation failed the
range check defined by TxSZ/SLx, the address was within a disabled TTBx, or a
valid translation table descriptor was not found for the address.

for event F_TRANSLATION:
0x0000ffffff90f040
is input address.

StreamID is: 0x0, so that looks as expected for rk3588.
(And if the SteamID was incorrect, I would have expected a C_BAD_STREAMID
event instead.)


Comparing the address of the IOMMU error:
0xffffff90f040
with the doorbell addr:
0xffffff9ff040
XOR value:
0x0000000f0000

We can see that they are not identical.


When RC side does:
devmem $((0xf0400000+0xf040)) 32 0
it results in the exact same IOMMU error on the EP side as the one above.

However, if I manually append the XOR value:
devmem $((0xf0400000+0xf040+0xf0000)) 32 0

I can see on the EP side:
[  631.399530] pci_epf_doorbell_handler
[  631.399850] pci_epf_test_doorbell



So why is the inbound translation incorrect?

Like I told you earlier, rk3588 has fixed size 1MB BARs,
so the BAR_MASK will be:
~(SZ_1M-1)
0xfff00000

So the physical address that you write in the iATU:
0xffffff9f0000
will actually be:
0xfffffff00000
after reading back the same register from the iATU,
since the lower bits will be masked away.

I'm guessing that you would need to do something like:
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index e5b6a65e3e16f..0ab5d61bf0493 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -675,6 +675,9 @@ static void pci_epf_enable_doorbell(struct pci_epf_test *epf_test, struct pci_ep
                return;
        }
 
+       if (epf_test->epc_features->bar[bar].type == BAR_FIXED)
+               align = max(epf_test->epc_features->bar[bar].fixed_size, align);
+
        msg = &epf->db_msg[0].msg;
        doorbell_addr = msg->address_hi;
        doorbell_addr <<= 32;
@@ -1016,15 +1021,22 @@ static int pci_epf_test_bind(struct pci_epf *epf)
                struct msi_msg *msg = &epf->db_msg[0].msg;
                u32 align = epc_features->align;
                u64 doorbell_addr;
+               enum pci_barno bar;
+
+               bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
 
                align = align ? align : 128;
+               if (epf_test->epc_features->bar[bar].type == BAR_FIXED)
+                       align = max(epf_test->epc_features->bar[bar].fixed_size,
+                                   align);
+
                doorbell_addr = msg->address_hi;
                doorbell_addr <<= 32;
                doorbell_addr |= msg->address_lo;
 
                reg->doorbell_addr = doorbell_addr & (align - 1);
                reg->doorbell_data = msg->data;
-               reg->doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
+               reg->doorbell_bar = bar;
        }
 
        return 0;



I tested the above on top of your series, and now
pcitest -B
works as expected :) yay!


Kind regards,
Niklas

Re: [PATCH v4 2/5] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[Krishna Chaitanya Chundru]

On 10/31/2024 9:57 PM, Frank Li wrote:
> 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.
>
Hi Frank,

Currently you are using single doorbell callback for all MSI's
received from the host side.
Instead of that can we expose a API which will be used by EPF driver
to request for IRQ by passing there own irq handler. we can skip
request_irq in epc_alloc_doorbell expose a seperate API for the
EPF driver to request_IRQ or let the EPF driver do request_irq by
filling  epf->db_msg[i].virq

- Krishna Chaitanya.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v3 to v4
> - msi change to use msi_get_virq() avoid use msi_for_each_desc().
> - add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
> - move mutex lock to epc function
> - initialize variable at declear place.
> - passdown epf to epc*() function to simplify code.
> ---
>   drivers/pci/endpoint/Makefile     |   2 +-
>   drivers/pci/endpoint/pci-ep-msi.c | 128 ++++++++++++++++++++++++++++++++++++++
>   include/linux/pci-ep-msi.h        |  15 +++++
>   include/linux/pci-epf.h           |  19 ++++++
>   4 files changed, 163 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..91207fb66db32
> --- /dev/null
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -0,0 +1,128 @@
> +// 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/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/msi.h>
> +#include <linux/pci-epc.h>
> +#include <linux/pci-epf.h>
> +#include <linux/pci-ep-cfs.h>
> +#include <linux/pci-ep-msi.h>
> +
> +static irqreturn_t pci_epf_doorbell_handler(int irq, void *data)
> +{
> +	struct pci_epf *epf = data;
> +	struct msi_desc *desc = irq_get_msi_desc(irq);
> +
> +	if (desc && epf->event_ops && epf->event_ops->doorbell)
> +		epf->event_ops->doorbell(epf, desc->msi_index);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static bool pci_epc_match_parent(struct device *dev, void *param)
> +{
> +	return dev->parent == param;
> +}
> +
> +static void pci_epc_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> +{
> +	struct pci_epc *epc __free(pci_epc_put) = NULL;
> +	struct pci_epf *epf;
> +
> +	epc = pci_epc_get_fn(pci_epc_match_parent, desc->dev);
> +	if (!epc)
> +		return;
> +
> +	/* Only support one EPF for doorbell */
> +	epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list);
> +
> +	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_epc_free_doorbell(struct pci_epc *epc, struct pci_epf *epf)
> +{
> +	int i;
> +
> +	guard(mutex)(&epc->lock);
> +
> +	for (i = 0; i < epf->num_db && epf->db_msg[i].virq; i++) {
> +		free_irq(epf->db_msg[i].virq, epf);
> +		epf->db_msg[i].virq = 0;
> +		kfree(epf->db_msg[i].name);
> +		epf->db_msg[i].name = NULL;
> +	}
> +
> +	platform_device_msi_free_irqs_all(epc->dev.parent);
> +}
> +
> +static int pci_epc_alloc_doorbell(struct pci_epc *epc, struct pci_epf *epf)
> +{
> +	struct device *dev = epc->dev.parent;
> +	u16 num_db = epf->num_db;
> +	int i = 0;
> +	int ret;
> +
> +	guard(mutex)(&epc->lock);
> +
> +	ret = platform_device_msi_init_and_alloc_irqs(dev, num_db, pci_epc_write_msi_msg);
> +	if (ret) {
> +		dev_err(dev, "Failed to allocate MSI\n");
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < num_db; i++) {
> +		epf->db_msg[i].virq = msi_get_virq(dev, i);
> +		epf->db_msg[i].name = kasprintf(GFP_KERNEL, "pci-epc-doorbell%d", i);
> +		ret = request_irq(epf->db_msg[i].virq, pci_epf_doorbell_handler, 0,
> +				  epf->db_msg[i].name, epf);
> +		if (ret) {
> +			dev_err(dev, "Failed to request doorbell\n");
> +			pci_epc_free_doorbell(epc, epf);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +};
> +
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> +{
> +	struct pci_epc *epc = epf->epc;
> +	void *msg;
> +	int ret;
> +
> +	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_epc_alloc_doorbell(epc, epf);
> +	if (ret)
> +		kfree(msg);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
> +
> +void pci_epf_free_doorbell(struct pci_epf *epf)
> +{
> +	struct pci_epc *epc = epf->epc;
> +
> +	pci_epc_free_doorbell(epc, epf);
> +
> +	kfree(epf->db_msg);
> +	epf->db_msg = NULL;
> +	epf->num_db = 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
> diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
> new file mode 100644
> index 0000000000000..f0cfecf491199
> --- /dev/null
> +++ b/include/linux/pci-ep-msi.h
> @@ -0,0 +1,15 @@
> +/* 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__
> +
> +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..50c0f877f2efb 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;
> @@ -75,6 +76,7 @@ struct pci_epf_ops {
>    * @link_up: Callback for the EPC link up event
>    * @link_down: Callback for the EPC link down event
>    * @bus_master_enable: Callback for the EPC Bus Master Enable event
> + * @doorbell: Callback for EPC receive MSI from RC side
>    */
>   struct pci_epc_event_ops {
>   	int (*epc_init)(struct pci_epf *epf);
> @@ -82,6 +84,7 @@ struct pci_epc_event_ops {
>   	int (*link_up)(struct pci_epf *epf);
>   	int (*link_down)(struct pci_epf *epf);
>   	int (*bus_master_enable)(struct pci_epf *epf);
> +	int (*doorbell)(struct pci_epf *epf, int index);
>   };
>   
>   /**
> @@ -125,6 +128,18 @@ 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;
> +	char *name;
> +};
> +
>   /**
>    * struct pci_epf - represents the PCI EPF device
>    * @dev: the PCI EPF device
> @@ -152,6 +167,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 +199,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;
>   };
>   
>   /**
>