linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
@ 2024-12-11 20:57 Frank Li
  2024-12-11 20:57 ` [PATCH v12 1/9] irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info Frank Li
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

┌────────────┐   ┌───────────────────────────────────┐   ┌────────────────┐
│            │   │                                   │   │                │
│            │   │ PCI Endpoint                      │   │ PCI Host       │
│            │   │                                   │   │                │
│            │◄──┤ 1.platform_msi_domain_alloc_irqs()│   │                │
│            │   │                                   │   │                │
│ MSI        ├──►│ 2.write_msi_msg()                 ├──►├─BAR<n>         │
│ Controller │   │   update doorbell register address│   │                │
│            │   │   for BAR                         │   │                │
│            │   │                                   │   │ 3. Write BAR<n>│
│            │◄──┼───────────────────────────────────┼───┤                │
│            │   │                                   │   │                │
│            ├──►│ 4.Irq Handle                      │   │                │
│            │   │                                   │   │                │
│            │   │                                   │   │                │
└────────────┘   └───────────────────────────────────┘   └────────────────┘

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

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

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

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

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

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

Changes in v10:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

    * Behavior remains unchanged.

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

To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Krzysztof Wilczyński <kw@linux.com>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>
To: Arnd Bergmann <arnd@arndb.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: Niklas Cassel <cassel@kernel.org>
Cc: dlemoal@kernel.org
Cc: jdmason@kudzu.us
To: Rafael J. Wysocki <rafael@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
To: Anup Patel <apatel@ventanamicro.com>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: Marc Zyngier <maz@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (9):
      irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info
      PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
      irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable()
      irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS
      PCI: endpoint: pci-ep-msi: Add check for MSI address/data pair immutability
      PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
      PCI: endpoint: pci-epf-test: Add doorbell test support
      misc: pci_endpoint_test: Add doorbell test case
      tools: PCI: Add 'B' option for test doorbell

 drivers/irqchip/irq-gic-v3-its-msi-parent.c   |   3 +-
 drivers/irqchip/irq-gic-v3-its.c              |   2 +-
 drivers/misc/pci_endpoint_test.c              |  80 ++++++++++++++
 drivers/pci/endpoint/Makefile                 |   2 +-
 drivers/pci/endpoint/functions/pci-epf-test.c | 132 ++++++++++++++++++++++
 drivers/pci/endpoint/pci-ep-msi.c             | 153 ++++++++++++++++++++++++++
 drivers/pci/endpoint/pci-epf-core.c           |  44 ++++++++
 include/linux/irqdomain.h                     |   8 ++
 include/linux/pci-ep-msi.h                    |  15 +++
 include/linux/pci-epf.h                       |  19 ++++
 include/uapi/linux/pcitest.h                  |   1 +
 tools/pci/pcitest.c                           |  16 ++-
 12 files changed, 471 insertions(+), 4 deletions(-)
---
base-commit: 102a25797c66c8f4cf7fafe0b0de95cc7126b3fa
change-id: 20241010-ep-msi-8b4cab33b1be

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v12 1/9] irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

  ┌───────────────────────────────┐    ┌───────┐
  │                               │    │       │
  │ PCI Endpoint Controller (1)   ├───►│ ITS   │
  │                               │    │       │
  └───────────────────────────────┘    └───────┘
      ▲        ▲
      │        │
  ┌───┴──┐  ┌──┴───┐
  │      │  │      │
  │Func1 │  │Func2 │
  │ (2)  │  │      │
  └──────┘  └──────┘
     ▲         ▲
     │         │
     └─────────┴─────────────── PCIe Bus

(1) is platform device, which is generally descripted by Device Tree(DT).
(2) Func1 and Func2 is created by configfs

The current platform MSI API supports only a single device. For instance,
a platform device (e.g., PCI Endpoint Controller) calls
platform_device_msi_init_and_alloc_irqs() to allocate MSI IRQs.

Child devices (e.g., function devices created by configfs) require
individual MSI domains, with the same MSI parent domain as the parent
device. These individual domains need specialized msi_prepare callbacks to
set  msi_alloc_info_t. However, the current ITS implementation overwrites
the msi_prepare callback with its_pmsi_prepare().

Modify the implementation to assign its_pmsi_prepare() only if
msi_domain_info::msi_prepare is NULL, allowing customized callbacks where
needed.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v11 to v12
- none

Change from v9 to v11
- new patch
---
 drivers/irqchip/irq-gic-v3-its-msi-parent.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 75aa0d4bd1346..33e94cfc4d506 100644
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -195,7 +195,8 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
 		 * FIXME: See the above PCI prepare comment. The domain
 		 * size is also known at domain creation time.
 		 */
-		info->ops->msi_prepare = its_pmsi_prepare;
+		if (!info->ops->msi_prepare)
+			info->ops->msi_prepare = its_pmsi_prepare;
 		break;
 	default:
 		/* Confused. How did the lib return true? */

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
  2024-12-11 20:57 ` [PATCH v12 1/9] irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-17 22:56   ` Thomas Gleixner
  2024-12-11 20:57 ` [PATCH v12 3/9] irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable() Frank Li
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Doorbell feature is implemented by mapping the EP's MSI interrupt
controller message address to a dedicated BAR in the EPC core. It is the
responsibility of the EPF driver to pass the actual message data to be
written by the host to the doorbell BAR region through its own logic.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v10 to v12
- none

Change from v9 to v10
- Create msi domain for each function device.
- Remove only function support limiation. My hardware only support one
function, so not test more than one case.
- use "msi-map" descript msi information

  msi-map = <func_no << 8  | vfunc_no, &its, start_stream_id,  size>;

Chagne from v8 to v9
- sort header file
- use pci_epc_get(dev_name(msi_desc_to_dev(desc)));
- check epf number at pci_epf_alloc_doorbell
- Add comments for miss msi-parent case

change from v5 to v8
-none

Change from v4 to v5
- Remove request_irq() in pci_epc_alloc_doorbell() and leave to EP function
driver, so ep function driver can register differece call back function for
difference doorbell events and set irq affinity to differece CPU core.
- Improve error message when MSI allocate failure.

Change from v3 to v4
- msi change to use msi_get_virq() avoid use msi_for_each_desc().
- add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
- move mutex lock to epc function
- initialize variable at declear place.
- passdown epf to epc*() function to simplify code.
---
 drivers/pci/endpoint/Makefile     |   2 +-
 drivers/pci/endpoint/pci-ep-msi.c | 148 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci-ep-msi.h        |  15 ++++
 include/linux/pci-epf.h           |  16 +++++
 4 files changed, 180 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..b0a91fde202f3
--- /dev/null
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCI Endpoint *Controller* (EPC) MSI library
+ *
+ * Copyright (C) 2024 NXP
+ * Author: Frank Li <Frank.Li@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_irq.h>
+#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
+#include <linux/pci-ep-cfs.h>
+#include <linux/pci-ep-msi.h>
+#include <linux/slab.h>
+
+static void pci_epf_write_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+	struct msi_desc *desc = irq_data_get_msi_desc(d);
+	struct pci_epf *epf = to_pci_epf(desc->dev);
+
+	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
+		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));
+}
+
+static void pci_epf_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
+{
+	arg->desc = desc;
+	arg->hwirq = desc->msi_index;
+}
+
+static int pci_epf_msi_prepare(struct irq_domain *domain, struct device *dev,
+			       int nvec, msi_alloc_info_t *arg)
+{
+	struct pci_epf *epf = to_pci_epf(dev);
+	struct msi_domain_info *msi_info;
+	struct pci_epc *epc = epf->epc;
+
+	memset(arg, 0, sizeof(*arg));
+	arg->scratchpad[0].ul = of_msi_map_id(epc->dev.parent, NULL,
+					      (epf->func_no << 8) | epf->vfunc_no);
+
+	/*
+	 * @domain->msi_domain_info->hwsize contains the size of the device
+	 * domain, but vector allocation happens one by one.
+	 */
+	msi_info = msi_get_domain_info(domain);
+	if (msi_info->hwsize > nvec)
+		nvec = msi_info->hwsize;
+
+	/* Allocate at least 32 MSIs, and always as a power of 2 */
+	nvec = max_t(int, 32, roundup_pow_of_two(nvec));
+
+	msi_info = msi_get_domain_info(domain->parent);
+	return msi_info->ops->msi_prepare(domain->parent, dev, nvec, arg);
+}
+
+static const struct msi_domain_template pci_epf_msi_template = {
+	.chip = {
+		.name			= "EP-MSI",
+		.irq_mask		= irq_chip_mask_parent,
+		.irq_unmask		= irq_chip_unmask_parent,
+		.irq_write_msi_msg	= pci_epf_write_msi_msg,
+		/* The rest is filled in by the MSI parent */
+	},
+
+	.ops = {
+		.msi_prepare		= pci_epf_msi_prepare,
+		.set_desc		= pci_epf_msi_set_desc,
+	},
+
+	.info = {
+		.bus_token		= DOMAIN_BUS_DEVICE_MSI,
+	},
+};
+
+static int pci_epf_device_msi_init_and_alloc_irqs(struct device *dev, unsigned int nvec)
+{
+	struct irq_domain *domain = dev->msi.domain;
+
+	if (!domain)
+		return -EINVAL;
+
+	if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN,
+					  &pci_epf_msi_template, nvec, NULL, NULL))
+		return -ENODEV;
+
+	return msi_domain_alloc_irqs_range(dev, MSI_DEFAULT_DOMAIN, 0, nvec - 1);
+}
+
+int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
+{
+	struct pci_epc *epc = epf->epc;
+	struct device *dev = &epf->dev;
+	struct irq_domain *dom;
+	void *msg;
+	u32 rid;
+	int ret;
+	int i;
+
+	rid = (epf->func_no << 8) | epf->vfunc_no;
+	dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_MSI);
+	if (!dom) {
+		dev_err(dev, "Can't find msi domain\n");
+		return -EINVAL;
+	}
+
+	dev_set_msi_domain(dev, dom);
+
+	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	epf->num_db = num_db;
+	epf->db_msg = msg;
+
+	ret = pci_epf_device_msi_init_and_alloc_irqs(dev, num_db);
+	if (ret) {
+		/*
+		 * The pcie_ep DT node has to specify 'msi-parent' for EP
+		 * doorbell support to work. Right now only GIC ITS is
+		 * supported. If you have GIC ITS and reached this print,
+		 * perhaps you are missing 'msi-map' in DT.
+		 */
+		dev_err(dev, "Failed to allocate MSI\n");
+		kfree(msg);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_db; i++)
+		epf->db_msg[i].virq = msi_get_virq(dev, i);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
+
+void pci_epf_free_doorbell(struct pci_epf *epf)
+{
+	msi_domain_free_irqs_all(&epf->dev, MSI_DEFAULT_DOMAIN);
+	msi_remove_device_irq_domain(&epf->dev, MSI_DEFAULT_DOMAIN);
+
+	kfree(epf->db_msg);
+	epf->db_msg = NULL;
+	epf->num_db = 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
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..5374e6515ffa0 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -12,6 +12,7 @@
 #include <linux/configfs.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/msi.h>
 #include <linux/pci.h>
 
 struct pci_epf;
@@ -125,6 +126,17 @@ struct pci_epf_bar {
 	int		flags;
 };
 
+/**
+ * struct pci_epf_doorbell_msg - represents doorbell message
+ * @msi_msg: MSI message
+ * @virq: irq number of this doorbell MSI message
+ * @name: irq name for doorbell interrupt
+ */
+struct pci_epf_doorbell_msg {
+	struct msi_msg msg;
+	int virq;
+};
+
 /**
  * struct pci_epf - represents the PCI EPF device
  * @dev: the PCI EPF device
@@ -152,6 +164,8 @@ struct pci_epf_bar {
  * @vfunction_num_map: bitmap to manage virtual function number
  * @pci_vepf: list of virtual endpoint functions associated with this function
  * @event_ops: Callbacks for capturing the EPC events
+ * @db_msg: data for MSI from RC side
+ * @num_db: number of doorbells
  */
 struct pci_epf {
 	struct device		dev;
@@ -182,6 +196,8 @@ struct pci_epf {
 	unsigned long		vfunction_num_map;
 	struct list_head	pci_vepf;
 	const struct pci_epc_event_ops *event_ops;
+	struct pci_epf_doorbell_msg *db_msg;
+	u16 num_db;
 };
 
 /**

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 3/9] irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable()
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
  2024-12-11 20:57 ` [PATCH v12 1/9] irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info Frank Li
  2024-12-11 20:57 ` [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 4/9] irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS Frank Li
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Add the flag IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and the API function
irq_domain_is_msi_immutable() to check if the MSI controller retains an
immutable address/data pair during irq_set_affinity().

Ensure compatibility with MSI users like PCIe Endpoint Doorbell, which
require the address/data pair to remain unchanged after setup. Use this
function to verify if the MSI controller is immutable.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v11 to v12
- change to IRQ_DOMAIN_FLAG_MSI_IMMUTABLE to minimized the code change.

Thomas Gleixner:
	I move IRQ_DOMAIN_FLAG_MSI_IMMUTABLE to irqdomain.h to simplify
helper function's implement. But I am not sure if it is reasonable.

	If you have any concern, let me know.
---
 include/linux/irqdomain.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e432b6a12a32f..e63652604b621 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -231,6 +231,9 @@ enum {
 	/* Irq domain must destroy generic chips when removed */
 	IRQ_DOMAIN_FLAG_DESTROY_GC	= (1 << 10),
 
+	/* Address and data pair is mutable when irq_set_affinity() */
+	IRQ_DOMAIN_FLAG_MSI_IMMUTABLE	= (1 << 11),
+
 	/*
 	 * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved
 	 * for implementation specific purposes and ignored by the
@@ -692,6 +695,11 @@ static inline bool irq_domain_is_msi_device(struct irq_domain *domain)
 	return domain->flags & IRQ_DOMAIN_FLAG_MSI_DEVICE;
 }
 
+static inline bool irq_domain_is_msi_immutable(struct irq_domain *domain)
+{
+	return domain->flags & IRQ_DOMAIN_FLAG_MSI_IMMUTABLE;
+}
+
 #else	/* CONFIG_IRQ_DOMAIN_HIERARCHY */
 static inline int irq_domain_alloc_irqs(struct irq_domain *domain,
 			unsigned int nr_irqs, int node, void *arg)

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 4/9] irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (2 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 3/9] irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable() Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 5/9] PCI: endpoint: pci-ep-msi: Add check for MSI address/data pair immutability Frank Li
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Set the IRQ_DOMAIN_FLAG_MSI_IMMUTABLE flag for ITS, as it does not change
the address/data pair after setup.

Ensure compatibility with MSI users, such as PCIe Endpoint Doorbell, which
require the address/data pair to remain unchanged. Enable PCIe endpoints to
use ITS for triggering doorbells from the PCIe Root Complex (RC) side.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v11 to v12
- new patch
---
 drivers/irqchip/irq-gic-v3-its.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 92244cfa04647..edaae13bbd3cc 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5126,7 +5126,7 @@ static int its_init_domain(struct its_node *its)
 	irq_domain_update_bus_token(inner_domain, DOMAIN_BUS_NEXUS);
 
 	inner_domain->msi_parent_ops = &gic_v3_its_msi_parent_ops;
-	inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+	inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT | IRQ_DOMAIN_FLAG_MSI_IMMUTABLE;
 
 	return 0;
 }

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 5/9] PCI: endpoint: pci-ep-msi: Add check for MSI address/data pair immutability
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (3 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 4/9] irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Check the immutability of the MSI address/data pair by calling
irq_domain_is_msi_immutable() when allocating a doorbell.

Prevent the use of MSI controllers that change the address/data pair during
irq_set_affinity(), as the current PCI endpoint implementation does not
support such behavior.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v11 to v12
- use helper function irq_domain_is_msi_immutable().
- remove msi parent check.
- split msi header, its and endpoint to 3 patches.
- rework commit message.

Change from v9 to v11
- new patch
---
 drivers/pci/endpoint/pci-ep-msi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
index b0a91fde202f3..eda027b734b73 100644
--- a/drivers/pci/endpoint/pci-ep-msi.c
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -107,6 +107,11 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
 		return -EINVAL;
 	}
 
+	if (!irq_domain_is_msi_immutable(dom)) {
+		dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
+		return -EINVAL;
+	}
+
 	dev_set_msi_domain(dev, dom);
 
 	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (4 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 5/9] PCI: endpoint: pci-ep-msi: Add check for MSI address/data pair immutability Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Introduce the helper function pci_epf_align_inbound_addr() to adjust
addresses according to PCI BAR alignment requirements, converting addresses
into base and offset values.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change form v9 to v12
- none

change from v8 to v9
- pci_epf_align_inbound_addr(), base and off must be not NULL
- rm pci_epf_align_inbound_addr_lo_hi()

change from v7 to v8
- change name to pci_epf_align_inbound_addr()
- update comment said only need for memory, which not allocated by
pci_epf_alloc_space().

change from v6 to v7
- new patch
---
 drivers/pci/endpoint/pci-epf-core.c | 44 +++++++++++++++++++++++++++++++++++++
 include/linux/pci-epf.h             |  3 +++
 2 files changed, 47 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 8fa2797d4169a..d7a80f9c1e661 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -464,6 +464,50 @@ struct pci_epf *pci_epf_create(const char *name)
 }
 EXPORT_SYMBOL_GPL(pci_epf_create);
 
+/**
+ * pci_epf_align_inbound_addr() - Get base address and offset that match BAR's
+ *			  alignment requirement
+ * @epf: the EPF device
+ * @addr: the address of the memory
+ * @bar: the BAR number corresponding to map addr
+ * @base: return base address, which match BAR's alignment requirement.
+ * @off: return offset.
+ *
+ * Helper function to convert input 'addr' to base and offset, which match
+ * BAR's alignment requirement.
+ *
+ * The pci_epf_alloc_space() function already accounts for alignment. This is
+ * primarily intended for use with other memory regions not allocated by
+ * pci_epf_alloc_space(), such as peripheral register spaces or the trigger
+ * address for a platform MSI controller.
+ */
+int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
+			       u64 addr, u64 *base, size_t *off)
+{
+	const struct pci_epc_features *epc_features;
+	u64 align;
+
+	if (!base || !off)
+		return -EINVAL;
+
+	epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
+	if (!epc_features) {
+		dev_err(&epf->dev, "epc_features not implemented\n");
+		return -EOPNOTSUPP;
+	}
+
+	align = epc_features->align;
+	align = align ? align : 128;
+	if (epc_features->bar[bar].type == BAR_FIXED)
+		align = max(epc_features->bar[bar].fixed_size, align);
+
+	*base = round_down(addr, align);
+	*off = addr & (align - 1);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_align_inbound_addr);
+
 static void pci_epf_dev_release(struct device *dev)
 {
 	struct pci_epf *epf = to_pci_epf(dev);
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 5374e6515ffa0..2847d195433bf 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -238,6 +238,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 			  enum pci_epc_interface_type type);
 void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
+
+int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
+			       u64 addr, u64 *base, size_t *off);
 int pci_epf_bind(struct pci_epf *epf);
 void pci_epf_unbind(struct pci_epf *epf);
 int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (5 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
  2024-12-11 20:57 ` [PATCH v12 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Add three registers: doorbell_bar, doorbell_addr, and doorbell_data. Use
pci_epf_alloc_doorbell() to allocate a doorbell address space.

Enable the Root Complex (RC) side driver to trigger pci-epc-test's doorbell
callback handler by writing doorbell_data to the mapped doorbell_bar's
address space.

Set STATUS_DOORBELL_SUCCESS in the doorbell callback to indicate
completion.

Avoid breaking compatibility between host and endpoint, add new command
COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL. Host side need send
COMMAND_ENABLE_DOORBELL to map one bar's inbound address to MSI space.
the command COMMAND_DISABLE_DOORBELL to recovery original inbound address
mapping.

	 	Host side new driver	Host side old driver

EP: new driver      S				F
EP: old driver      F				F

S: If EP side support MSI, 'pcitest -B' return success.
   If EP side doesn't support MSI, the same to 'F'.

F: 'pcitest -B' return failure, other case as usual.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v9 to v12
- none

Change from v8 to v9
- move pci_epf_alloc_doorbell() into pci_epf_{enable/disable}_doorbell().
- remove doorbell_done in commit message.
- rename pci_epf_{enable/disable}_doorbell() to
pci_epf_test_{enable/disable}_doorbell() to align corrent code style.

Change from v7 to v8
- rename to pci_epf_align_inbound_addr_lo_hi()

Change from v6 to v7
- use help function pci_epf_align_addr_lo_hi()

Change from v5 to v6
- rename doorbell_addr to doorbell_offset

Chagne from v4 to v5
- Add doorbell free at unbind function.
- Move msi irq handler to here to more complex user case, such as differece
doorbell can use difference handler function.
- Add Niklas's code to handle fixed bar's case. If need add your signed-off
tag or co-developer tag, please let me know.

change from v3 to v4
- remove revid requirement
- Add command COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
- call pci_epc_set_bar() to map inbound address to MSI space only at
COMMAND_ENABLE_DOORBELL.
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 132 ++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index ef6677f34116e..a0a0e86a081cb 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -11,12 +11,14 @@
 #include <linux/dmaengine.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/msi.h>
 #include <linux/slab.h>
 #include <linux/pci_ids.h>
 #include <linux/random.h>
 
 #include <linux/pci-epc.h>
 #include <linux/pci-epf.h>
+#include <linux/pci-ep-msi.h>
 #include <linux/pci_regs.h>
 
 #define IRQ_TYPE_INTX			0
@@ -29,6 +31,8 @@
 #define COMMAND_READ			BIT(3)
 #define COMMAND_WRITE			BIT(4)
 #define COMMAND_COPY			BIT(5)
+#define COMMAND_ENABLE_DOORBELL		BIT(6)
+#define COMMAND_DISABLE_DOORBELL	BIT(7)
 
 #define STATUS_READ_SUCCESS		BIT(0)
 #define STATUS_READ_FAIL		BIT(1)
@@ -39,6 +43,11 @@
 #define STATUS_IRQ_RAISED		BIT(6)
 #define STATUS_SRC_ADDR_INVALID		BIT(7)
 #define STATUS_DST_ADDR_INVALID		BIT(8)
+#define STATUS_DOORBELL_SUCCESS		BIT(9)
+#define STATUS_DOORBELL_ENABLE_SUCCESS	BIT(10)
+#define STATUS_DOORBELL_ENABLE_FAIL	BIT(11)
+#define STATUS_DOORBELL_DISABLE_SUCCESS BIT(12)
+#define STATUS_DOORBELL_DISABLE_FAIL	BIT(13)
 
 #define FLAG_USE_DMA			BIT(0)
 
@@ -74,6 +83,9 @@ struct pci_epf_test_reg {
 	u32	irq_type;
 	u32	irq_number;
 	u32	flags;
+	u32	doorbell_bar;
+	u32	doorbell_offset;
+	u32	doorbell_data;
 } __packed;
 
 static struct pci_epf_header test_header = {
@@ -642,6 +654,117 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 	}
 }
 
+static irqreturn_t pci_epf_test_doorbell_handler(int irq, void *data)
+{
+	struct pci_epf_test *epf_test = data;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+
+	reg->status |= STATUS_DOORBELL_SUCCESS;
+	pci_epf_test_raise_irq(epf_test, reg);
+
+	return IRQ_HANDLED;
+}
+
+static void pci_epf_test_doorbell_cleanup(struct pci_epf_test *epf_test)
+{
+	struct pci_epf_test_reg *reg = epf_test->reg[epf_test->test_reg_bar];
+	struct pci_epf *epf = epf_test->epf;
+
+	if (reg->doorbell_bar > 0) {
+		free_irq(epf->db_msg[0].virq, epf_test);
+		reg->doorbell_bar = NO_BAR;
+	}
+
+	if (epf->db_msg)
+		pci_epf_free_doorbell(epf);
+}
+
+static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
+					 struct pci_epf_test_reg *reg)
+{
+	struct pci_epf *epf = epf_test->epf;
+	struct pci_epf_bar db_bar = {};
+	struct pci_epc *epc = epf->epc;
+	struct msi_msg *msg;
+	enum pci_barno bar;
+	size_t offset;
+	int ret;
+
+	ret = pci_epf_alloc_doorbell(epf, 1);
+	if (ret) {
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		return;
+	}
+
+	msg = &epf->db_msg[0].msg;
+	bar = pci_epc_get_next_free_bar(epf_test->epc_features, epf_test->test_reg_bar + 1);
+	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		return;
+	}
+
+	ret = request_irq(epf->db_msg[0].virq, pci_epf_test_doorbell_handler, 0,
+			  "pci-test-doorbell", epf_test);
+	if (ret) {
+		dev_err(&epf->dev,
+			"Failed to request irq %d, doorbell feature is not supported\n",
+			epf->db_msg[0].virq);
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+		return;
+	}
+
+	reg->doorbell_data = msg->data;
+	reg->doorbell_bar = bar;
+
+	msg = &epf->db_msg[0].msg;
+	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
+					 &db_bar.phys_addr, &offset);
+
+	if (ret) {
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+		return;
+	}
+
+	reg->doorbell_offset = offset;
+
+	db_bar.barno = bar;
+	db_bar.size = epf->bar[bar].size;
+	db_bar.flags = epf->bar[bar].flags;
+
+	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &db_bar);
+	if (ret) {
+		reg->status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+	} else {
+		reg->status |= STATUS_DOORBELL_ENABLE_SUCCESS;
+	}
+}
+
+static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test,
+					  struct pci_epf_test_reg *reg)
+{
+	enum pci_barno bar = reg->doorbell_bar;
+	struct pci_epf *epf = epf_test->epf;
+	struct pci_epc *epc = epf->epc;
+	int ret;
+
+	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
+		reg->status |= STATUS_DOORBELL_DISABLE_FAIL;
+		return;
+	}
+
+	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
+	if (ret)
+		reg->status |= STATUS_DOORBELL_DISABLE_FAIL;
+	else
+		reg->status |= STATUS_DOORBELL_DISABLE_SUCCESS;
+
+	pci_epf_test_doorbell_cleanup(epf_test);
+}
+
 static void pci_epf_test_cmd_handler(struct work_struct *work)
 {
 	u32 command;
@@ -688,6 +811,14 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 		pci_epf_test_copy(epf_test, reg);
 		pci_epf_test_raise_irq(epf_test, reg);
 		break;
+	case COMMAND_ENABLE_DOORBELL:
+		pci_epf_test_enable_doorbell(epf_test, reg);
+		pci_epf_test_raise_irq(epf_test, reg);
+		break;
+	case COMMAND_DISABLE_DOORBELL:
+		pci_epf_test_disable_doorbell(epf_test, reg);
+		pci_epf_test_raise_irq(epf_test, reg);
+		break;
 	default:
 		dev_err(dev, "Invalid command 0x%x\n", command);
 		break;
@@ -934,6 +1065,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 		pci_epf_test_clean_dma_chan(epf_test);
 		pci_epf_test_clear_bar(epf);
 	}
+	pci_epf_test_doorbell_cleanup(epf_test);
 	pci_epf_test_free_space(epf);
 }
 

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 8/9] misc: pci_endpoint_test: Add doorbell test case
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (6 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
@ 2024-12-11 20:57 ` Frank Li
  2024-12-11 20:57 ` [PATCH v12 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Add three registers: PCIE_ENDPOINT_TEST_DB_BAR, PCIE_ENDPOINT_TEST_DB_ADDR,
and PCIE_ENDPOINT_TEST_DB_DATA.

Trigger the doorbell by writing data from PCI_ENDPOINT_TEST_DB_DATA to the
address provided by PCI_ENDPOINT_TEST_DB_OFFSET and wait for endpoint
feedback.

Add two command to COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL
to enable EP side's doorbell support and avoid compatible problem, which
host side driver miss-match with endpoint side function driver. See below
table:

		Host side new driver	Host side old driver
EP: new driver		S			F
EP: old driver		F			F

S: If EP side support MSI, 'pcitest -B' return success.
   If EP side doesn't support MSI, the same to 'F'.

F: 'pcitest -B' return failure, other case as usual.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v9 to v12
- none

Change from v8 to v9
- change PCITEST_DOORBELL to 0xa

Change form v6 to v8
- none

Change from v5 to v6
- %s/PCI_ENDPOINT_TEST_DB_ADDR/PCI_ENDPOINT_TEST_DB_OFFSET/g

Change from v4 to v5
- remove unused varible
- add irq_type at pci_endpoint_test_doorbell();

change from v3 to v4
- Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
- Remove new DID requirement.
---
 drivers/misc/pci_endpoint_test.c | 80 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/pcitest.h     |  1 +
 2 files changed, 81 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 3aaaf47fa4ee2..b3f36b6ba8ba2 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -42,6 +42,8 @@
 #define COMMAND_READ				BIT(3)
 #define COMMAND_WRITE				BIT(4)
 #define COMMAND_COPY				BIT(5)
+#define COMMAND_ENABLE_DOORBELL			BIT(6)
+#define COMMAND_DISABLE_DOORBELL		BIT(7)
 
 #define PCI_ENDPOINT_TEST_STATUS		0x8
 #define STATUS_READ_SUCCESS			BIT(0)
@@ -53,6 +55,11 @@
 #define STATUS_IRQ_RAISED			BIT(6)
 #define STATUS_SRC_ADDR_INVALID			BIT(7)
 #define STATUS_DST_ADDR_INVALID			BIT(8)
+#define STATUS_DOORBELL_SUCCESS			BIT(9)
+#define STATUS_DOORBELL_ENABLE_SUCCESS		BIT(10)
+#define STATUS_DOORBELL_ENABLE_FAIL		BIT(11)
+#define STATUS_DOORBELL_DISABLE_SUCCESS		BIT(12)
+#define STATUS_DOORBELL_DISABLE_FAIL		BIT(13)
 
 #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0x0c
 #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
@@ -67,6 +74,10 @@
 #define PCI_ENDPOINT_TEST_IRQ_NUMBER		0x28
 
 #define PCI_ENDPOINT_TEST_FLAGS			0x2c
+#define PCI_ENDPOINT_TEST_DB_BAR		0x30
+#define PCI_ENDPOINT_TEST_DB_OFFSET		0x34
+#define PCI_ENDPOINT_TEST_DB_DATA		0x38
+
 #define FLAG_USE_DMA				BIT(0)
 
 #define PCI_DEVICE_ID_TI_AM654			0xb00c
@@ -108,6 +119,7 @@ enum pci_barno {
 	BAR_3,
 	BAR_4,
 	BAR_5,
+	NO_BAR = -1,
 };
 
 struct pci_endpoint_test {
@@ -746,6 +758,71 @@ static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
 	return false;
 }
 
+static bool pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
+{
+	struct pci_dev *pdev = test->pdev;
+	struct device *dev = &pdev->dev;
+	int irq_type = test->irq_type;
+	enum pci_barno bar;
+	u32 data, status;
+	u32 addr;
+
+	if (irq_type < IRQ_TYPE_INTX || irq_type > IRQ_TYPE_MSIX) {
+		dev_err(dev, "Invalid IRQ type option\n");
+		return false;
+	}
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
+				 COMMAND_ENABLE_DOORBELL);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+	if (status & STATUS_DOORBELL_ENABLE_FAIL) {
+		dev_err(dev, "Failed to enable doorbell\n");
+		return false;
+	}
+
+	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
+	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET);
+	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0);
+
+	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
+
+	writel(data, test->bar[bar] + addr);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+
+	if (!(status & STATUS_DOORBELL_SUCCESS))
+		dev_err(dev, "Endpoint have not received Doorbell\n");
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
+				 COMMAND_DISABLE_DOORBELL);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status |= pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+
+	if (status & STATUS_DOORBELL_DISABLE_FAIL) {
+		dev_err(dev, "Failed to disable doorbell\n");
+		return false;
+	}
+
+	if (!(status & STATUS_DOORBELL_SUCCESS))
+		return false;
+
+	return true;
+}
+
 static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 				    unsigned long arg)
 {
@@ -793,6 +870,9 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 	case PCITEST_CLEAR_IRQ:
 		ret = pci_endpoint_test_clear_irq(test);
 		break;
+	case PCITEST_DOORBELL:
+		ret = pci_endpoint_test_doorbell(test);
+		break;
 	}
 
 ret:
diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index 94b46b043b536..b82e7f2ed937d 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -20,6 +20,7 @@
 #define PCITEST_MSIX		_IOW('P', 0x7, int)
 #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
 #define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
+#define PCITEST_DOORBELL	_IO('P', 0xa)
 #define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
 
 #define PCITEST_FLAGS_USE_DMA	0x00000001

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v12 9/9] tools: PCI: Add 'B' option for test doorbell
  2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
                   ` (7 preceding siblings ...)
  2024-12-11 20:57 ` [PATCH v12 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
@ 2024-12-11 20:57 ` Frank Li
  8 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-11 20:57 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Thomas Gleixner,
	Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

Add doorbell test support.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v12
- none
---
 tools/pci/pcitest.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 7b530d838d408..fcff0224a3381 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -34,6 +34,7 @@ struct pci_test {
 	bool		copy;
 	unsigned long	size;
 	bool		use_dma;
+	bool		doorbell;
 };
 
 static int run_test(struct pci_test *test)
@@ -147,6 +148,15 @@ static int run_test(struct pci_test *test)
 			fprintf(stdout, "%s\n", result[ret]);
 	}
 
+	if (test->doorbell) {
+		ret = ioctl(fd, PCITEST_DOORBELL, 0);
+		fprintf(stdout, "Ringing doorbell on the EP\t\t");
+		if (ret < 0)
+			fprintf(stdout, "TEST FAILED\n");
+		else
+			fprintf(stdout, "%s\n", result[ret]);
+	}
+
 	fflush(stdout);
 	close(fd);
 	return (ret < 0) ? ret : 1 - ret; /* return 0 if test succeeded */
@@ -172,7 +182,7 @@ int main(int argc, char **argv)
 	/* set default endpoint device */
 	test->device = "/dev/pci-endpoint-test.0";
 
-	while ((c = getopt(argc, argv, "D:b:m:x:i:deIlhrwcs:")) != EOF)
+	while ((c = getopt(argc, argv, "D:b:m:x:i:BdeIlhrwcs:")) != EOF)
 	switch (c) {
 	case 'D':
 		test->device = optarg;
@@ -222,6 +232,9 @@ int main(int argc, char **argv)
 	case 'd':
 		test->use_dma = true;
 		continue;
+	case 'B':
+		test->doorbell = true;
+		continue;
 	case 'h':
 	default:
 usage:
@@ -241,6 +254,7 @@ int main(int argc, char **argv)
 			"\t-w			Write buffer test\n"
 			"\t-c			Copy buffer test\n"
 			"\t-s <size>		Size of buffer {default: 100KB}\n"
+			"\t-B			Doorbell test\n"
 			"\t-h			Print this help message\n",
 			argv[0]);
 		return -EINVAL;

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
  2024-12-11 20:57 ` [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
@ 2024-12-17 22:56   ` Thomas Gleixner
  2024-12-18 17:44     ` Frank Li
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2024-12-17 22:56 UTC (permalink / raw)
  To: Frank Li, Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Anup Patel, Marc Zyngier
  Cc: linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel, Frank Li

On Wed, Dec 11 2024 at 15:57, Frank Li wrote:
> +static int pci_epf_msi_prepare(struct irq_domain *domain, struct device *dev,
> +			       int nvec, msi_alloc_info_t *arg)
> +{
> +	struct pci_epf *epf = to_pci_epf(dev);
> +	struct msi_domain_info *msi_info;
> +	struct pci_epc *epc = epf->epc;
> +
> +	memset(arg, 0, sizeof(*arg));
> +	arg->scratchpad[0].ul = of_msi_map_id(epc->dev.parent, NULL,
> +					      (epf->func_no << 8) | epf->vfunc_no);
> +
> +	/*
> +	 * @domain->msi_domain_info->hwsize contains the size of the device
> +	 * domain, but vector allocation happens one by one.
> +	 */
> +	msi_info = msi_get_domain_info(domain);
> +	if (msi_info->hwsize > nvec)
> +		nvec = msi_info->hwsize;
> +
> +	/* Allocate at least 32 MSIs, and always as a power of 2 */
> +	nvec = max_t(int, 32, roundup_pow_of_two(nvec));
> +
> +	msi_info = msi_get_domain_info(domain->parent);
> +	return msi_info->ops->msi_prepare(domain->parent, dev, nvec, arg);

While I was trying to make sense of the change log of patch [1/9] I
looked at this function to understand why this needs an override.

This is a copy of its_msi_prepare() except for the scratchpad[0].ul
part. But that's a GIC-V3 implementation specific detail, which has
absolutely no business in code which claims to be a generic library for
PCI endpoints.

Worse you created a GIC-V3 only PCI endpoint library under the
assumption that the underlying ITS/MSI implementation is immutable. Of
course there is no safety net either to validate that the underlying
parent domain is actually GIC-V3-ITS. That's wrong in every aspect.

So let's take a step back and analyze what is actually required to make
this a proper generic library.

The endpoint function device needs its own device ID which is required
to set up a device specific translation in the interrupt remapping unit.

Now you decided that this is bound to a DT mapping, which is odd to
begin with. What's DT specific about this? The cirumstance that your
hardware is DT based and the endpoint controller ID map needs to be
retrieved from there? How is this generic in any way? How is this
supposed to work with ACPI enumerated hardware? Not to ask the question
how this should work with non GIC-V3-ITS based hardware.

That's all but generic, it's an ad hoc hack to support your particular
setup implemented by layering violations.

In fact the mapping ID is composed by the parent mapping ID and the
function numbers, right?

The general PCIe convention here is:

    domain:bus:slot.func

That's well defined and if you look at real devices then lspci shows:

0000:3d:00.1 Ethernet controller: Ethernet Connection for 10GBASE-T
0000:3d:06.0 Ethernet controller: Ethernet Virtual Function
0000:...
0000:3d:06.7 Ethernet controller: Ethernet Virtual Function
0000:3d:07.0 Ethernet controller: Ethernet Virtual Function
0000:...
0000:3d:07.7 Ethernet controller: Ethernet Virtual Function

In PCI address representation:

   domain:bus:slot:function

which is usually condensed into a single word based on the range limits
of function, device and bus:

   function:    bit 0-2         (max. 8)
   device:      bit 3-7         (max. 32)
   bus:         bit 8-15        (max. 256)
   domain:      bit 16-31       (mostly theoretical)

Endpoint devices should follow exactly the same scheme, no?

Now looking at your ID retrieval:

> +	arg->scratchpad[0].ul = of_msi_map_id(epc->dev.parent, NULL,
> +					      (epf->func_no << 8) | epf->vfunc_no);

I really have to ask why this is making up its own representation
instead of simply using the standard PCI B/D/F conventions?

Whatever the reason is, fact is that the actual interrupt domain support
needs to be done differently. There is no way that the endpoint library
makes assumption about the underlying interrupt domain and copies a
function just because. This has to be completely agnostic, no if, no
but.

So the consequence is that the underlying MSI parent domains needs to
know about the endpoint requirements, which is how all MSI variants are
modeled, i.e. with a MSI domain bus.

That also solves the problem of immutable MSI messages without any
further magic. Interrupt domains, which do not provide them, won't
provide the endpoint MSI domain bus and therefore the lookup of the
parent MSI domain for the endpoint fails.

The uncompilable mockup below should give you a hint.

Thanks,

        tglx
---
 drivers/irqchip/irq-gic-v3-its-msi-parent.c |   50 ++++++++++++++++++++--------
 drivers/irqchip/irq-msi-lib.c               |    5 ++
 drivers/irqchip/irq-msi-lib.h               |   12 +++++-
 include/linux/irqdomain_defs.h              |    2 +
 4 files changed, 51 insertions(+), 18 deletions(-)

--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -126,20 +126,9 @@ int __weak iort_pmsi_get_dev_id(struct d
 	return -1;
 }
 
-static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
-			    int nvec, msi_alloc_info_t *info)
+static int __its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+			      int nvec, msi_alloc_info_t *info, u32 dev_id)
 {
-	struct msi_domain_info *msi_info;
-	u32 dev_id;
-	int ret;
-
-	if (dev->of_node)
-		ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
-	else
-		ret = iort_pmsi_get_dev_id(dev, &dev_id);
-	if (ret)
-		return ret;
-
 	/* ITS specific DeviceID, as the core ITS ignores dev. */
 	info->scratchpad[0].ul = dev_id;
 
@@ -159,6 +148,36 @@ static int its_pmsi_prepare(struct irq_d
 					  dev, nvec, info);
 }
 
+static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
+				  int nvec, msi_alloc_info_t *info)
+{
+	u32 dev_id = dev_get_pci_ep_id(dev);
+	struct msi_domain_info *msi_info;
+	int ret = -ENOTSUPP;
+
+	if (dev->of_node)
+		ret = do_magic_ep_id_map();
+	if (ret)
+		return ret;
+	return __its_pmsi_prepare(domain, dev, nvec, info, dev_id);
+}
+
+static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+			    int nvec, msi_alloc_info_t *info)
+{
+	struct msi_domain_info *msi_info;
+	u32 dev_id;
+	int ret;
+
+	if (dev->of_node)
+		ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
+	else
+		ret = iort_pmsi_get_dev_id(dev, &dev_id);
+	if (ret)
+		return ret;
+	return __its_pmsi_prepare(domain, dev, nvec, info, dev_id);
+}
+
 static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
 				  struct irq_domain *real_parent, struct msi_domain_info *info)
 {
@@ -183,6 +202,9 @@ static bool its_init_dev_msi_info(struct
 		 */
 		info->ops->msi_prepare = its_pci_msi_prepare;
 		break;
+	case DOMAIN_BUS_PCI_DEVICE_EP_MSI:
+		info->ops->msi_prepare = its_pci_ep_msi_prepare;
+		break;
 	case DOMAIN_BUS_DEVICE_MSI:
 	case DOMAIN_BUS_WIRED_TO_MSI:
 		/*
@@ -204,7 +226,7 @@ const struct msi_parent_ops gic_v3_its_m
 	.supported_flags	= ITS_MSI_FLAGS_SUPPORTED,
 	.required_flags		= ITS_MSI_FLAGS_REQUIRED,
 	.bus_select_token	= DOMAIN_BUS_NEXUS,
-	.bus_select_mask	= MATCH_PCI_MSI | MATCH_PLATFORM_MSI,
+	.bus_select_mask	= MATCH_PCI_MSI | MATCH_PLATFORM_PCI_EP_MSI | MATCH_PLATFORM_MSI,
 	.prefix			= "ITS-",
 	.init_dev_msi_info	= its_init_dev_msi_info,
 };
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -55,8 +55,11 @@ bool msi_lib_init_dev_msi_info(struct de
 	case DOMAIN_BUS_PCI_DEVICE_MSIX:
 		if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_MSI)))
 			return false;
-
 		break;
+	case DOMAIN_BUS_DEVICE_PCI_EP_MSI:
+		if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_ENDPOINT)))
+			return false;
+		fallthrough;
 	case DOMAIN_BUS_DEVICE_MSI:
 		/*
 		 * Per device MSI should never have any MSI feature bits
--- a/drivers/irqchip/irq-msi-lib.h
+++ b/drivers/irqchip/irq-msi-lib.h
@@ -10,12 +10,18 @@
 #include <linux/msi.h>
 
 #ifdef CONFIG_PCI_MSI
-#define MATCH_PCI_MSI		BIT(DOMAIN_BUS_PCI_MSI)
+#define MATCH_PCI_MSI			BIT(DOMAIN_BUS_PCI_MSI)
 #else
-#define MATCH_PCI_MSI		(0)
+#define MATCH_PCI_MSI			(0)
 #endif
 
-#define MATCH_PLATFORM_MSI	BIT(DOMAIN_BUS_PLATFORM_MSI)
+#ifdef CONFIG_PCI_ENDPOINT
+#define MATCH_PLATFORM_PCI_EP_MSI	BIT(DOMAIN_BUS_PLATFORM_PCI_EP_MSI)
+#else
+#define MATCH_PLATFORM_PCI_EP_MSI	(0)
+#endif
+
+#define MATCH_PLATFORM_MSI		BIT(DOMAIN_BUS_PLATFORM_MSI)
 
 int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
 			      enum irq_domain_bus_token bus_token);
--- a/include/linux/irqdomain_defs.h
+++ b/include/linux/irqdomain_defs.h
@@ -15,6 +15,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_GENERIC_MSI,
 	DOMAIN_BUS_PCI_MSI,
 	DOMAIN_BUS_PLATFORM_MSI,
+	DOMAIN_BUS_PLATFORM_PCI_EP_MSI,
 	DOMAIN_BUS_NEXUS,
 	DOMAIN_BUS_IPI,
 	DOMAIN_BUS_FSL_MC_MSI,
@@ -27,6 +28,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_AMDVI,
 	DOMAIN_BUS_DEVICE_MSI,
 	DOMAIN_BUS_WIRED_TO_MSI,
+	DOMAIN_BUS_DEVICE_PCI_EP_MSI,
 };
 
 #endif /* _LINUX_IRQDOMAIN_DEFS_H */


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
  2024-12-17 22:56   ` Thomas Gleixner
@ 2024-12-18 17:44     ` Frank Li
  0 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2024-12-18 17:44 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Arnd Bergmann,
	Greg Kroah-Hartman, Rafael J. Wysocki, Anup Patel, Marc Zyngier,
	linux-kernel, linux-pci, imx, Niklas Cassel, dlemoal, jdmason,
	linux-arm-kernel

On Tue, Dec 17, 2024 at 11:56:18PM +0100, Thomas Gleixner wrote:
> On Wed, Dec 11 2024 at 15:57, Frank Li wrote:
> > +static int pci_epf_msi_prepare(struct irq_domain *domain, struct device *dev,
> > +			       int nvec, msi_alloc_info_t *arg)
> > +{
> > +	struct pci_epf *epf = to_pci_epf(dev);
> > +	struct msi_domain_info *msi_info;
> > +	struct pci_epc *epc = epf->epc;
> > +
> > +	memset(arg, 0, sizeof(*arg));
> > +	arg->scratchpad[0].ul = of_msi_map_id(epc->dev.parent, NULL,
> > +					      (epf->func_no << 8) | epf->vfunc_no);
> > +
> > +	/*
> > +	 * @domain->msi_domain_info->hwsize contains the size of the device
> > +	 * domain, but vector allocation happens one by one.
> > +	 */
> > +	msi_info = msi_get_domain_info(domain);
> > +	if (msi_info->hwsize > nvec)
> > +		nvec = msi_info->hwsize;
> > +
> > +	/* Allocate at least 32 MSIs, and always as a power of 2 */
> > +	nvec = max_t(int, 32, roundup_pow_of_two(nvec));
> > +
> > +	msi_info = msi_get_domain_info(domain->parent);
> > +	return msi_info->ops->msi_prepare(domain->parent, dev, nvec, arg);
>
> While I was trying to make sense of the change log of patch [1/9] I
> looked at this function to understand why this needs an override.
>
> This is a copy of its_msi_prepare() except for the scratchpad[0].ul
> part. But that's a GIC-V3 implementation specific detail, which has
> absolutely no business in code which claims to be a generic library for
> PCI endpoints.
>
> Worse you created a GIC-V3 only PCI endpoint library under the
> assumption that the underlying ITS/MSI implementation is immutable. Of
> course there is no safety net either to validate that the underlying
> parent domain is actually GIC-V3-ITS. That's wrong in every aspect.
>
> So let's take a step back and analyze what is actually required to make
> this a proper generic library.
>
> The endpoint function device needs its own device ID which is required
> to set up a device specific translation in the interrupt remapping unit.
>
> Now you decided that this is bound to a DT mapping, which is odd to
> begin with. What's DT specific about this? The cirumstance that your
> hardware is DT based and the endpoint controller ID map needs to be
> retrieved from there? How is this generic in any way? How is this
> supposed to work with ACPI enumerated hardware? Not to ask the question
> how this should work with non GIC-V3-ITS based hardware.
>
> That's all but generic, it's an ad hoc hack to support your particular
> setup implemented by layering violations.
>
> In fact the mapping ID is composed by the parent mapping ID and the
> function numbers, right?
>
> The general PCIe convention here is:
>
>     domain:bus:slot.func
>
> That's well defined and if you look at real devices then lspci shows:
>
> 0000:3d:00.1 Ethernet controller: Ethernet Connection for 10GBASE-T
> 0000:3d:06.0 Ethernet controller: Ethernet Virtual Function
> 0000:...
> 0000:3d:06.7 Ethernet controller: Ethernet Virtual Function
> 0000:3d:07.0 Ethernet controller: Ethernet Virtual Function
> 0000:...
> 0000:3d:07.7 Ethernet controller: Ethernet Virtual Function
>
> In PCI address representation:
>
>    domain:bus:slot:function
>
> which is usually condensed into a single word based on the range limits
> of function, device and bus:
>
>    function:    bit 0-2         (max. 8)
>    device:      bit 3-7         (max. 32)
>    bus:         bit 8-15        (max. 256)
>    domain:      bit 16-31       (mostly theoretical)
>
> Endpoint devices should follow exactly the same scheme, no?

Can't reuse BDF beasue Bus because device have not any means for EP side.
such as PCI EP controller have 8 physical functions. called EP.1 EP.2 ....

EP.n may connect PCI host 1, which bus number is 1. so BDF
	0000.01.00.1: sampe EP devices

but if it connect another PCI bus 3,  so BDF is
	0000.03.00.1 sampe EP devices.

At EP side, it don't care connect to RC bus1 or bus3.

	EP.1  use MSI irq 3
	EP.2  use MSI ire 4

regardless EP connect bus 1 or bus 3, door bell always trigger EP side
irq 3 for EP.1,  irq 4 for EP.2

So only function and vfunction should be used for as request id.

I worry about some hardware (I have not met yet) direct use BDF from RC
side because it is simple hardware implementation for dual role pci
controller.

>
> Now looking at your ID retrieval:
>
> > +	arg->scratchpad[0].ul = of_msi_map_id(epc->dev.parent, NULL,
> > +					      (epf->func_no << 8) | epf->vfunc_no);
>
> I really have to ask why this is making up its own representation
> instead of simply using the standard PCI B/D/F conventions?

See above!

Addtional difference vfunc need difference doorbell msi domain.
I have not sure how virtual function to map to BDF. If it map D, we just
simple mask B. I want reserve it to avoid ABI change in future if vfunc
support since my hardware don't support EP's side vfunc yet.

Anyway, let me implement what your below suggestion then fine tune this.

>
> Whatever the reason is, fact is that the actual interrupt domain support
> needs to be done differently. There is no way that the endpoint library
> makes assumption about the underlying interrupt domain and copies a
> function just because. This has to be completely agnostic, no if, no
> but.
>
> So the consequence is that the underlying MSI parent domains needs to
> know about the endpoint requirements, which is how all MSI variants are
> modeled, i.e. with a MSI domain bus.
>
> That also solves the problem of immutable MSI messages without any
> further magic. Interrupt domains, which do not provide them, won't
> provide the endpoint MSI domain bus and therefore the lookup of the
> parent MSI domain for the endpoint fails.
>
> The uncompilable mockup below should give you a hint.

Thank for your example, let me try to implement it.

Frank

>
> Thanks,
>
>         tglx
> ---
>  drivers/irqchip/irq-gic-v3-its-msi-parent.c |   50 ++++++++++++++++++++--------
>  drivers/irqchip/irq-msi-lib.c               |    5 ++
>  drivers/irqchip/irq-msi-lib.h               |   12 +++++-
>  include/linux/irqdomain_defs.h              |    2 +
>  4 files changed, 51 insertions(+), 18 deletions(-)
>
> --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> @@ -126,20 +126,9 @@ int __weak iort_pmsi_get_dev_id(struct d
>  	return -1;
>  }
>
> -static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> -			    int nvec, msi_alloc_info_t *info)
> +static int __its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> +			      int nvec, msi_alloc_info_t *info, u32 dev_id)
>  {
> -	struct msi_domain_info *msi_info;
> -	u32 dev_id;
> -	int ret;
> -
> -	if (dev->of_node)
> -		ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
> -	else
> -		ret = iort_pmsi_get_dev_id(dev, &dev_id);
> -	if (ret)
> -		return ret;
> -
>  	/* ITS specific DeviceID, as the core ITS ignores dev. */
>  	info->scratchpad[0].ul = dev_id;
>
> @@ -159,6 +148,36 @@ static int its_pmsi_prepare(struct irq_d
>  					  dev, nvec, info);
>  }
>
> +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> +				  int nvec, msi_alloc_info_t *info)
> +{
> +	u32 dev_id = dev_get_pci_ep_id(dev);
> +	struct msi_domain_info *msi_info;
> +	int ret = -ENOTSUPP;
> +
> +	if (dev->of_node)
> +		ret = do_magic_ep_id_map();
> +	if (ret)
> +		return ret;
> +	return __its_pmsi_prepare(domain, dev, nvec, info, dev_id);
> +}
> +
> +static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> +			    int nvec, msi_alloc_info_t *info)
> +{
> +	struct msi_domain_info *msi_info;
> +	u32 dev_id;
> +	int ret;
> +
> +	if (dev->of_node)
> +		ret = of_pmsi_get_dev_id(domain->parent, dev, &dev_id);
> +	else
> +		ret = iort_pmsi_get_dev_id(dev, &dev_id);
> +	if (ret)
> +		return ret;
> +	return __its_pmsi_prepare(domain, dev, nvec, info, dev_id);
> +}
> +
>  static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
>  				  struct irq_domain *real_parent, struct msi_domain_info *info)
>  {
> @@ -183,6 +202,9 @@ static bool its_init_dev_msi_info(struct
>  		 */
>  		info->ops->msi_prepare = its_pci_msi_prepare;
>  		break;
> +	case DOMAIN_BUS_PCI_DEVICE_EP_MSI:
> +		info->ops->msi_prepare = its_pci_ep_msi_prepare;
> +		break;
>  	case DOMAIN_BUS_DEVICE_MSI:
>  	case DOMAIN_BUS_WIRED_TO_MSI:
>  		/*
> @@ -204,7 +226,7 @@ const struct msi_parent_ops gic_v3_its_m
>  	.supported_flags	= ITS_MSI_FLAGS_SUPPORTED,
>  	.required_flags		= ITS_MSI_FLAGS_REQUIRED,
>  	.bus_select_token	= DOMAIN_BUS_NEXUS,
> -	.bus_select_mask	= MATCH_PCI_MSI | MATCH_PLATFORM_MSI,
> +	.bus_select_mask	= MATCH_PCI_MSI | MATCH_PLATFORM_PCI_EP_MSI | MATCH_PLATFORM_MSI,
>  	.prefix			= "ITS-",
>  	.init_dev_msi_info	= its_init_dev_msi_info,
>  };
> --- a/drivers/irqchip/irq-msi-lib.c
> +++ b/drivers/irqchip/irq-msi-lib.c
> @@ -55,8 +55,11 @@ bool msi_lib_init_dev_msi_info(struct de
>  	case DOMAIN_BUS_PCI_DEVICE_MSIX:
>  		if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_MSI)))
>  			return false;
> -
>  		break;
> +	case DOMAIN_BUS_DEVICE_PCI_EP_MSI:
> +		if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_ENDPOINT)))
> +			return false;
> +		fallthrough;
>  	case DOMAIN_BUS_DEVICE_MSI:
>  		/*
>  		 * Per device MSI should never have any MSI feature bits
> --- a/drivers/irqchip/irq-msi-lib.h
> +++ b/drivers/irqchip/irq-msi-lib.h
> @@ -10,12 +10,18 @@
>  #include <linux/msi.h>
>
>  #ifdef CONFIG_PCI_MSI
> -#define MATCH_PCI_MSI		BIT(DOMAIN_BUS_PCI_MSI)
> +#define MATCH_PCI_MSI			BIT(DOMAIN_BUS_PCI_MSI)
>  #else
> -#define MATCH_PCI_MSI		(0)
> +#define MATCH_PCI_MSI			(0)
>  #endif
>
> -#define MATCH_PLATFORM_MSI	BIT(DOMAIN_BUS_PLATFORM_MSI)
> +#ifdef CONFIG_PCI_ENDPOINT
> +#define MATCH_PLATFORM_PCI_EP_MSI	BIT(DOMAIN_BUS_PLATFORM_PCI_EP_MSI)
> +#else
> +#define MATCH_PLATFORM_PCI_EP_MSI	(0)
> +#endif
> +
> +#define MATCH_PLATFORM_MSI		BIT(DOMAIN_BUS_PLATFORM_MSI)
>
>  int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
>  			      enum irq_domain_bus_token bus_token);
> --- a/include/linux/irqdomain_defs.h
> +++ b/include/linux/irqdomain_defs.h
> @@ -15,6 +15,7 @@ enum irq_domain_bus_token {
>  	DOMAIN_BUS_GENERIC_MSI,
>  	DOMAIN_BUS_PCI_MSI,
>  	DOMAIN_BUS_PLATFORM_MSI,
> +	DOMAIN_BUS_PLATFORM_PCI_EP_MSI,
>  	DOMAIN_BUS_NEXUS,
>  	DOMAIN_BUS_IPI,
>  	DOMAIN_BUS_FSL_MC_MSI,
> @@ -27,6 +28,7 @@ enum irq_domain_bus_token {
>  	DOMAIN_BUS_AMDVI,
>  	DOMAIN_BUS_DEVICE_MSI,
>  	DOMAIN_BUS_WIRED_TO_MSI,
> +	DOMAIN_BUS_DEVICE_PCI_EP_MSI,
>  };
>
>  #endif /* _LINUX_IRQDOMAIN_DEFS_H */


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-12-18 17:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-11 20:57 [PATCH v12 0/9] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
2024-12-11 20:57 ` [PATCH v12 1/9] irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info Frank Li
2024-12-11 20:57 ` [PATCH v12 2/9] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
2024-12-17 22:56   ` Thomas Gleixner
2024-12-18 17:44     ` Frank Li
2024-12-11 20:57 ` [PATCH v12 3/9] irqdomain: Add IRQ_DOMAIN_FLAG_MSI_IMMUTABLE and irq_domain_is_msi_immutable() Frank Li
2024-12-11 20:57 ` [PATCH v12 4/9] irqchip/gic-v3-its: Set IRQ_DOMAIN_FLAG_MSI_IMMUTABLE for ITS Frank Li
2024-12-11 20:57 ` [PATCH v12 5/9] PCI: endpoint: pci-ep-msi: Add check for MSI address/data pair immutability Frank Li
2024-12-11 20:57 ` [PATCH v12 6/9] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment Frank Li
2024-12-11 20:57 ` [PATCH v12 7/9] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
2024-12-11 20:57 ` [PATCH v12 8/9] misc: pci_endpoint_test: Add doorbell test case Frank Li
2024-12-11 20:57 ` [PATCH v12 9/9] tools: PCI: Add 'B' option for test doorbell Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).