linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support
@ 2025-08-15 22:20 Frank Li
  2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Frank Li @ 2025-08-15 22:20 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
	Allen Hubbe
  Cc: linux-pci, linux-kernel, ntb, Frank Li

Since commit 1c3b002c6bf68 PCI: endpoint: Add RC-to-EP doorbell support
using platform MSI controller, PCI EP can get notification from Host.

VNTB use this feature to reduce ping latency.

The first patch impove epf core API to allow set any MMIO address to specfic
bar.

The second patch add MSI doorbell support.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (2):
      PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
      PCI: endpoint: pci-epf-vntb: Add MSI doorbell support

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
 drivers/pci/endpoint/pci-epf-core.c           |  69 +++++++++---
 include/linux/pci-epc.h                       |   5 -
 include/linux/pci-epf.h                       |  35 +++++-
 4 files changed, 223 insertions(+), 39 deletions(-)
---
base-commit: c2a282d1fccc53a989da61a5da4f03c9d67ee99a
change-id: 20250812-vntb_msi_doorbell-bf0fbac6d6d7

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


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

* [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
  2025-08-15 22:20 [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support Frank Li
@ 2025-08-15 22:20 ` Frank Li
  2025-08-29 17:21   ` ALOK TIWARI
  2025-08-30 14:46   ` Manivannan Sadhasivam
  2025-08-15 22:20 ` [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
  2025-08-29 16:35 ` [PATCH 0/2] pci: endpoint: vntb: add " Frank Li
  2 siblings, 2 replies; 8+ messages in thread
From: Frank Li @ 2025-08-15 22:20 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
	Allen Hubbe
  Cc: linux-pci, linux-kernel, ntb, Frank Li

Enhance pci_epf_alloc_space() to handle setting any physical address as
inbound memory space, such as an MSI message base address. The function
already accounts for different alignment requirements for different BARs,
so reuse this logic and rename the function to pci_epf_set_inbound_space().

Make pci_epf_alloc_space() inline and call pci_epf_set_inbound_space() with
from_space set to true to maintain compatibility.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
---
 drivers/pci/endpoint/pci-epf-core.c | 69 ++++++++++++++++++++++++++++++-------
 include/linux/pci-epc.h             |  5 ---
 include/linux/pci-epf.h             | 35 ++++++++++++++++---
 3 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index d54e18872aefc07c655c94c104a347328ff7a432..5b802b1ea3e28a32e38f4ab6a649cb97a2f29b95 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -249,20 +249,26 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 EXPORT_SYMBOL_GPL(pci_epf_free_space);
 
 /**
- * pci_epf_alloc_space() - allocate memory for the PCI EPF register space
+ * pci_epf_set_inbound_space() - set memory for the PCI EPF inbound address space
  * @epf: the EPF device to whom allocate the memory
  * @size: the size of the memory that has to be allocated
  * @bar: the BAR number corresponding to the allocated register space
  * @epc_features: the features provided by the EPC specific to this EPF
  * @type: Identifies if the allocation is for primary EPC or secondary EPC
+ * @from_memory: allocate from system memory
+ * @inbound_addr: Any physical address space such as MSI message address that
+ *                work as inbound address space. from_memory need be false.
  *
  * Invoke to allocate memory for the PCI EPF register space.
  * Flag PCI_BASE_ADDRESS_MEM_TYPE_64 will automatically get set if the BAR
  * can only be a 64-bit BAR, or if the requested size is larger than 2 GB.
  */
-void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
-			  const struct pci_epc_features *epc_features,
-			  enum pci_epc_interface_type type)
+int pci_epf_set_inbound_space(struct pci_epf *epf, size_t size,
+			      enum pci_barno bar,
+			      const struct pci_epc_features *epc_features,
+			      enum pci_epc_interface_type type,
+			      bool from_memory,
+			      dma_addr_t inbound_addr)
 {
 	u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
 	size_t aligned_size, align = epc_features->align;
@@ -270,7 +276,32 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	dma_addr_t phys_addr;
 	struct pci_epc *epc;
 	struct device *dev;
-	void *space;
+	void *space = NULL;
+	dma_addr_t up;
+
+	up = inbound_addr + size - 1;
+
+	/*
+	 *  Bits:            15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
+	 *  Inbound_addr:    U  U  U  U  U  U  0 X X X X X X X X X
+	 *  Up:              U  U  U  U  U  U  1 X X X X X X X X X
+	 *
+	 *  U means address bits have not change in Range [Inbound_Addr, Up]
+	 *  X means bit 0 or 1.
+	 *
+	 *  Inbound_addr^Up  0  0  0  0  0  0  1 X X X X X X X X X
+	 *  Find first bit 1 pos from MSB, 2 ^ pos windows will cover
+	 *  [Inbound_Addr, Up] range.
+	 */
+	if (!from_memory) {
+		int pos;
+
+		for (pos = sizeof(dma_addr_t) - 1; pos > 0; pos--)
+			if ((up ^ inbound_addr) & BIT_ULL(pos))
+				break;
+
+		size = 1 << pos;
+	}
 
 	if (size < 128)
 		size = 128;
@@ -283,7 +314,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 		if (size > bar_fixed_size) {
 			dev_err(&epf->dev,
 				"requested BAR size is larger than fixed size\n");
-			return NULL;
+			return -EINVAL;
 		}
 		size = bar_fixed_size;
 	} else {
@@ -308,13 +339,25 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	}
 
 	dev = epc->dev.parent;
-	space = dma_alloc_coherent(dev, aligned_size, &phys_addr, GFP_KERNEL);
-	if (!space) {
-		dev_err(dev, "failed to allocate mem space\n");
-		return NULL;
+
+	if (from_memory) {
+		space = dma_alloc_coherent(dev, aligned_size,
+					   &phys_addr, GFP_KERNEL);
+		if (!space) {
+			dev_err(dev, "failed to allocate mem space\n");
+			return -ENOMEM;
+		}
+	}
+
+	epf_bar[bar].phys_addr = from_memory ?
+			phys_addr : ALIGN_DOWN(inbound_addr, aligned_size);
+
+	if (!from_memory && (epf_bar[bar].phys_addr + size < up)) {
+		dev_err(&epf->dev,
+			"Given bar can't fit required inbound memory region\n");
+		return -EINVAL;
 	}
 
-	epf_bar[bar].phys_addr = phys_addr;
 	epf_bar[bar].addr = space;
 	epf_bar[bar].size = size;
 	epf_bar[bar].aligned_size = aligned_size;
@@ -324,9 +367,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	else
 		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
 
-	return space;
+	return 0;
 }
-EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
+EXPORT_SYMBOL_GPL(pci_epf_set_inbound_space);
 
 static void pci_epf_remove_cfs(struct pci_epf_driver *driver)
 {
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 4286bfdbfdfad2754d763be2b8474e5d2d403a1f..5f1d8787c3bb7a6130adb54c079a48c82d5afcf4 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -13,11 +13,6 @@
 
 struct pci_epc;
 
-enum pci_epc_interface_type {
-	UNKNOWN_INTERFACE = -1,
-	PRIMARY_INTERFACE,
-	SECONDARY_INTERFACE,
-};
 
 static inline const char *
 pci_epc_interface_string(enum pci_epc_interface_type type)
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 2e85504ba2baf93827224884ca19ae2bd0e6906b..4311a8ba1081b8a8aa327c8ed10f9c5d9af2feba 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -17,7 +17,12 @@
 
 struct pci_epf;
 struct pci_epc_features;
-enum pci_epc_interface_type;
+
+enum pci_epc_interface_type {
+	UNKNOWN_INTERFACE = -1,
+	PRIMARY_INTERFACE,
+	SECONDARY_INTERFACE,
+};
 
 enum pci_barno {
 	NO_BAR = -1,
@@ -236,9 +241,31 @@ void pci_epf_destroy(struct pci_epf *epf);
 int __pci_epf_register_driver(struct pci_epf_driver *driver,
 			      struct module *owner);
 void pci_epf_unregister_driver(struct pci_epf_driver *driver);
-void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
-			  const struct pci_epc_features *epc_features,
-			  enum pci_epc_interface_type type);
+
+int pci_epf_set_inbound_space(struct pci_epf *epf, size_t size,
+			      enum pci_barno bar,
+			      const struct pci_epc_features *epc_features,
+			      enum pci_epc_interface_type type,
+			      bool from_memory,
+			      dma_addr_t inbound_addr);
+
+static inline void *
+pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
+		    const struct pci_epc_features *epc_features,
+		    enum pci_epc_interface_type type)
+{
+	int ret;
+
+	ret = pci_epf_set_inbound_space(epf, size, bar, epc_features, type,
+					true, 0);
+
+	if (ret)
+		return NULL;
+
+	return type == PRIMARY_INTERFACE ? epf->bar[bar].addr :
+					   epf->sec_epc_bar[bar].addr;
+}
+
 void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
 

-- 
2.34.1


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

* [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
  2025-08-15 22:20 [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support Frank Li
  2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
@ 2025-08-15 22:20 ` Frank Li
  2025-08-30 15:44   ` Manivannan Sadhasivam
  2025-08-29 16:35 ` [PATCH 0/2] pci: endpoint: vntb: add " Frank Li
  2 siblings, 1 reply; 8+ messages in thread
From: Frank Li @ 2025-08-15 22:20 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
	Allen Hubbe
  Cc: linux-pci, linux-kernel, ntb, Frank Li

Add MSI doorbell support to reduce latency between PCI host and EP.

Before this change:
  ping 169.254.172.137
  64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
  64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
  64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
  64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms

After this change:
  ping 169.254.144.71
  64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
  64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
  64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms

Change u64 db to atomic_64 because difference doorbell may happen at the
same time.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 83e9ab10f9c4fc2b485d5463faa2172500f12999..1c586205835fe9c7c5352e74819bccb4ece84438 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -36,11 +36,13 @@
  * PCIe Root Port                        PCI EP
  */
 
+#include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
+#include <linux/pci-ep-msi.h>
 #include <linux/pci-epc.h>
 #include <linux/pci-epf.h>
 #include <linux/ntb.h>
@@ -126,12 +128,13 @@ struct epf_ntb {
 	u32 db_count;
 	u32 spad_count;
 	u64 mws_size[MAX_MW];
-	u64 db;
+	atomic64_t db;
 	u32 vbus_number;
 	u16 vntb_pid;
 	u16 vntb_vid;
 
 	bool linkup;
+	bool msi_doorbell;
 	u32 spad_size;
 
 	enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
@@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 
 	ntb = container_of(work, struct epf_ntb, cmd_handler.work);
 
-	for (i = 1; i < ntb->db_count; i++) {
+	for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
 		if (ntb->epf_db[i]) {
-			ntb->db |= 1 << (i - 1);
+			atomic64_or(1 << (i - 1), &ntb->db);
 			ntb_db_event(&ntb->ntb, i);
 			ntb->epf_db[i] = 0;
 		}
@@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 
 reset_handler:
 	queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
-			   msecs_to_jiffies(5));
+			   ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
+}
+
+static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
+{
+	struct epf_ntb *ntb = data;
+	int i = 0;
+
+	for (i = 1; i < ntb->db_count; i++)
+		if (irq == ntb->epf->db_msg[i].virq) {
+			atomic64_or(1 << (i - 1), &ntb->db);
+			ntb_db_event(&ntb->ntb, i);
+		}
+
+	if (irq == ntb->epf->db_msg[0].virq)
+		queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
+
+	return IRQ_HANDLED;
 }
 
 /**
@@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
 	return 0;
 }
 
+static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
+					    struct pci_epf_bar *db_bar,
+					    const struct pci_epc_features *epc_features,
+					    enum pci_barno barno)
+{
+	struct pci_epf *epf = ntb->epf;
+	dma_addr_t low, high;
+	struct msi_msg *msg;
+	size_t sz;
+	int ret;
+	int i;
+
+	ret = pci_epf_alloc_doorbell(epf,  ntb->db_count);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ntb->db_count; i++) {
+		ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
+				  0, "vntb_db", ntb);
+
+		if (ret) {
+			dev_err(&epf->dev,
+				"Failed to request doorbell IRQ: %d\n",
+				epf->db_msg[i].virq);
+			goto err_request_irq;
+		}
+	}
+
+	msg = &epf->db_msg[0].msg;
+
+	high = 0;
+	low = (u64)msg->address_hi << 32 | msg->address_lo;
+
+	for (i = 0; i < ntb->db_count; i++) {
+		struct msi_msg *msg = &epf->db_msg[i].msg;
+		dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
+
+		low = min(low, addr);
+		high = max(high, addr);
+	}
+
+	sz = high - low + sizeof(u32);
+
+	ret = pci_epf_set_inbound_space(epf, sz, barno,
+					epc_features, 0, false, low);
+
+	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, db_bar);
+	if (ret) {
+		dev_err(&epf->dev, "Doorbell BAR set failed\n");
+		goto err_request_irq;
+	}
+
+	for (i = 0; i < ntb->db_count; i++) {
+		struct msi_msg *msg = &epf->db_msg[i].msg;
+		dma_addr_t addr;
+		size_t offset;
+
+		ret = pci_epf_align_inbound_addr(epf, db_bar->barno,
+				((u64)msg->address_hi << 32) | msg->address_lo,
+				&addr, &offset);
+
+		if (ret) {
+			ntb->msi_doorbell = false;
+			goto err_request_irq;
+		}
+
+		ntb->reg->db_data[i] = msg->data;
+		ntb->reg->db_offset[i] = offset;
+	}
+
+	ntb->reg->db_entry_size = 0;
+
+	ntb->msi_doorbell = true;
+
+	return 0;
+
+err_request_irq:
+	for (i--; i >= 0; i--)
+		free_irq(epf->db_msg[i].virq, ntb);
+
+	pci_epf_free_doorbell(ntb->epf);
+	return ret;
+}
+
 /**
  * epf_ntb_db_bar_init() - Configure Doorbell window BARs
  * @ntb: NTB device that facilitates communication between HOST and VHOST
@@ -520,22 +624,27 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
 					    ntb->epf->func_no,
 					    ntb->epf->vfunc_no);
 	barno = ntb->epf_ntb_bar[BAR_DB];
-
-	mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, epc_features, 0);
-	if (!mw_addr) {
-		dev_err(dev, "Failed to allocate OB address\n");
-		return -ENOMEM;
-	}
-
-	ntb->epf_db = mw_addr;
-
 	epf_bar = &ntb->epf->bar[barno];
 
-	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
+	ret = epf_ntb_db_bar_init_msi_doorbell(ntb, epf_bar, epc_features, barno);
 	if (ret) {
-		dev_err(dev, "Doorbell BAR set failed\n");
+		/* fall back to polling mode */
+		mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, epc_features, 0);
+		if (!mw_addr) {
+			dev_err(dev, "Failed to allocate OB address\n");
+			return -ENOMEM;
+		}
+
+		ntb->epf_db = mw_addr;
+
+		ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
+				      ntb->epf->vfunc_no, epf_bar);
+		if (ret) {
+			dev_err(dev, "Doorbell BAR set failed\n");
 			goto err_alloc_peer_mem;
+		}
 	}
+
 	return ret;
 
 err_alloc_peer_mem:
@@ -554,6 +663,16 @@ static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
 {
 	enum pci_barno barno;
 
+	if (ntb->msi_doorbell) {
+		int i;
+
+		for (i = 0; i < ntb->db_count; i++)
+			free_irq(ntb->epf->db_msg[i].virq, ntb);
+	}
+
+	if (ntb->epf->db_msg)
+		pci_epf_free_doorbell(ntb->epf);
+
 	barno = ntb->epf_ntb_bar[BAR_DB];
 	pci_epf_free_space(ntb->epf, ntb->epf_db, barno, 0);
 	pci_epc_clear_bar(ntb->epf->epc,
@@ -1268,7 +1387,7 @@ static u64 vntb_epf_db_read(struct ntb_dev *ndev)
 {
 	struct epf_ntb *ntb = ntb_ndev(ndev);
 
-	return ntb->db;
+	return atomic64_read(&ntb->db);
 }
 
 static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
@@ -1308,7 +1427,7 @@ static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
 {
 	struct epf_ntb *ntb = ntb_ndev(ndev);
 
-	ntb->db &= ~db_bits;
+	atomic64_and(~db_bits, &ntb->db);
 	return 0;
 }
 

-- 
2.34.1


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

* Re: [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support
  2025-08-15 22:20 [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support Frank Li
  2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
  2025-08-15 22:20 ` [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
@ 2025-08-29 16:35 ` Frank Li
  2025-08-29 16:41   ` Manivannan Sadhasivam
  2 siblings, 1 reply; 8+ messages in thread
From: Frank Li @ 2025-08-29 16:35 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
	Allen Hubbe
  Cc: linux-pci, linux-kernel, ntb

On Fri, Aug 15, 2025 at 06:20:52PM -0400, Frank Li wrote:
> Since commit 1c3b002c6bf68 PCI: endpoint: Add RC-to-EP doorbell support
> using platform MSI controller, PCI EP can get notification from Host.
>
> VNTB use this feature to reduce ping latency.
>
> The first patch impove epf core API to allow set any MMIO address to specfic
> bar.
>
> The second patch add MSI doorbell support.

Manivannan Sadhasivam:

	Do you have time to review this patch?

Frank

>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Frank Li (2):
>       PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
>       PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
>
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
>  drivers/pci/endpoint/pci-epf-core.c           |  69 +++++++++---
>  include/linux/pci-epc.h                       |   5 -
>  include/linux/pci-epf.h                       |  35 +++++-
>  4 files changed, 223 insertions(+), 39 deletions(-)
> ---
> base-commit: c2a282d1fccc53a989da61a5da4f03c9d67ee99a
> change-id: 20250812-vntb_msi_doorbell-bf0fbac6d6d7
>
> Best regards,
> --
> Frank Li <Frank.Li@nxp.com>
>

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

* Re: [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support
  2025-08-29 16:35 ` [PATCH 0/2] pci: endpoint: vntb: add " Frank Li
@ 2025-08-29 16:41   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-29 16:41 UTC (permalink / raw)
  To: Frank Li
  Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
	Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb

On Fri, Aug 29, 2025 at 12:35:48PM GMT, Frank Li wrote:
> On Fri, Aug 15, 2025 at 06:20:52PM -0400, Frank Li wrote:
> > Since commit 1c3b002c6bf68 PCI: endpoint: Add RC-to-EP doorbell support
> > using platform MSI controller, PCI EP can get notification from Host.
> >
> > VNTB use this feature to reduce ping latency.
> >
> > The first patch impove epf core API to allow set any MMIO address to specfic
> > bar.
> >
> > The second patch add MSI doorbell support.
> 
> Manivannan Sadhasivam:
> 
> 	Do you have time to review this patch?
> 

Sorry for the delay. It is still in my review queue. Will get to it this
weekend.

- Mani

> Frank
> 
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Frank Li (2):
> >       PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
> >       PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
> >
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> >  drivers/pci/endpoint/pci-epf-core.c           |  69 +++++++++---
> >  include/linux/pci-epc.h                       |   5 -
> >  include/linux/pci-epf.h                       |  35 +++++-
> >  4 files changed, 223 insertions(+), 39 deletions(-)
> > ---
> > base-commit: c2a282d1fccc53a989da61a5da4f03c9d67ee99a
> > change-id: 20250812-vntb_msi_doorbell-bf0fbac6d6d7
> >
> > Best regards,
> > --
> > Frank Li <Frank.Li@nxp.com>
> >

-- 
மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
  2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
@ 2025-08-29 17:21   ` ALOK TIWARI
  2025-08-30 14:46   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 8+ messages in thread
From: ALOK TIWARI @ 2025-08-29 17:21 UTC (permalink / raw)
  To: Frank Li, Manivannan Sadhasivam, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
	Allen Hubbe
  Cc: linux-pci, linux-kernel, ntb



On 8/16/2025 3:50 AM, Frank Li wrote:
> +int pci_epf_set_inbound_space(struct pci_epf *epf, size_t size,
> +			      enum pci_barno bar,
> +			      const struct pci_epc_features *epc_features,
> +			      enum pci_epc_interface_type type,
> +			      bool from_memory,
> +			      dma_addr_t inbound_addr)
>   {
>   	u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
>   	size_t aligned_size, align = epc_features->align;
> @@ -270,7 +276,32 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>   	dma_addr_t phys_addr;
>   	struct pci_epc *epc;
>   	struct device *dev;
> -	void *space;
> +	void *space = NULL;
> +	dma_addr_t up;
> +
> +	up = inbound_addr + size - 1;
> +
> +	/*
> +	 *  Bits:            15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
> +	 *  Inbound_addr:    U  U  U  U  U  U  0 X X X X X X X X X
> +	 *  Up:              U  U  U  U  U  U  1 X X X X X X X X X
> +	 *
> +	 *  U means address bits have not change in Range [Inbound_Addr, Up]
> +	 *  X means bit 0 or 1.
> +	 *
> +	 *  Inbound_addr^Up  0  0  0  0  0  0  1 X X X X X X X X X
> +	 *  Find first bit 1 pos from MSB, 2 ^ pos windows will cover
> +	 *  [Inbound_Addr, Up] range.
> +	 */
> +	if (!from_memory) {
> +		int pos;
> +
> +		for (pos = sizeof(dma_addr_t) - 1; pos > 0; pos--)
> +			if ((up ^ inbound_addr) & BIT_ULL(pos))
> +				break;
> +

sizeof(dma_addr_t) returns bytes, not bits.
so 7..1 time loop in enough here ?

> +		size = 1 << pos;
> +	}
>   
>   	if (size < 128)
>   		size = 128;
> @@ -283,7 +314,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>   		if (size > bar_fixed_size) {
>   			dev_err(&epf->dev,
>   				"requested BAR size is larger than fixed size\n");
> -			return NULL;
> +			return -EINVAL;
>   		}
>   		size = bar_fixed_size;
>   	} else {
> @@ -308,13 +339,25 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>   	}
>   
>   	dev = epc->dev.parent;
> -	space = dma_alloc_coherent(dev, aligned_size, &phys_addr, GFP_KERNEL);
> -	if (!space) {
> -		dev_err(dev, "failed to allocate mem space\n");
> -		return NULL;
> +
> +	if (from_memory) {
> +		space = dma_alloc_coherent(dev, aligned_size,
> +					   &phys_addr, GFP_KERNEL);
> +		if (!space) {
> +			dev_err(dev, "failed to allocate mem space\n");
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	epf_bar[bar].phys_addr = from_memory ?
> +			phys_addr : ALIGN_DOWN(inbound_addr, aligned_size);
> +
> +	if (!from_memory && (epf_bar[bar].phys_addr + size < up)) {
> +		dev_err(&epf->dev,
> +			"Given bar can't fit required inbound memory region\n");

consider bar -> BAR

> +		return -EINVAL;
>   	}
>   


Thanks,
Alok

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

* Re: [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space()
  2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
  2025-08-29 17:21   ` ALOK TIWARI
@ 2025-08-30 14:46   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 14:46 UTC (permalink / raw)
  To: Frank Li
  Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
	Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb

On Fri, Aug 15, 2025 at 06:20:53PM GMT, Frank Li wrote:
> Enhance pci_epf_alloc_space() to handle setting any physical address as
> inbound memory space, such as an MSI message base address. The function
> already accounts for different alignment requirements for different BARs,
> so reuse this logic and rename the function to pci_epf_set_inbound_space().
> 

I don't think combining both APIs is a good idea. One allocates space for
inbound memory/populates epf_bar and another reuses existing memory/populates
epf_bar. Combining both, logically makes little sense and another makes the code
messy.

If you want to reuse the alignment checks and epf_bar setting from
pci_epf_alloc_space(), then create a separate helper(s) out of it and call from
both APIs.

> Make pci_epf_alloc_space() inline and call pci_epf_set_inbound_space() with
> from_space set to true to maintain compatibility.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> ---
>  drivers/pci/endpoint/pci-epf-core.c | 69 ++++++++++++++++++++++++++++++-------
>  include/linux/pci-epc.h             |  5 ---
>  include/linux/pci-epf.h             | 35 ++++++++++++++++---
>  3 files changed, 87 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index d54e18872aefc07c655c94c104a347328ff7a432..5b802b1ea3e28a32e38f4ab6a649cb97a2f29b95 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -249,20 +249,26 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
>  EXPORT_SYMBOL_GPL(pci_epf_free_space);
>  
>  /**
> - * pci_epf_alloc_space() - allocate memory for the PCI EPF register space
> + * pci_epf_set_inbound_space() - set memory for the PCI EPF inbound address space
>   * @epf: the EPF device to whom allocate the memory
>   * @size: the size of the memory that has to be allocated
>   * @bar: the BAR number corresponding to the allocated register space
>   * @epc_features: the features provided by the EPC specific to this EPF
>   * @type: Identifies if the allocation is for primary EPC or secondary EPC
> + * @from_memory: allocate from system memory
> + * @inbound_addr: Any physical address space such as MSI message address that
> + *                work as inbound address space. from_memory need be false.
>   *
>   * Invoke to allocate memory for the PCI EPF register space.
>   * Flag PCI_BASE_ADDRESS_MEM_TYPE_64 will automatically get set if the BAR
>   * can only be a 64-bit BAR, or if the requested size is larger than 2 GB.
>   */
> -void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> -			  const struct pci_epc_features *epc_features,
> -			  enum pci_epc_interface_type type)
> +int pci_epf_set_inbound_space(struct pci_epf *epf, size_t size,
> +			      enum pci_barno bar,
> +			      const struct pci_epc_features *epc_features,
> +			      enum pci_epc_interface_type type,
> +			      bool from_memory,
> +			      dma_addr_t inbound_addr)
>  {
>  	u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
>  	size_t aligned_size, align = epc_features->align;
> @@ -270,7 +276,32 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>  	dma_addr_t phys_addr;
>  	struct pci_epc *epc;
>  	struct device *dev;
> -	void *space;
> +	void *space = NULL;
> +	dma_addr_t up;
> +
> +	up = inbound_addr + size - 1;
> +
> +	/*
> +	 *  Bits:            15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
> +	 *  Inbound_addr:    U  U  U  U  U  U  0 X X X X X X X X X
> +	 *  Up:              U  U  U  U  U  U  1 X X X X X X X X X
> +	 *
> +	 *  U means address bits have not change in Range [Inbound_Addr, Up]
> +	 *  X means bit 0 or 1.
> +	 *
> +	 *  Inbound_addr^Up  0  0  0  0  0  0  1 X X X X X X X X X
> +	 *  Find first bit 1 pos from MSB, 2 ^ pos windows will cover
> +	 *  [Inbound_Addr, Up] range.

On what basis this size calculation is used?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
  2025-08-15 22:20 ` [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
@ 2025-08-30 15:44   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 15:44 UTC (permalink / raw)
  To: Frank Li
  Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
	Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb

On Fri, Aug 15, 2025 at 06:20:54PM GMT, Frank Li wrote:
> Add MSI doorbell support to reduce latency between PCI host and EP.
> 
> Before this change:
>   ping 169.254.172.137
>   64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
>   64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
>   64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
>   64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
> 
> After this change:
>   ping 169.254.144.71
>   64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
>   64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
>   64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
> 
> Change u64 db to atomic_64 because difference doorbell may happen at the
> same time.
> 

Only the atomicity of db variable is enough?

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
>  1 file changed, 136 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 83e9ab10f9c4fc2b485d5463faa2172500f12999..1c586205835fe9c7c5352e74819bccb4ece84438 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -36,11 +36,13 @@
>   * PCIe Root Port                        PCI EP
>   */
>  
> +#include <linux/atomic.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  
> +#include <linux/pci-ep-msi.h>
>  #include <linux/pci-epc.h>
>  #include <linux/pci-epf.h>
>  #include <linux/ntb.h>
> @@ -126,12 +128,13 @@ struct epf_ntb {
>  	u32 db_count;
>  	u32 spad_count;
>  	u64 mws_size[MAX_MW];
> -	u64 db;
> +	atomic64_t db;
>  	u32 vbus_number;
>  	u16 vntb_pid;
>  	u16 vntb_vid;
>  
>  	bool linkup;
> +	bool msi_doorbell;
>  	u32 spad_size;
>  
>  	enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>  
>  	ntb = container_of(work, struct epf_ntb, cmd_handler.work);
>  
> -	for (i = 1; i < ntb->db_count; i++) {
> +	for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
>  		if (ntb->epf_db[i]) {
> -			ntb->db |= 1 << (i - 1);
> +			atomic64_or(1 << (i - 1), &ntb->db);
>  			ntb_db_event(&ntb->ntb, i);
>  			ntb->epf_db[i] = 0;
>  		}
> @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>  
>  reset_handler:
>  	queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> -			   msecs_to_jiffies(5));
> +			   ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> +}
> +
> +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> +{
> +	struct epf_ntb *ntb = data;
> +	int i = 0;
> +
> +	for (i = 1; i < ntb->db_count; i++)
> +		if (irq == ntb->epf->db_msg[i].virq) {
> +			atomic64_or(1 << (i - 1), &ntb->db);
> +			ntb_db_event(&ntb->ntb, i);
> +		}
> +
> +	if (irq == ntb->epf->db_msg[0].virq)
> +		queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> +
> +	return IRQ_HANDLED;
>  }
>  
>  /**
> @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  	return 0;
>  }
>  
> +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> +					    struct pci_epf_bar *db_bar,
> +					    const struct pci_epc_features *epc_features,
> +					    enum pci_barno barno)
> +{
> +	struct pci_epf *epf = ntb->epf;
> +	dma_addr_t low, high;
> +	struct msi_msg *msg;
> +	size_t sz;
> +	int ret;
> +	int i;
> +
> +	ret = pci_epf_alloc_doorbell(epf,  ntb->db_count);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < ntb->db_count; i++) {
> +		ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> +				  0, "vntb_db", ntb);
> +
> +		if (ret) {
> +			dev_err(&epf->dev,
> +				"Failed to request doorbell IRQ: %d\n",
> +				epf->db_msg[i].virq);
> +			goto err_request_irq;
> +		}
> +	}
> +
> +	msg = &epf->db_msg[0].msg;
> +
> +	high = 0;
> +	low = (u64)msg->address_hi << 32 | msg->address_lo;

Can you remind me when the 'address_{hi/lo}' pairs are set?

Rest looks OK to me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

end of thread, other threads:[~2025-08-30 15:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 22:20 [PATCH 0/2] pci: endpoint: vntb: add MSI doorbell support Frank Li
2025-08-15 22:20 ` [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and rename to pci_epf_set_inbound_space() Frank Li
2025-08-29 17:21   ` ALOK TIWARI
2025-08-30 14:46   ` Manivannan Sadhasivam
2025-08-15 22:20 ` [PATCH 2/2] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
2025-08-30 15:44   ` Manivannan Sadhasivam
2025-08-29 16:35 ` [PATCH 0/2] pci: endpoint: vntb: add " Frank Li
2025-08-29 16:41   ` Manivannan Sadhasivam

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