[PATCH v4 0/3] pci: endpoint: vntb: add MSI doorbell support

[PATCH v4 0/3] pci: endpoint: vntb: add MSI doorbell support

[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>
---
Changes in v4:
- use 'bar_size' instead of aligned_bar_size
- Link to v3: https://lore.kernel.org/r/20250925-vntb_msi_doorbell-v3-0-ae0b0c93caae@nxp.com

Changes in v3:
- update function name to pci_epf_assign_bar_space()
- detail check each patch's change log
- Link to v2: https://lore.kernel.org/r/20250915-vntb_msi_doorbell-v2-0-ca71605e3444@nxp.com

Changes in v2:
- add help funciton to get bar's inbounce size
- fix miss x8 when caculate bits
- Link to v1: https://lore.kernel.org/r/20250815-vntb_msi_doorbell-v1-0-32df6c4bf96c@nxp.com

---
Frank Li (3):
      PCI: endpoint: Add helper function pci_epf_get_bar_required_size()
      PCI: endpoint: Add API pci_epf_assign_bar_space()
      PCI: endpoint: pci-epf-vntb: Add MSI doorbell support

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 152 +++++++++++++++++++++---
 drivers/pci/endpoint/pci-epf-core.c           | 161 +++++++++++++++++++++-----
 include/linux/pci-epf.h                       |   6 +
 3 files changed, 272 insertions(+), 47 deletions(-)
---
base-commit: c2a282d1fccc53a989da61a5da4f03c9d67ee99a
change-id: 20250812-vntb_msi_doorbell-bf0fbac6d6d7

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

[PATCH v4 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()

[Frank Li]

Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
size and memory size. Prepare for adding support to set an MMIO address to
a specific BAR.

Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid
confuse.

No functional changes.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v4
- use size_t *bar_size as in/out arugment.

change in v3
- change return value to int.
- use two pointers return bar size aligned and memory start address aligned
- update comments about why need memory align size. Actually iATU require
start address match aligned requirement. Since kernel return align to
size's address.
- use two varible aligned_bar_size and aligned_mem_size to avoid confuse
use 'size'.

change in v2
- new patch
---
 drivers/pci/endpoint/pci-epf-core.c | 81 +++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index d54e18872aefc07c655c94c104a347328ff7a432..db0420f601d81de426a3e805c7fc309de58d54b7 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf)
 }
 EXPORT_SYMBOL_GPL(pci_epf_remove_vepf);
 
+static int
+pci_epf_get_bar_required_size(struct pci_epf *epf, size_t *bar_size,
+			      size_t *aligned_mem_size,
+			      enum pci_barno bar,
+			      const struct pci_epc_features *epc_features,
+			      enum pci_epc_interface_type type)
+{
+	u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
+	size_t align = epc_features->align;
+	size_t size = *bar_size;
+
+	if (size < 128)
+		size = 128;
+
+	/* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
+	if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
+		size = SZ_1M;
+
+	if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
+		if (size > bar_fixed_size) {
+			dev_err(&epf->dev,
+				"requested BAR size is larger than fixed size\n");
+			return -ENOMEM;
+		}
+		size = bar_fixed_size;
+	} else {
+		/* BAR size must be power of two */
+		size = roundup_pow_of_two(size);
+	}
+
+	*bar_size = size;
+
+	/*
+	 * The EPC's BAR start address must meet alignment requirements. In most
+	 * cases, the alignment will match the BAR size. However, differences
+	 * can occur—for example, when the fixed BAR size (e.g., 128 bytes) is
+	 * smaller than the required alignment (e.g., 4 KB).
+	 */
+	*aligned_mem_size = align ? ALIGN(size, align) : size;
+
+	return 0;
+}
+
 /**
  * pci_epf_free_space() - free the allocated PCI EPF register space
  * @epf: the EPF device from whom to free the memory
@@ -264,40 +307,16 @@ 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)
 {
-	u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
-	size_t aligned_size, align = epc_features->align;
 	struct pci_epf_bar *epf_bar;
+	size_t aligned_mem_size;
 	dma_addr_t phys_addr;
 	struct pci_epc *epc;
 	struct device *dev;
 	void *space;
 
-	if (size < 128)
-		size = 128;
-
-	/* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
-	if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
-		size = SZ_1M;
-
-	if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
-		if (size > bar_fixed_size) {
-			dev_err(&epf->dev,
-				"requested BAR size is larger than fixed size\n");
-			return NULL;
-		}
-		size = bar_fixed_size;
-	} else {
-		/* BAR size must be power of two */
-		size = roundup_pow_of_two(size);
-	}
-
-	/*
-	 * Allocate enough memory to accommodate the iATU alignment
-	 * requirement.  In most cases, this will be the same as .size but
-	 * it might be different if, for example, the fixed size of a BAR
-	 * is smaller than align.
-	 */
-	aligned_size = align ? ALIGN(size, align) : size;
+	if (pci_epf_get_bar_required_size(epf, &size, &aligned_mem_size, bar,
+					  epc_features, type))
+		return NULL;
 
 	if (type == PRIMARY_INTERFACE) {
 		epc = epf->epc;
@@ -308,7 +327,9 @@ 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);
+
+	space = dma_alloc_coherent(dev, aligned_mem_size,
+				   &phys_addr, GFP_KERNEL);
 	if (!space) {
 		dev_err(dev, "failed to allocate mem space\n");
 		return NULL;
@@ -317,7 +338,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	epf_bar[bar].phys_addr = phys_addr;
 	epf_bar[bar].addr = space;
 	epf_bar[bar].size = size;
-	epf_bar[bar].aligned_size = aligned_size;
+	epf_bar[bar].aligned_size = aligned_mem_size;
 	epf_bar[bar].barno = bar;
 	if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
 		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;

-- 
2.34.1

[PATCH v4 2/3] PCI: endpoint: Add API pci_epf_assign_bar_space()

[Frank Li]

Add pci_epf_assign_bar_space() to allow setting any physical address as
inbound memory space, such as an MSI message base address.

Since PCI BAR size must be a power of two, the input MMIO range
[inbound_addr, inbound_addr + size) is mapped by finding n such that
[base, base + 2^n) covers the range. The base address is also required
to be aligned to 2^n.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v4
- use size for bar size.

chagne in v3.
- update function name to pci_epf_assign_bar_space()
- s/allocated/assigned/
- add check when align down input address to memory align require, may not
bar's size can't cover required ragion.

change in v2
- add new API pci_epf_set_inbound_space()
- fix bits 8 * size_of(dma_addr_t);
---
 drivers/pci/endpoint/pci-epf-core.c | 80 +++++++++++++++++++++++++++++++++++++
 include/linux/pci-epf.h             |  6 +++
 2 files changed, 86 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index db0420f601d81de426a3e805c7fc309de58d54b7..d0b8a02c1de988873d28ede76800d5d4e9810827 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -349,6 +349,86 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 }
 EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
 
+/**
+ * pci_epf_assign_bar_space() - Assign PCI EPF BAR space
+ * @epf: the EPF device to whom allocate the memory
+ * @size: the size of the memory that has to be assigned
+ * @bar: the BAR number corresponding to the assigned register space
+ * @epc_features: the features provided by the EPC specific to this EPF
+ * @type: Identifies if the assignment is for primary EPC or secondary EPC
+ * @bar_addr: Address to be assigned for the @bar
+ *
+ * Invoke to assigned 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.
+ */
+int pci_epf_assign_bar_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,
+			     dma_addr_t bar_addr)
+{
+	size_t bar_size, aligned_mem_size;
+	struct pci_epf_bar *epf_bar;
+	struct pci_epc *epc;
+	dma_addr_t limit;
+	int pos;
+
+	if (!size)
+		return -EINVAL;
+
+	limit = bar_addr + size - 1;
+
+	/*
+	 *  Bits:		15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
+	 *  bar_addr:		U  U  U  U  U  U  0 X X X X X X X X X
+	 *  limit:		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 [bar_addr, limit]
+	 *  X means bit 0 or 1.
+	 *
+	 *  bar_addr^limit	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
+	 *  [bar_Addr, limit] range.
+	 */
+	for (pos = 8 * sizeof(dma_addr_t) - 1; pos > 0; pos--)
+		if ((limit ^ bar_addr) & BIT_ULL(pos))
+			break;
+
+	if (pos == 8 * sizeof(dma_addr_t) - 1)
+		return -EINVAL;
+
+	bar_size = BIT_ULL(pos + 1);
+	if (pci_epf_get_bar_required_size(epf, &bar_size, &aligned_mem_size,
+					  bar, epc_features, type))
+		return -ENOMEM;
+
+	if (type == PRIMARY_INTERFACE) {
+		epc = epf->epc;
+		epf_bar = epf->bar;
+	} else {
+		epc = epf->sec_epc;
+		epf_bar = epf->sec_epc_bar;
+	}
+
+	epf_bar[bar].phys_addr = ALIGN_DOWN(bar_addr, aligned_mem_size);
+
+	if (epf_bar[bar].phys_addr + bar_size < limit)
+		return -ENOMEM;
+
+	epf_bar[bar].addr = NULL;
+	epf_bar[bar].size = bar_size;
+	epf_bar[bar].aligned_size = aligned_mem_size;
+	epf_bar[bar].barno = bar;
+	if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+	else
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_assign_bar_space);
+
 static void pci_epf_remove_cfs(struct pci_epf_driver *driver)
 {
 	struct config_group *group, *tmp;
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 2e85504ba2baf93827224884ca19ae2bd0e6906b..d2cdcc06d366ce61db0d7633105965e50cb49a7b 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -242,6 +242,12 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
 
+int pci_epf_assign_bar_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,
+			     dma_addr_t inbound_addr);
+
 int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
 			       u64 addr, dma_addr_t *base, size_t *off);
 int pci_epf_bind(struct pci_epf *epf);

-- 
2.34.1

[PATCH v4 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support

[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>
---
change in v4
- add check of return value pci_epf_assign_bar_space()

change in v3
- update api pci_epf_assign_bar_space
- remove dead code for db 0.

change in v2
- update api pci_epf_set_inbound_space
- atomic_64 should be enough, which just record doorbell events, which is
similar with W1C irq status register.
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 152 +++++++++++++++++++++++---
 1 file changed, 135 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..6e48a1efe1eb267c4c80c7c09fe1c6f73c7ccf44 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,21 @@ 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);
+		}
+
+	return IRQ_HANDLED;
 }
 
 /**
@@ -500,6 +517,93 @@ 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_assign_bar_space(epf, sz, barno, epc_features, 0, low);
+	if (ret) {
+		dev_err(&epf->dev, "Doorbell BAR assign space failed\n");
+		goto err_request_irq;
+	}
+
+	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,21 +624,25 @@ 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;
 
@@ -554,6 +662,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 +1386,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 +1426,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

Re: [PATCH v4 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()

[Niklas Cassel]

On Tue, Sep 30, 2025 at 04:39:37PM -0400, Frank Li wrote:
> Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
> size and memory size. Prepare for adding support to set an MMIO address to
> a specific BAR.
> 
> Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid
> confuse.

'aligned_bar_size' has been renamed, so this sentence should be updated.


(snip)

> @@ -308,7 +327,9 @@ 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);
> +
> +	space = dma_alloc_coherent(dev, aligned_mem_size,
> +				   &phys_addr, GFP_KERNEL);
>  	if (!space) {
>  		dev_err(dev, "failed to allocate mem space\n");
>  		return NULL;
> @@ -317,7 +338,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>  	epf_bar[bar].phys_addr = phys_addr;
>  	epf_bar[bar].addr = space;
>  	epf_bar[bar].size = size;
> -	epf_bar[bar].aligned_size = aligned_size;
> +	epf_bar[bar].aligned_size = aligned_mem_size;

I like that this local variable is now named aligned_mem_size
to more clearly highlight that it is the aligned _memory_ size.

Perhaps you could also rename the struct pci_epf_bar struct member
'aligned_size' to 'aligned_mem_size' or perhaps even better,
'backing_mem_size' or 'mem_size' ?


Kind regards,
Niklas

Re: [External] : [PATCH v4 2/3] PCI: endpoint: Add API pci_epf_assign_bar_space()

[ALOK TIWARI]

On 10/1/2025 2:09 AM, Frank Li wrote:
> +int pci_epf_assign_bar_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,
> +			     dma_addr_t inbound_addr);

nit: Any particular reason for using inbound_addr instead of bar_addr ?

> +
>   int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
>   			       u64 addr, dma_addr_t *base, size_t *off);
>   int pci_epf_bind(struct pci_epf *epf);


Thanks,
Alok

Re: [External] : [PATCH v4 2/3] PCI: endpoint: Add API pci_epf_assign_bar_space()

[Frank Li]

On Tue, Oct 07, 2025 at 12:25:59AM +0530, ALOK TIWARI wrote:
>
>
> On 10/1/2025 2:09 AM, Frank Li wrote:
> > +int pci_epf_assign_bar_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,
> > +			     dma_addr_t inbound_addr);
>
> nit: Any particular reason for using inbound_addr instead of bar_addr ?

bar_addr is easy to confuse with RC side's bar windows's address. This one
means the ATU convert bar's inbound transfer to what EP side's address.

The below API also use term 'inbound_addr'.

It is not big deal. Manivannan or Niklas Cassel, do you have any perfer?

I am okay for both names.

Frank
>
> > +
> >   int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
> >   			       u64 addr, dma_addr_t *base, size_t *off);
> >   int pci_epf_bind(struct pci_epf *epf);
>
>
> Thanks,
> Alok

Re: [External] : [PATCH v4 2/3] PCI: endpoint: Add API pci_epf_assign_bar_space()

[ALOK TIWARI]

On 10/7/2025 8:41 PM, Frank Li wrote:
> On Tue, Oct 07, 2025 at 12:25:59AM +0530, ALOK TIWARI wrote:
>>
>> On 10/1/2025 2:09 AM, Frank Li wrote:
>>> +int pci_epf_assign_bar_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,
>>> +			     dma_addr_t inbound_addr);
>> nit: Any particular reason for using inbound_addr instead of bar_addr ?
> bar_addr is easy to confuse with RC side's bar windows's address. This one
> means the ATU convert bar's inbound transfer to what EP side's address.
> 
> The below API also use term 'inbound_addr'.
> 
> It is not big deal. Manivannan or Niklas Cassel, do you have any perfer?
> 
> I am okay for both names.

It is not a major issue, just noticed that the .c file refers to 
dma_addr_t bar_addr while the .h file uses inbound_addr.
Just curious to know the reason.

> 
> Frank
>>> +
>>>    int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
>>>    			       u64 addr, dma_addr_t *base, size_t *off);
>>>    int pci_epf_bind(struct pci_epf *epf);
>>
>> Thanks,
>> Alok



Thanks,
Alok

Re: [PATCH v4 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()

[Frank Li]

On Mon, Oct 06, 2025 at 12:43:52PM +0200, Niklas Cassel wrote:
> On Tue, Sep 30, 2025 at 04:39:37PM -0400, Frank Li wrote:
> > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
> > size and memory size. Prepare for adding support to set an MMIO address to
> > a specific BAR.
> >
> > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid
> > confuse.
>
> 'aligned_bar_size' has been renamed, so this sentence should be updated.
>
>
> (snip)
>
> > @@ -308,7 +327,9 @@ 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);
> > +
> > +	space = dma_alloc_coherent(dev, aligned_mem_size,
> > +				   &phys_addr, GFP_KERNEL);
> >  	if (!space) {
> >  		dev_err(dev, "failed to allocate mem space\n");
> >  		return NULL;
> > @@ -317,7 +338,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> >  	epf_bar[bar].phys_addr = phys_addr;
> >  	epf_bar[bar].addr = space;
> >  	epf_bar[bar].size = size;
> > -	epf_bar[bar].aligned_size = aligned_size;
> > +	epf_bar[bar].aligned_size = aligned_mem_size;
>
> I like that this local variable is now named aligned_mem_size
> to more clearly highlight that it is the aligned _memory_ size.
>
> Perhaps you could also rename the struct pci_epf_bar struct member
> 'aligned_size' to 'aligned_mem_size' or perhaps even better,
> 'backing_mem_size' or 'mem_size' ?

How about alloc_mem_size ? which actually used for dma_free() function.

Frank
>
>
> Kind regards,
> Niklas

Re: [PATCH v4 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()

[Niklas Cassel]

On 7 October 2025 20:26:01 CEST, Frank Li <Frank.li@nxp.com> wrote:
>On Mon, Oct 06, 2025 at 12:43:52PM +0200, Niklas Cassel wrote:
>> On Tue, Sep 30, 2025 at 04:39:37PM -0400, Frank Li wrote:
>> > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
>> > size and memory size. Prepare for adding support to set an MMIO address to
>> > a specific BAR.
>> >
>> > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid
>> > confuse.
>>
>> 'aligned_bar_size' has been renamed, so this sentence should be updated.
>>
>>
>> (snip)
>>
>> > @@ -308,7 +327,9 @@ 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);
>> > +
>> > +	space = dma_alloc_coherent(dev, aligned_mem_size,
>> > +				   &phys_addr, GFP_KERNEL);
>> >  	if (!space) {
>> >  		dev_err(dev, "failed to allocate mem space\n");
>> >  		return NULL;
>> > @@ -317,7 +338,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
>> >  	epf_bar[bar].phys_addr = phys_addr;
>> >  	epf_bar[bar].addr = space;
>> >  	epf_bar[bar].size = size;
>> > -	epf_bar[bar].aligned_size = aligned_size;
>> > +	epf_bar[bar].aligned_size = aligned_mem_size;
>>
>> I like that this local variable is now named aligned_mem_size
>> to more clearly highlight that it is the aligned _memory_ size.
>>
>> Perhaps you could also rename the struct pci_epf_bar struct member
>> 'aligned_size' to 'aligned_mem_size' or perhaps even better,
>> 'backing_mem_size' or 'mem_size' ?
>
>How about alloc_mem_size ? which actually used for dma_free() function.


I prefer mem_size, but I am not the maintainer.
Hopefully Mani has an option to avoid bike shedding :)

Have an awesome day!


/Niklas