[PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Koichiro Den]

Hi,

Some DWC endpoint controllers keep the MSI-X Table and PBA at fixed
locations in a reserved BAR. On RK3588, the MSI-X doorbell uses this
hardware-owned layout, as discussed in e.g. [1].

The reserved-region types for the Table and PBA were added with the
Tegra194 description. The MSI-X setup API, however, still takes only the
Table BAR and offset and assumes that the PBA immediately follows it in
the same BAR.

This series passes the complete layout to pci_epc_set_msix() and adds
pci_epc_get_hw_msix_layout() to expose a hardware-owned layout. The core
does not select it automatically. The choice stays with the EPF.

A hardware-owned layout is not a direct replacement for every caller.
pci-epf-ntb, for example, reads host-programmed Table entries from its
own BAR to set up peer outbound mappings.

The series also fixes an interrupt-type mismatch in vNTB. ntb_hw_epf can
select MSI-X, while pci-epf-vntb currently configures and raises only
MSI. On RK3588, selecting the fixed BAR4 layout also selects the DWC
MSI-X doorbell. EPF-owned layouts continue to use the regular path.

In short:

- pci-epf-vntb gains MSI-X support and uses the hardware-owned layout
  when available.
- pci-epf-test, pci-epf-ntb, and the NVMe PCI EPF keep their EPF-owned
  layouts. This avoids unnecessary changes and reduces regression risk.
  They can use a hardware-owned layout later if/when needed.

[1] https://lore.kernel.org/r/aY2q80zeRKSRO21H@fedora

Best regards,
Koichiro
---
Changes in v2:
  - Return an error if the regular MSI-X helper is called for a
    hardware-owned layout. (Sashiko)
  - Collect Frank's Reviewed-by tag.

v1: https://lore.kernel.org/r/20260827182012.1984960-1-den@valinux.co.jp/


Koichiro Den (3):
  PCI: endpoint: Support hardware-owned MSI-X table and PBA
  PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588
  PCI: endpoint: pci-epf-vntb: Honor MSI-X selection

 drivers/nvme/target/pci-epf.c                 |  16 ++-
 .../pci/controller/cadence/pcie-cadence-ep.c  |   9 +-
 .../pci/controller/dwc/pcie-designware-ep.c   |  30 +++++-
 drivers/pci/controller/dwc/pcie-designware.h  |   1 +
 drivers/pci/controller/dwc/pcie-dw-rockchip.c |  28 ++++-
 drivers/pci/endpoint/functions/pci-epf-ntb.c  |  30 +++---
 drivers/pci/endpoint/functions/pci-epf-test.c |  17 +--
 drivers/pci/endpoint/functions/pci-epf-vntb.c |  67 +++++++++---
 drivers/pci/endpoint/pci-epc-core.c           | 102 +++++++++++++++++-
 include/linux/pci-epc.h                       |  25 ++++-
 10 files changed, 268 insertions(+), 57 deletions(-)


base-commit: 0a6f72eda328c773324555491e1ad7f0b0153155
-- 
2.51.0

[PATCH v2 1/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Koichiro Den]

Some endpoint controllers expose the MSI-X table and Pending Bit Array
(PBA) in fixed, hardware-owned BAR regions. The EPC set_msix() callback
currently receives only the table BAR and offset and assumes that the PBA
immediately follows the table in the same BAR. It cannot describe a fixed
layout whose table and PBA have independent locations.

Add struct pci_epc_msix_layout and pass the full layout to
pci_epc_set_msix(). Leave layout selection to EPF implementations. For
instance, pci-epf-ntb reads the MSI-X Table from its own BAR. Using a
fixed layout there would require a way to read hardware-owned Table
entries. Add pci_epc_get_hw_msix_layout() for EPFs that want the fixed
layout. Update the existing callers and validate the supplied layout.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/nvme/target/pci-epf.c                 |  16 ++-
 .../pci/controller/cadence/pcie-cadence-ep.c  |   9 +-
 .../pci/controller/dwc/pcie-designware-ep.c   |   7 +-
 drivers/pci/endpoint/functions/pci-epf-ntb.c  |  30 +++---
 drivers/pci/endpoint/functions/pci-epf-test.c |  17 +--
 drivers/pci/endpoint/pci-epc-core.c           | 102 +++++++++++++++++-
 include/linux/pci-epc.h                       |  25 ++++-
 7 files changed, 168 insertions(+), 38 deletions(-)

diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 4e9db96ebfec..37182f6d29b1 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -201,7 +201,7 @@ struct nvmet_pci_epf {
 	const struct pci_epc_features	*epc_features;
 
 	void				*reg_bar;
-	size_t				msix_table_offset;
+	struct pci_epc_msix_layout	msix_layout;
 
 	unsigned int			irq_type;
 	unsigned int			nr_vectors;
@@ -2187,8 +2187,14 @@ static int nvmet_pci_epf_configure_bar(struct nvmet_pci_epf *nvme_epf)
 		size_t pba_size;
 
 		msix_table_size = PCI_MSIX_ENTRY_SIZE * epf->msix_interrupts;
-		nvme_epf->msix_table_offset = reg_size;
-		pba_size = ALIGN(DIV_ROUND_UP(epf->msix_interrupts, 8), 8);
+		pba_size = BITS_TO_U64(epf->msix_interrupts) * sizeof(u64);
+
+		nvme_epf->msix_layout.table_bar = BAR_0;
+		nvme_epf->msix_layout.table_offset = reg_size;
+		nvme_epf->msix_layout.table_size = msix_table_size;
+		nvme_epf->msix_layout.pba_bar = BAR_0;
+		nvme_epf->msix_layout.pba_offset = reg_size + msix_table_size;
+		nvme_epf->msix_layout.pba_size = pba_size;
 
 		reg_size += msix_table_size + pba_size;
 	}
@@ -2245,8 +2251,8 @@ static int nvmet_pci_epf_init_irq(struct nvmet_pci_epf *nvme_epf)
 	/* Enable MSI-X if supported, otherwise, use MSI. */
 	if (epc_features->msix_capable && epf->msix_interrupts) {
 		ret = pci_epc_set_msix(epf->epc, epf->func_no, epf->vfunc_no,
-				       epf->msix_interrupts, BAR_0,
-				       nvme_epf->msix_table_offset);
+				       epf->msix_interrupts,
+				       &nvme_epf->msix_layout);
 		if (ret) {
 			dev_err(&epf->dev, "Failed to configure MSI-X\n");
 			return ret;
diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index c0e1194a936b..2b69ea88aed2 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -293,7 +293,8 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 }
 
 static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
-				 u16 nr_irqs, enum pci_barno bir, u32 offset)
+				 u16 nr_irqs,
+				 const struct pci_epc_msix_layout *layout)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
@@ -311,12 +312,12 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
 
 	/* Set MSI-X BAR and offset */
 	reg = cap + PCI_MSIX_TABLE;
-	val = offset | bir;
+	val = layout->table_offset | layout->table_bar;
 	cdns_pcie_ep_fn_writel(pcie, fn, reg, val);
 
-	/* Set PBA BAR and offset.  BAR must match MSI-X BAR */
+	/* Set PBA BAR and offset */
 	reg = cap + PCI_MSIX_PBA;
-	val = (offset + (nr_irqs * PCI_MSIX_ENTRY_SIZE)) | bir;
+	val = layout->pba_offset | layout->pba_bar;
 	cdns_pcie_ep_fn_writel(pcie, fn, reg, val);
 
 	return 0;
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 7d2794945704..147b043589f0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -737,7 +737,8 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 }
 
 static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
-			       u16 nr_irqs, enum pci_barno bir, u32 offset)
+			       u16 nr_irqs,
+			       const struct pci_epc_msix_layout *layout)
 {
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -757,11 +758,11 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	dw_pcie_ep_writew_dbi(ep, func_no, reg, val);
 
 	reg = ep_func->msix_cap + PCI_MSIX_TABLE;
-	val = offset | bir;
+	val = layout->table_offset | layout->table_bar;
 	dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
 
 	reg = ep_func->msix_cap + PCI_MSIX_PBA;
-	val = (offset + (nr_irqs * PCI_MSIX_ENTRY_SIZE)) | bir;
+	val = layout->pba_offset | layout->pba_bar;
 	dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
 
 	dw_pcie_dbi_ro_wr_dis(pci);
diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c
index 5314aca2188a..f3e0e1b3ffb9 100644
--- a/drivers/pci/endpoint/functions/pci-epf-ntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c
@@ -90,12 +90,11 @@ struct epf_ntb_epc {
 	u8 vfunc_no;
 	bool linkup;
 	bool is_msix;
-	int msix_bar;
 	u32 spad_size;
 	struct pci_epc *epc;
 	struct epf_ntb *epf_ntb;
 	void __iomem *mw_addr[6];
-	size_t msix_table_offset;
+	struct pci_epc_msix_layout msix_layout;
 	struct epf_ntb_ctrl *reg;
 	struct pci_epf_bar *epf_bar;
 	enum pci_barno epf_ntb_bar[6];
@@ -475,9 +474,9 @@ static int epf_ntb_configure_msi(struct epf_ntb *ntb,
  *
  * The MSI-X address is in the MSI-X table of EP CONTROLLER 2 and
  * the count of doorbell is in ctrl->argument of epf_ntb_epc that is connected
- * to HOST2. MSI-X table is stored memory mapped to ntb_epc->msix_bar and the
- * offset is in ntb_epc->msix_table_offset. From this epf_ntb_configure_msix()
- * gets the MSI-X address and data.
+ * to HOST2. The location of the memory-mapped MSI-X table is described by
+ * ntb_epc->msix_layout. From this epf_ntb_configure_msix() gets the MSI-X
+ * address and data.
  *
  * epf_ntb_configure_msix() also stores the MSI-X data to raise each interrupt
  * in db_data of the peer's control region. This helps the peer to raise
@@ -505,8 +504,8 @@ static int epf_ntb_configure_msix(struct epf_ntb *ntb,
 	ntb_epc = ntb->epc[type];
 	epc = ntb_epc->epc;
 
-	epf_bar = &ntb_epc->epf_bar[ntb_epc->msix_bar];
-	msix_tbl = epf_bar->addr + ntb_epc->msix_table_offset;
+	epf_bar = &ntb_epc->epf_bar[ntb_epc->msix_layout.table_bar];
+	msix_tbl = epf_bar->addr + ntb_epc->msix_layout.table_offset;
 
 	peer_ntb_epc = ntb->epc[!type];
 	peer_barno = peer_ntb_epc->epf_ntb_bar[BAR_DB_MW1];
@@ -1036,10 +1035,14 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb,
 	if (msix_capable) {
 		msix_table_size = PCI_MSIX_ENTRY_SIZE * ntb->db_count;
 		ctrl_size = ALIGN(ctrl_size, 8);
-		ntb_epc->msix_table_offset = ctrl_size;
-		ntb_epc->msix_bar = barno;
-		/* Align to QWORD or 8 Bytes */
-		pba_size = ALIGN(DIV_ROUND_UP(ntb->db_count, 8), 8);
+		pba_size = BITS_TO_U64(ntb->db_count) * sizeof(u64);
+
+		ntb_epc->msix_layout.table_bar = barno;
+		ntb_epc->msix_layout.table_offset = ctrl_size;
+		ntb_epc->msix_layout.table_size = msix_table_size;
+		ntb_epc->msix_layout.pba_bar = barno;
+		ntb_epc->msix_layout.pba_offset = ctrl_size + msix_table_size;
+		ntb_epc->msix_layout.pba_size = pba_size;
 		ctrl_size = ctrl_size + msix_table_size + pba_size;
 	}
 
@@ -1317,10 +1320,9 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb,
 
 	if (msix_capable) {
 		ret = pci_epc_set_msix(epc, func_no, vfunc_no, ntb->db_count,
-				       ntb_epc->msix_bar,
-				       ntb_epc->msix_table_offset);
+				       &ntb_epc->msix_layout);
 		if (ret) {
-			dev_err(dev, "MSI configuration failed\n");
+			dev_err(dev, "MSI-X configuration failed\n");
 			return ret;
 		}
 	}
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 4802d4f80f78..e48292e0fb37 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -82,7 +82,7 @@ struct pci_epf_test {
 	struct pci_epf		*epf;
 	struct config_group	group;
 	enum pci_barno		test_reg_bar;
-	size_t			msix_table_offset;
+	struct pci_epc_msix_layout msix_layout;
 	struct delayed_work	cmd_handler;
 	struct dma_chan		*dma_chan_tx;
 	struct dma_chan		*dma_chan_rx;
@@ -1221,8 +1221,7 @@ static int pci_epf_test_epc_init(struct pci_epf *epf)
 	if (epc_features->msix_capable) {
 		ret = pci_epc_set_msix(epc, epf->func_no, epf->vfunc_no,
 				       epf->msix_interrupts,
-				       epf_test->test_reg_bar,
-				       epf_test->msix_table_offset);
+				       &epf_test->msix_layout);
 		if (ret) {
 			dev_err(dev, "MSI-X configuration failed\n");
 			return ret;
@@ -1288,9 +1287,15 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 
 	if (epc_features->msix_capable) {
 		msix_table_size = PCI_MSIX_ENTRY_SIZE * epf->msix_interrupts;
-		epf_test->msix_table_offset = test_reg_bar_size;
-		/* Align to QWORD or 8 Bytes */
-		pba_size = ALIGN(DIV_ROUND_UP(epf->msix_interrupts, 8), 8);
+		pba_size = BITS_TO_U64(epf->msix_interrupts) * sizeof(u64);
+
+		epf_test->msix_layout.table_bar = test_reg_bar;
+		epf_test->msix_layout.table_offset = test_reg_bar_size;
+		epf_test->msix_layout.table_size = msix_table_size;
+		epf_test->msix_layout.pba_bar = test_reg_bar;
+		epf_test->msix_layout.pba_offset = test_reg_bar_size +
+						   msix_table_size;
+		epf_test->msix_layout.pba_size = pba_size;
 	}
 	test_reg_size = test_reg_bar_size + msix_table_size + pba_size;
 
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 831b40458dcd..4a0ceb62f38f 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -6,6 +6,7 @@
  * Author: Kishon Vijay Abraham I <kishon@ti.com>
  */
 
+#include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -113,6 +114,84 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
 }
 EXPORT_SYMBOL_GPL(pci_epc_get_next_free_bar);
 
+static int pci_epc_get_msix_region(const struct pci_epc_features *epc_features,
+				   enum pci_epc_bar_rsvd_region_type type,
+				   enum pci_barno *bar, u32 *offset,
+				   resource_size_t *size)
+{
+	const struct pci_epc_bar_rsvd_region *region;
+	const struct pci_epc_bar_desc *bar_desc;
+	bool found = false;
+	int i, j;
+
+	if (!epc_features)
+		return -ENOENT;
+
+	for (i = BAR_0; i < PCI_STD_NUM_BARS; i++) {
+		bar_desc = &epc_features->bar[i];
+		if (bar_desc->nr_rsvd_regions && !bar_desc->rsvd_regions)
+			return -EINVAL;
+
+		for (j = 0; j < bar_desc->nr_rsvd_regions; j++) {
+			region = &bar_desc->rsvd_regions[j];
+			if (region->type != type)
+				continue;
+
+			if (found || bar_desc->type != BAR_RESERVED || !region->size ||
+			    region->offset > PCI_MSIX_TABLE_OFFSET ||
+			    !IS_ALIGNED(region->offset, 8))
+				return -EINVAL;
+
+			found = true;
+			*bar = i;
+			*offset = region->offset;
+			*size = region->size;
+		}
+	}
+
+	return found ? 0 : -ENOENT;
+}
+
+/**
+ * pci_epc_get_hw_msix_layout() - get a hardware-owned MSI-X table and PBA layout
+ * @epc_features: features provided by an EPC for an endpoint function
+ * @layout: layout to populate
+ *
+ * Return: 0 if the EPC describes both hardware-owned MSI-X regions, -ENOENT if
+ * neither region is described, or an error if the description is invalid.
+ */
+int pci_epc_get_hw_msix_layout(const struct pci_epc_features *epc_features,
+			       struct pci_epc_msix_layout *layout)
+{
+	struct pci_epc_msix_layout hw_layout;
+	int table_ret, pba_ret;
+
+	if (!layout)
+		return -EINVAL;
+
+	table_ret = pci_epc_get_msix_region(epc_features,
+					    PCI_EPC_BAR_RSVD_MSIX_TBL_RAM,
+					    &hw_layout.table_bar,
+					    &hw_layout.table_offset,
+					    &hw_layout.table_size);
+	pba_ret = pci_epc_get_msix_region(epc_features,
+					  PCI_EPC_BAR_RSVD_MSIX_PBA_RAM,
+					  &hw_layout.pba_bar,
+					  &hw_layout.pba_offset,
+					  &hw_layout.pba_size);
+
+	if (table_ret == -ENOENT && pba_ret == -ENOENT)
+		return -ENOENT;
+
+	if (table_ret || pba_ret)
+		return -EINVAL;
+
+	*layout = hw_layout;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epc_get_hw_msix_layout);
+
 static bool pci_epc_function_is_valid(struct pci_epc *epc,
 				      u8 func_no, u8 vfunc_no)
 {
@@ -443,14 +522,14 @@ EXPORT_SYMBOL_GPL(pci_epc_get_msix);
  * @func_no: the physical endpoint function number in the EPC device
  * @vfunc_no: the virtual endpoint function number in the physical function
  * @nr_irqs: number of MSI-X interrupts required by the EPF
- * @bir: BAR where the MSI-X table resides
- * @offset: Offset pointing to the start of MSI-X table
+ * @layout: MSI-X table and PBA layout selected by the EPF
  *
  * Invoke to set the required number of MSI-X interrupts.
  */
 int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u16 nr_irqs,
-		     enum pci_barno bir, u32 offset)
+		     const struct pci_epc_msix_layout *layout)
 {
+	size_t table_size, pba_size;
 	int ret;
 
 	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
@@ -459,11 +538,26 @@ int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u16 nr_irqs,
 	if (nr_irqs < 1 || nr_irqs > 2048)
 		return -EINVAL;
 
+	if (!layout || layout->table_bar < BAR_0 ||
+	    layout->table_bar >= PCI_STD_NUM_BARS ||
+	    layout->pba_bar < BAR_0 || layout->pba_bar >= PCI_STD_NUM_BARS ||
+	    !IS_ALIGNED(layout->table_offset, 8) ||
+	    !IS_ALIGNED(layout->pba_offset, 8) ||
+	    layout->table_offset > PCI_MSIX_TABLE_OFFSET ||
+	    layout->pba_offset > PCI_MSIX_PBA_OFFSET)
+		return -EINVAL;
+
+	table_size = nr_irqs * PCI_MSIX_ENTRY_SIZE;
+	pba_size = BITS_TO_U64(nr_irqs) * sizeof(u64);
+
+	if (layout->table_size < table_size || layout->pba_size < pba_size)
+		return -ENOSPC;
+
 	if (!epc->ops->set_msix)
 		return 0;
 
 	mutex_lock(&epc->lock);
-	ret = epc->ops->set_msix(epc, func_no, vfunc_no, nr_irqs, bir, offset);
+	ret = epc->ops->set_msix(epc, func_no, vfunc_no, nr_irqs, layout);
 	mutex_unlock(&epc->lock);
 
 	return ret;
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index f247cf9bcf1a..3d28231f092a 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -102,6 +102,24 @@ struct pci_epc_aux_resource {
 	} u;
 };
 
+/**
+ * struct pci_epc_msix_layout - layout of an MSI-X table and PBA
+ * @table_bar: BAR containing the MSI-X table
+ * @table_offset: offset of the MSI-X table within @table_bar
+ * @table_size: size of the MSI-X table region
+ * @pba_bar: BAR containing the MSI-X Pending Bit Array (PBA)
+ * @pba_offset: offset of the MSI-X PBA within @pba_bar
+ * @pba_size: size of the MSI-X PBA region
+ */
+struct pci_epc_msix_layout {
+	enum pci_barno		table_bar;
+	u32			table_offset;
+	resource_size_t		table_size;
+	enum pci_barno		pba_bar;
+	u32			pba_offset;
+	resource_size_t		pba_size;
+};
+
 /**
  * struct pci_epc_ops - set of function pointers for performing EPC operations
  * @write_header: ops to populate configuration space header
@@ -147,7 +165,8 @@ struct pci_epc_ops {
 			   u8 nr_irqs);
 	int	(*get_msi)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
 	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
-			    u16 nr_irqs, enum pci_barno, u32 offset);
+			    u16 nr_irqs,
+			    const struct pci_epc_msix_layout *layout);
 	int	(*get_msix)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
 	int	(*raise_irq)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 			     unsigned int type, u16 interrupt_num);
@@ -381,8 +400,10 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u8 nr_irqs);
 int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
 int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u16 nr_irqs,
-		     enum pci_barno, u32 offset);
+		     const struct pci_epc_msix_layout *layout);
 int pci_epc_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
+int pci_epc_get_hw_msix_layout(const struct pci_epc_features *epc_features,
+			       struct pci_epc_msix_layout *layout);
 int pci_epc_map_msi_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 			phys_addr_t phys_addr, u8 interrupt_num,
 			u32 entry_size, u32 *msi_data, u32 *msi_addr_offset);
-- 
2.51.0

[PATCH v2 2/3] PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588

[Koichiro Den]

The RK3588 PCIe endpoint controller exposes its MSI-X table at
BAR4+0x4000 and its PBA at BAR4+0x5000. Describe both as reserved regions
so EPF drivers can select the hardware-owned layout.

The regular DesignWare MSI-X interrupt path reads an EPF-owned table
through epf_bar[]. A reserved, hardware-owned table has no such backing.
Record whether the layout selected for each function matches the
hardware-owned layout, and use the controller MSI-X doorbell on RK3588
only in that case. Continue to use the regular path for EPF-owned
layouts.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - Return an error if the regular MSI-X helper is called for a
    hardware-owned layout. (Sashiko)

 .../pci/controller/dwc/pcie-designware-ep.c   | 23 +++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h  |  1 +
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 28 +++++++++++++++++--
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 147b043589f0..96726afec564 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -736,6 +736,26 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 	return val + 1;
 }
 
+static bool
+dw_pcie_ep_msix_layout_is_hw_owned(struct dw_pcie_ep *ep,
+				   const struct pci_epc_msix_layout *layout)
+{
+	const struct pci_epc_features *features;
+	struct pci_epc_msix_layout hw_layout;
+
+	if (!ep->ops->get_features)
+		return false;
+
+	features = ep->ops->get_features(ep);
+	if (pci_epc_get_hw_msix_layout(features, &hw_layout))
+		return false;
+
+	return layout->table_bar == hw_layout.table_bar &&
+	       layout->table_offset == hw_layout.table_offset &&
+	       layout->pba_bar == hw_layout.pba_bar &&
+	       layout->pba_offset == hw_layout.pba_offset;
+}
+
 static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 			       u16 nr_irqs,
 			       const struct pci_epc_msix_layout *layout)
@@ -764,6 +784,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	reg = ep_func->msix_cap + PCI_MSIX_PBA;
 	val = layout->pba_offset | layout->pba_bar;
 	dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
+	ep_func->msix_hw_owned = dw_pcie_ep_msix_layout_is_hw_owned(ep, layout);
 
 	dw_pcie_dbi_ro_wr_dis(pci);
 
@@ -1109,6 +1130,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
 	ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
 	if (!ep_func || !ep_func->msix_cap)
 		return -EINVAL;
+	if (ep_func->msix_hw_owned)
+		return -EOPNOTSUPP;
 
 	reg = ep_func->msix_cap + PCI_MSIX_TABLE;
 	tbl_offset = dw_pcie_ep_readl_dbi(ep, func_no, reg);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index de4b245b1758..043c39b5881d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -495,6 +495,7 @@ struct dw_pcie_ep_func {
 	u8			func_no;
 	u8			msi_cap;	/* MSI capability offset */
 	u8			msix_cap;	/* MSI-X capability offset */
+	bool			msix_hw_owned;
 	u8			bar_to_atu[PCI_STD_NUM_BARS];
 	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
 
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 731d93663cca..d622723dfcf8 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -120,6 +120,7 @@ struct rockchip_pcie {
 struct rockchip_pcie_of_data {
 	enum dw_pcie_device_mode mode;
 	const struct pci_epc_features *epc_features;
+	bool msix_doorbell;
 };
 
 static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, u32 reg)
@@ -481,6 +482,8 @@ static int rockchip_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 				   unsigned int type, u16 interrupt_num)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+	struct dw_pcie_ep_func *ep_func;
 
 	switch (type) {
 	case PCI_IRQ_INTX:
@@ -488,6 +491,12 @@ static int rockchip_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 	case PCI_IRQ_MSI:
 		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
 	case PCI_IRQ_MSIX:
+		ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
+		if (rockchip->data->msix_doorbell && ep_func &&
+		    ep_func->msix_hw_owned)
+			return dw_pcie_ep_raise_msix_irq_doorbell(ep, func_no,
+							      interrupt_num);
+
 		return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
 	default:
 		dev_err(pci->dev, "UNKNOWN IRQ type\n");
@@ -517,12 +526,24 @@ static const struct pci_epc_bar_rsvd_region rk3588_bar4_rsvd[] = {
 		.offset = 0x0,
 		.size = 0x2000,
 	},
+	{
+		/* MSI-X Table (BAR4: MSI-X Table) */
+		.type = PCI_EPC_BAR_RSVD_MSIX_TBL_RAM,
+		.offset = 0x4000,
+		.size = SZ_4K,
+	},
+	{
+		/* MSI-X PBA (BAR4: MSI-X PBA) */
+		.type = PCI_EPC_BAR_RSVD_MSIX_PBA_RAM,
+		.offset = 0x5000,
+		.size = SZ_4K,
+	},
 };
 
 /*
- * BAR4 on rk3588 exposes the ATU Port Logic Structure to the host regardless of
- * iATU settings for BAR4. This means that BAR4 cannot be used by an EPF driver,
- * so mark it as RESERVED.
+ * BAR4 on RK3588 exposes the DMA and ATU Port Logic Structures and the MSI-X
+ * table and PBA to the host regardless of iATU settings for BAR4. This means
+ * that BAR4 cannot be used by an EPF driver, so mark it as RESERVED.
  */
 static const struct pci_epc_features rockchip_pcie_epc_features_rk3588 = {
 	DWC_EPC_COMMON_FEATURES,
@@ -841,6 +862,7 @@ static const struct rockchip_pcie_of_data rockchip_pcie_ep_of_data_rk3568 = {
 static const struct rockchip_pcie_of_data rockchip_pcie_ep_of_data_rk3588 = {
 	.mode = DW_PCIE_EP_TYPE,
 	.epc_features = &rockchip_pcie_epc_features_rk3588,
+	.msix_doorbell = true,
 };
 
 static const struct of_device_id rockchip_pcie_of_match[] = {
-- 
2.51.0

[PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Honor MSI-X selection

[Koichiro Den]

ntb_hw_epf tries MSI-X first and falls back to MSI. It reports the
result in COMMAND_CONFIGURE_DOORBELL. pci-epf-vntb ignores MSIX_ENABLE,
configures only MSI, and always raises peer doorbells with PCI_IRQ_MSI.
When MSI-X is selected, the host does not program MSI, so raising it can
issue a write to an invalid address. This was observed with an IOMMU
enabled on the RC.

Configure MSI-X when supported and use the selected type for peer
doorbells. Use the hardware-owned layout when available. Otherwise
allocate an EPF-owned Table and PBA in the config BAR. Configure db_count
entries to cover the link event, the reserved slot, and the doorbell
slots.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 67 ++++++++++++++-----
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index c3caec927d74..d9622a5d4710 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -148,6 +148,7 @@ struct epf_ntb {
 	u16 vntb_vid;
 
 	bool linkup;
+	bool peer_msix;
 
 	/*
 	 * True when doorbells are interrupt-driven (MSI or embedded), false
@@ -155,6 +156,7 @@ struct epf_ntb {
 	 */
 	bool msi_doorbell;
 	u32 spad_size;
+	struct pci_epc_msix_layout msix_layout;
 
 	enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
 
@@ -303,6 +305,7 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 
 	switch (command) {
 	case COMMAND_CONFIGURE_DOORBELL:
+		WRITE_ONCE(ntb->peer_msix, argument & MSIX_ENABLE);
 		ctrl->command_status = COMMAND_STATUS_OK;
 		break;
 	case COMMAND_TEARDOWN_DOORBELL:
@@ -439,9 +442,9 @@ static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
  *   region
  * @ntb: NTB device that facilitates communication between HOST and VHOST
  *
- * Allocate the Local Memory mentioned in the above diagram. The size of
- * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD REGION
- * is obtained from "spad-count" configfs entry.
+ * Allocate the control and scratchpad regions described in the above diagram.
+ * If the EPC does not provide a hardware-owned MSI-X table and PBA, allocate
+ * space for them between the control and scratchpad regions.
  *
  * Returns: Zero for success, or an error code in case of failure
  */
@@ -454,7 +457,7 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
 	struct device *dev = &epf->dev;
 	u32 spad_count;
 	void *base;
-	int i;
+	int i, ret;
 	const struct pci_epc_features *epc_features = pci_epc_get_features(epf->epc,
 								epf->func_no,
 								epf->vfunc_no);
@@ -462,6 +465,29 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
 	spad_count = ntb->spad_count;
 
 	ctrl_size = ALIGN(sizeof(struct epf_ntb_ctrl), sizeof(u32));
+	if (epc_features->msix_capable) {
+		ret = pci_epc_get_hw_msix_layout(epc_features,
+						 &ntb->msix_layout);
+		if (ret && ret != -ENOENT) {
+			dev_err(dev, "Invalid hardware-owned MSI-X layout\n");
+			return ret;
+		}
+
+		if (ret == -ENOENT) {
+			ntb->msix_layout.table_bar = barno;
+			ntb->msix_layout.table_offset = ALIGN(ctrl_size, 8);
+			ntb->msix_layout.table_size =
+				ntb->db_count * PCI_MSIX_ENTRY_SIZE;
+			ntb->msix_layout.pba_bar = barno;
+			ntb->msix_layout.pba_offset =
+				ntb->msix_layout.table_offset +
+				ntb->msix_layout.table_size;
+			ntb->msix_layout.pba_size =
+				BITS_TO_U64(ntb->db_count) * sizeof(u64);
+			ctrl_size = ntb->msix_layout.pba_offset +
+				    ntb->msix_layout.pba_size;
+		}
+	}
 	spad_size = 2 * spad_count * sizeof(u32);
 
 	base = pci_epf_alloc_space(epf, ctrl_size + spad_size,
@@ -502,6 +528,7 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
 static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
 {
 	const struct pci_epc_features *epc_features;
+	struct pci_epf *epf = ntb->epf;
 	struct device *dev;
 	int ret;
 
@@ -521,16 +548,22 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
 	}
 
 	if (epc_features->msi_capable) {
-		ret = pci_epc_set_msi(ntb->epf->epc,
-				      ntb->epf->func_no,
-				      ntb->epf->vfunc_no,
-				      16);
+		ret = pci_epc_set_msi(epf->epc, epf->func_no, epf->vfunc_no, 16);
 		if (ret) {
 			dev_err(dev, "MSI configuration failed\n");
 			return ret;
 		}
 	}
 
+	if (epc_features->msix_capable) {
+		ret = pci_epc_set_msix(epf->epc, epf->func_no, epf->vfunc_no,
+				       ntb->db_count, &ntb->msix_layout);
+		if (ret) {
+			dev_err(dev, "MSI-X configuration failed\n");
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
@@ -1512,6 +1545,7 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
 	struct epf_ntb *ntb = container_of(work, struct epf_ntb, peer_db_work);
 	struct pci_epf *epf = ntb->epf;
 	unsigned int budget = VNTB_PEER_DB_WORK_BUDGET;
+	unsigned int irq_type;
 	u8 func_no, vfunc_no;
 	unsigned int db_bit;
 	u32 interrupt_num;
@@ -1523,6 +1557,7 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
 
 	func_no = epf->func_no;
 	vfunc_no = epf->vfunc_no;
+	irq_type = READ_ONCE(ntb->peer_msix) ? PCI_IRQ_MSIX : PCI_IRQ_MSI;
 
 	/*
 	 * Drain doorbells from peer_db_pending in snapshots (atomic64_xchg()).
@@ -1536,16 +1571,16 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
 
 		while (db_bits) {
 			/*
-			 * pci_epc_raise_irq() for MSI expects a 1-based
-			 * interrupt number. The first usable doorbell starts
-			 * at EPF_IRQ_DB_START in the legacy slot layout.
+			 * pci_epc_raise_irq() expects a 1-based interrupt
+			 * number for MSI and MSI-X. The first usable doorbell
+			 * starts at EPF_IRQ_DB_START in the legacy slot layout.
 			 *
 			 * Legacy mapping (kept for compatibility):
 			 *
-			 *   MSI #1 : link event (reserved)
-			 *   MSI #2 : unused (historical offset)
-			 *   MSI #3 : doorbell bit 0 (DB#0)
-			 *   MSI #4 : doorbell bit 1 (DB#1)
+			 *   IRQ #1 : link event (reserved)
+			 *   IRQ #2 : unused (historical offset)
+			 *   IRQ #3 : doorbell bit 0 (DB#0)
+			 *   IRQ #4 : doorbell bit 1 (DB#1)
 			 *   ...
 			 *
 			 * Do not change this mapping to avoid breaking
@@ -1556,7 +1591,7 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
 			db_bits &= ~BIT_ULL(db_bit);
 
 			ret = pci_epc_raise_irq(epf->epc, func_no, vfunc_no,
-						PCI_IRQ_MSI, interrupt_num);
+						irq_type, interrupt_num);
 			if (ret)
 				dev_err(&ntb->ntb.dev,
 					"Failed to raise IRQ for interrupt_num %u: %d\n",
-- 
2.51.0

Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Niklas Cassel]

Hello Koichiro,

On Mon, Aug 31, 2026 at 12:19:45AM +0900, Koichiro Den wrote:
> Hi,
> 
> Some DWC endpoint controllers keep the MSI-X Table and PBA at fixed
> locations in a reserved BAR. On RK3588, the MSI-X doorbell uses this
> hardware-owned layout, as discussed in e.g. [1].
> 
> The reserved-region types for the Table and PBA were added with the
> Tegra194 description. The MSI-X setup API, however, still takes only the
> Table BAR and offset and assumes that the PBA immediately follows it in
> the same BAR.
> 
> This series passes the complete layout to pci_epc_set_msix() and adds
> pci_epc_get_hw_msix_layout() to expose a hardware-owned layout. The core
> does not select it automatically. The choice stays with the EPF.
> 
> A hardware-owned layout is not a direct replacement for every caller.
> pci-epf-ntb, for example, reads host-programmed Table entries from its
> own BAR to set up peer outbound mappings.
> 
> The series also fixes an interrupt-type mismatch in vNTB. ntb_hw_epf can
> select MSI-X, while pci-epf-vntb currently configures and raises only
> MSI. On RK3588, selecting the fixed BAR4 layout also selects the DWC
> MSI-X doorbell. EPF-owned layouts continue to use the regular path.
> 
> In short:
> 
> - pci-epf-vntb gains MSI-X support and uses the hardware-owned layout
>   when available.
> - pci-epf-test, pci-epf-ntb, and the NVMe PCI EPF keep their EPF-owned
>   layouts. This avoids unnecessary changes and reduces regression risk.
>   They can use a hardware-owned layout later if/when needed.
> 
> [1] https://lore.kernel.org/r/aY2q80zeRKSRO21H@fedora


Perhaps you could improve the cover letter to more clearly state why you
are doing this change.

Some guesses:
- Better performance. We avoid the need to map + unmap the MSI target
  address using an iATU each time we raise an MSI-X. We also avoid the
  need to flush posted write before unmap.
  Is there any performance difference? If so, it would be nice with some
  numbers.
- Allows more concurrent I/Os. By not using an iATU when raising an MSI-X,
  we have one more iATU available, so we can have one more outstanding I/O.
- Less waste of BAR space. (Since the MSI-X table and PBA already
  always takes up space in one of the BARs, it is wasteful to have
  the EPF drive duplicate it in another BAR.)



Personally, I don't see why we should only change pci-epf-vntb to use
the hardware-owned layout when available. I don't see why we would not
want to change pci-epf-test, pci-epf-ntb, and nvmet-pci-epf as well.
(If the EPC defines a HW defined MSI-X table + PBA, why not always
use that? If there is no HW defined MSI-X table + PBA, let the EPF
put the MSI-X table in any BAR it likes.)


I understand that you introduce dw_pcie_ep_msix_layout_is_hw_owned()
because you want an EPF driver optionally use the HW defined table.
But if all EPF drivers always use the HW defined table if available, I think
you can avoid introducing this helper, and let rockchip_pcie_raise_irq()
unconditionally call dw_pcie_ep_raise_msix_irq_doorbell() for case PCI_IRQ_MSIX.

See e.g. drivers/pci/controller/dwc/pci-layerscape-ep.c which already calls
dw_pcie_ep_raise_msix_irq_doorbell() unconditionally for case PCI_IRQ_MSIX.


Kind regards,
Niklas

Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Koichiro Den]

On Mon, Aug 31, 2026 at 12:24:55PM +0200, Niklas Cassel wrote:
> Hello Koichiro,
> 
> On Mon, Aug 31, 2026 at 12:19:45AM +0900, Koichiro Den wrote:
> > Hi,
> > 
> > Some DWC endpoint controllers keep the MSI-X Table and PBA at fixed
> > locations in a reserved BAR. On RK3588, the MSI-X doorbell uses this
> > hardware-owned layout, as discussed in e.g. [1].
> > 
> > The reserved-region types for the Table and PBA were added with the
> > Tegra194 description. The MSI-X setup API, however, still takes only the
> > Table BAR and offset and assumes that the PBA immediately follows it in
> > the same BAR.
> > 
> > This series passes the complete layout to pci_epc_set_msix() and adds
> > pci_epc_get_hw_msix_layout() to expose a hardware-owned layout. The core
> > does not select it automatically. The choice stays with the EPF.
> > 
> > A hardware-owned layout is not a direct replacement for every caller.
> > pci-epf-ntb, for example, reads host-programmed Table entries from its
> > own BAR to set up peer outbound mappings.
> > 
> > The series also fixes an interrupt-type mismatch in vNTB. ntb_hw_epf can
> > select MSI-X, while pci-epf-vntb currently configures and raises only
> > MSI. On RK3588, selecting the fixed BAR4 layout also selects the DWC
> > MSI-X doorbell. EPF-owned layouts continue to use the regular path.
> > 
> > In short:
> > 
> > - pci-epf-vntb gains MSI-X support and uses the hardware-owned layout
> >   when available.
> > - pci-epf-test, pci-epf-ntb, and the NVMe PCI EPF keep their EPF-owned
> >   layouts. This avoids unnecessary changes and reduces regression risk.
> >   They can use a hardware-owned layout later if/when needed.
> > 
> > [1] https://lore.kernel.org/r/aY2q80zeRKSRO21H@fedora
> 

Hello Niklas,

> 
> Perhaps you could improve the cover letter to more clearly state why you
> are doing this change.

Thanks, that's a fair point, the cover letter does not explain the
motivation clearly enough.

> 
> Some guesses:
> - Better performance. We avoid the need to map + unmap the MSI target
>   address using an iATU each time we raise an MSI-X. We also avoid the
>   need to flush posted write before unmap.
>   Is there any performance difference? If so, it would be nice with some
>   numbers.
> - Allows more concurrent I/Os. By not using an iATU when raising an MSI-X,
>   we have one more iATU available, so we can have one more outstanding I/O.
> - Less waste of BAR space. (Since the MSI-X table and PBA already
>   always takes up space in one of the BARs, it is wasteful to have
>   the EPF drive duplicate it in another BAR.)

This is not driven by a performance target, so I do not have numbers. The
immediate issue is correctness. ntb_hw_epf can select MSI-X, but pci-epf-vntb
currently configures and raises MSI. I want that MSI-X path to work on RK3588
with the controller doorbell. I will make it clear in the next cover letter
that, with this series, a doorbell sent from ntb_tool on the pci-epf-vntb side
of RK3588 reaches ntb_tool on the ntb_hw_epf host side when MSI-X is selected.
I believe that is the simplest E2E test showing what this series fixes.

For pci-epf-vntb this is a new MSI-X path, not a change to an existing working
path, so it seemed like a safe first user of the hardware-owned layout.
Honestly, adding a new MSI-X path to pci-epf-vntb while allocating another
Table/PBA in an EPF-owned BAR on RK3588 seemd rather odd to me.

So, avoiding the temporary iATU mapping and duplicate Table/PBA is good, but for
me performance was not the goal for my RK3588 case.

> 
> 
> 
> Personally, I don't see why we should only change pci-epf-vntb to use
> the hardware-owned layout when available. I don't see why we would not

I initially considered making the hardware-owned layout automatic for every EPF.
pci-epf-ntb (not vNTB!) is the reason I did not. It currently reads
host-programmed MSI-X entries through epf_bar[] backing and uses them to set up
peer outbound mappings. A hardware-owned Table has no such backing, and the
current EPC API has no generic way to read those entries. I suspect that
supporting it there would require a broader design change. Also, I do not have
hardware for the pci-epf-ntb bridge configuration.

pci-epf-test and nvmet-pci-epf should be easier to convert. However, converting
only those would not remove the per-layout handling while pci-epf-ntb still uses
an EPF-owned Table. I would prefer to handle those separately when they can be
properly tested. This is what I meant by this part of the cover letter:

     [...]

     layouts. This avoids unnecessary changes and reduces regression risk.
     They can use a hardware-owned layout later if/when needed.

> want to change pci-epf-test, pci-epf-ntb, and nvmet-pci-epf as well.
> (If the EPC defines a HW defined MSI-X table + PBA, why not always
> use that? If there is no HW defined MSI-X table + PBA, let the EPF
> put the MSI-X table in any BAR it likes.)
> 
> 
> I understand that you introduce dw_pcie_ep_msix_layout_is_hw_owned()
> because you want an EPF driver optionally use the HW defined table.
> But if all EPF drivers always use the HW defined table if available, I think
> you can avoid introducing this helper, and let rockchip_pcie_raise_irq()
> unconditionally call dw_pcie_ep_raise_msix_irq_doorbell() for case PCI_IRQ_MSIX.

Yes, under that assumption I agree. With the current mix, however, the Rockchip
callback still has to choose the helper according to the selected layout. That
is why the DWC code records which layout was selected.

I will update the cover letter to state the motivation and this boundary more
directly. Does keeping the layout choice with each EPF for this reason sound
reasonable?

Best regards,
Koichiro

> 
> See e.g. drivers/pci/controller/dwc/pci-layerscape-ep.c which already calls
> dw_pcie_ep_raise_msix_irq_doorbell() unconditionally for case PCI_IRQ_MSIX.
> 
> 
> Kind regards,
> Niklas

Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Niklas Cassel]

Hello Koichiro,

On Tue, Sep 01, 2026 at 01:11:09AM +0900, Koichiro Den wrote:
> This is not driven by a performance target, so I do not have numbers. The
> immediate issue is correctness. ntb_hw_epf can select MSI-X, but pci-epf-vntb
> currently configures and raises MSI. I want that MSI-X path to work on RK3588
> with the controller doorbell. I will make it clear in the next cover letter
> that, with this series, a doorbell sent from ntb_tool on the pci-epf-vntb side
> of RK3588 reaches ntb_tool on the ntb_hw_epf host side when MSI-X is selected.
> I believe that is the simplest E2E test showing what this series fixes.

Okay, if the main issue is that pci-epf-vntb does not raise MSI-X, even when
ntb_hw_epf host side has requested MSI-X, then I think that the first patch
in the series should fix this, so that the fix can easily be backported.

(Right now you fix this in patch 3/3, and the fix in patch 3/3 depends on
both patches 1/3 and 2/3.)

I think that pci-epf-vntb actually respecting the configured IRQ type by the
host is a separate logical change from adding support for HW defined layout.


> I initially considered making the hardware-owned layout automatic for every EPF.
> pci-epf-ntb (not vNTB!) is the reason I did not. It currently reads
> host-programmed MSI-X entries through epf_bar[] backing and uses them to set up
> peer outbound mappings. A hardware-owned Table has no such backing, and the
> current EPC API has no generic way to read those entries. I suspect that
> supporting it there would require a broader design change. Also, I do not have
> hardware for the pci-epf-ntb bridge configuration.
> 
> pci-epf-test and nvmet-pci-epf should be easier to convert. However, converting
> only those would not remove the per-layout handling while pci-epf-ntb still uses
> an EPF-owned Table. I would prefer to handle those separately when they can be
> properly tested. This is what I meant by this part of the cover letter:
> 
>      [...]
> 
>      layouts. This avoids unnecessary changes and reduces regression risk.
>      They can use a hardware-owned layout later if/when needed.

Okay, if pci-epf-ntb is special, perhaps we should add a comment in
pci-epf-ntb which explains why it cannot support HW defined layout.

But for consistency, if we modify pci-epf-vntb, I think it would make sense
to also modify pci-epf-test and nvmet-pci-epf as well in the same series.

pci-epf-test should be easy to test, just run the pci endpoint selftest.

nvmet-pci-epf should be quite easy to test too, just run the normal fio based
testing that we usually run, and unless you see worse performance, I think we
are good. (If we see worse performance, that could be an indication that one
of the MSI-X for the different completion queues are not being correctly
triggered. By default, the host side nvme driver creates one completion queue
(and one submission queue) per host CPU.


What I suggest that you instead do something like:
Patch 1/7: pci-epf-vntb: Fix to actually use MSI-X when requested.
           This probably includes adding the code that is inside the
	   if (ret == -ENOENT) { }.
Patch 2/7: PCI: endpoint: Support hardware-owned MSI-X table and PBA
Patch 3/7: PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588
Patch 4/7: pci-epf-vntb: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 5/7: pci-epf-test: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 6/7: nvmet-pci-epf: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 7/7: pci-epf-ntb: Add comment that explains why it cannot call
           pci_epc_get_hw_msix_layout() like all other EPF drivers that
	   support MSI-X.


This way, patch 1/7 can be backported, and will help all users vNTB users,
even the users that do not use RK3588 as an EPC.


> > I understand that you introduce dw_pcie_ep_msix_layout_is_hw_owned()
> > because you want an EPF driver optionally use the HW defined table.
> > But if all EPF drivers always use the HW defined table if available, I think
> > you can avoid introducing this helper, and let rockchip_pcie_raise_irq()
> > unconditionally call dw_pcie_ep_raise_msix_irq_doorbell() for case PCI_IRQ_MSIX.
> 
> Yes, under that assumption I agree. With the current mix, however, the Rockchip
> callback still has to choose the helper according to the selected layout. That
> is why the DWC code records which layout was selected.
> 
> I will update the cover letter to state the motivation and this boundary more
> directly. Does keeping the layout choice with each EPF for this reason sound
> reasonable?

Yes, since pci-epf-ntb apparently is special, I can see why you need
both ep_func->msix_hw_owned and dw_pcie_ep_msix_layout_is_hw_owned().


Kind regards,
Niklas

Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Koichiro Den]

On Mon, Aug 31, 2026 at 10:01:31PM +0200, Niklas Cassel wrote:
> Hello Koichiro,
> 
> On Tue, Sep 01, 2026 at 01:11:09AM +0900, Koichiro Den wrote:
> > This is not driven by a performance target, so I do not have numbers. The
> > immediate issue is correctness. ntb_hw_epf can select MSI-X, but pci-epf-vntb
> > currently configures and raises MSI. I want that MSI-X path to work on RK3588
> > with the controller doorbell. I will make it clear in the next cover letter
> > that, with this series, a doorbell sent from ntb_tool on the pci-epf-vntb side
> > of RK3588 reaches ntb_tool on the ntb_hw_epf host side when MSI-X is selected.
> > I believe that is the simplest E2E test showing what this series fixes.
> 
> Okay, if the main issue is that pci-epf-vntb does not raise MSI-X, even when
> ntb_hw_epf host side has requested MSI-X, then I think that the first patch
> in the series should fix this, so that the fix can easily be backported.
> 
> (Right now you fix this in patch 3/3, and the fix in patch 3/3 depends on
> both patches 1/3 and 2/3.)
> 
> I think that pci-epf-vntb actually respecting the configured IRQ type by the
> host is a separate logical change from adding support for HW defined layout.

Sure. It can be a separate patch, first making pci-epf-vntb use an EPF-owned
MSI-X Table/PBA like the other in-tree EPF drivers with MSI-X support.

> 
> 
> > I initially considered making the hardware-owned layout automatic for every EPF.
> > pci-epf-ntb (not vNTB!) is the reason I did not. It currently reads
> > host-programmed MSI-X entries through epf_bar[] backing and uses them to set up
> > peer outbound mappings. A hardware-owned Table has no such backing, and the
> > current EPC API has no generic way to read those entries. I suspect that
> > supporting it there would require a broader design change. Also, I do not have
> > hardware for the pci-epf-ntb bridge configuration.
> > 
> > pci-epf-test and nvmet-pci-epf should be easier to convert. However, converting
> > only those would not remove the per-layout handling while pci-epf-ntb still uses
> > an EPF-owned Table. I would prefer to handle those separately when they can be
> > properly tested. This is what I meant by this part of the cover letter:
> > 
> >      [...]
> > 
> >      layouts. This avoids unnecessary changes and reduces regression risk.
> >      They can use a hardware-owned layout later if/when needed.
> 
> Okay, if pci-epf-ntb is special, perhaps we should add a comment in
> pci-epf-ntb which explains why it cannot support HW defined layout.
> 
> But for consistency, if we modify pci-epf-vntb, I think it would make sense
> to also modify pci-epf-test and nvmet-pci-epf as well in the same series.
> 
> pci-epf-test should be easy to test, just run the pci endpoint selftest.
> 
> nvmet-pci-epf should be quite easy to test too, just run the normal fio based
> testing that we usually run, and unless you see worse performance, I think we
> are good. (If we see worse performance, that could be an indication that one
> of the MSI-X for the different completion queues are not being correctly
> triggered. By default, the host side nvme driver creates one completion queue
> (and one submission queue) per host CPU.

Fair enough. As an experiment, I converted nvmet-pci-epf and tries to test it. I
could not get as far as running fio because host-side NVMe initialization timed
out.

I found that on RK3588, host writes through BAR4 to the Message Address field
(the first 8 bytes) of entry #0 do not seem to show up in the internal iMSIX-TX
Table RAM. The rest of entry #0 and the entries from entry #1 onward that I
tested seem fine. The same two DWORDs can be written through direct DBI Table
RAM access, and the doorbell for entry #0 then works.

I'm not sure but one thing I wondered is whether this could be related to the
boundary between the BAR4-mapped unrolled iATU registers and the MSI-X Table. On
RK3588, the iATU register window starts at 0x2000 and the MSI-X Table
immediately follows at 0x4000. If there somehow is an address decoding issue at
that boundary, with the first part of the Table being taken as an iATU access, I
would like to know.

This still sounds really odd, and it does not obviously explain why both DWORDs
at 0x4000 and 0x4004 are affected. I may be missing something important. If
anything similar involving iMSIX-TX Table RAM entry #0 has been discussed
before, please point me to the relevant thread.

(Also, I can share my current WIP v3 series, which makes pci-epf-test and
nvmet-pci-epf use the hardware-owned layout when available, while leaving
pci-epf-ntb with its EPF-owned layout. If you have any setup where you could
test it, please let me know.)

> 
> 
> What I suggest that you instead do something like:
> Patch 1/7: pci-epf-vntb: Fix to actually use MSI-X when requested.
>            This probably includes adding the code that is inside the
> 	   if (ret == -ENOENT) { }.
> Patch 2/7: PCI: endpoint: Support hardware-owned MSI-X table and PBA
> Patch 3/7: PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588
> Patch 4/7: pci-epf-vntb: Add code that calls pci_epc_get_hw_msix_layout()
>            and uses that layout, if the function returned success.
> Patch 5/7: pci-epf-test: Add code that calls pci_epc_get_hw_msix_layout()
>            and uses that layout, if the function returned success.
> Patch 6/7: nvmet-pci-epf: Add code that calls pci_epc_get_hw_msix_layout()
>            and uses that layout, if the function returned success.
> Patch 7/7: pci-epf-ntb: Add comment that explains why it cannot call
>            pci_epc_get_hw_msix_layout() like all other EPF drivers that
> 	   support MSI-X.
> 
> 
> This way, patch 1/7 can be backported, and will help all users vNTB users,
> even the users that do not use RK3588 as an EPC.

That split makes sense. However, given the issue I described above, perhaps I
should send patch 1/7 as a standalone fix and hold off on the rest until the
issue is understood and resolved.

Testing vNTB alone (ie. this v2 way) did not expose it because its first data
doorbell uses MSI-X Table entry #2. I only found the issue because your
suggestion led me to convert and test nvmet-pci-epf, thank you!

> 
> 
> > > I understand that you introduce dw_pcie_ep_msix_layout_is_hw_owned()
> > > because you want an EPF driver optionally use the HW defined table.
> > > But if all EPF drivers always use the HW defined table if available, I think
> > > you can avoid introducing this helper, and let rockchip_pcie_raise_irq()
> > > unconditionally call dw_pcie_ep_raise_msix_irq_doorbell() for case PCI_IRQ_MSIX.
> > 
> > Yes, under that assumption I agree. With the current mix, however, the Rockchip
> > callback still has to choose the helper according to the selected layout. That
> > is why the DWC code records which layout was selected.
> > 
> > I will update the cover letter to state the motivation and this boundary more
> > directly. Does keeping the layout choice with each EPF for this reason sound
> > reasonable?
> 
> Yes, since pci-epf-ntb apparently is special, I can see why you need
> both ep_func->msix_hw_owned and dw_pcie_ep_msix_layout_is_hw_owned().

Sounds good.
Thanks for taking a look!

Best regards,
Koichiro

> 
> 
> Kind regards,
> Niklas

Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

[Niklas Cassel]

On Wed, Sep 02, 2026 at 11:14:00AM +0900, Koichiro Den wrote:
> 
> This still sounds really odd, and it does not obviously explain why both DWORDs
> at 0x4000 and 0x4004 are affected. I may be missing something important. If
> anything similar involving iMSIX-TX Table RAM entry #0 has been discussed
> before, please point me to the relevant thread.
> 
> (Also, I can share my current WIP v3 series, which makes pci-epf-test and
> nvmet-pci-epf use the hardware-owned layout when available, while leaving
> pci-epf-ntb with its EPF-owned layout. If you have any setup where you could
> test it, please let me know.)

Some time ago, I did verify that I could raise an IRQ using
dw_pcie_ep_raise_msix_irq_doorbell(). If I remember correctly, I did so using
pci-epf-test and not nvmet-pci-epf.

Did pci-epf-test work? (From what you explain, it seems to be related to if
MSI-X Table entry #0 is used or not.)

Please share your branch. I should have time to do a quick test.


> 
> > 
> > 
> > What I suggest that you instead do something like:
> > Patch 1/7: pci-epf-vntb: Fix to actually use MSI-X when requested.
> >            This probably includes adding the code that is inside the
> > 	   if (ret == -ENOENT) { }.
> > Patch 2/7: PCI: endpoint: Support hardware-owned MSI-X table and PBA
> > Patch 3/7: PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588
> > Patch 4/7: pci-epf-vntb: Add code that calls pci_epc_get_hw_msix_layout()
> >            and uses that layout, if the function returned success.
> > Patch 5/7: pci-epf-test: Add code that calls pci_epc_get_hw_msix_layout()
> >            and uses that layout, if the function returned success.
> > Patch 6/7: nvmet-pci-epf: Add code that calls pci_epc_get_hw_msix_layout()
> >            and uses that layout, if the function returned success.
> > Patch 7/7: pci-epf-ntb: Add comment that explains why it cannot call
> >            pci_epc_get_hw_msix_layout() like all other EPF drivers that
> > 	   support MSI-X.
> > 
> > 
> > This way, patch 1/7 can be backported, and will help all users vNTB users,
> > even the users that do not use RK3588 as an EPC.
> 
> That split makes sense. However, given the issue I described above, perhaps I
> should send patch 1/7 as a standalone fix and hold off on the rest until the
> issue is understood and resolved.

Agreed, a standalone 1/7 patch could be picked up without the need to wait for
any other changes.


> 
> Testing vNTB alone (ie. this v2 way) did not expose it because its first data
> doorbell uses MSI-X Table entry #2. I only found the issue because your
> suggestion led me to convert and test nvmet-pci-epf, thank you!

I'm sorry that we have a problem.
At least it is better to discover it now, rather than discovering it after
the patches had been merged.


Kind regards,
Niklas