* [PATCH 01/11] NTB: Add atomic MW translation group operations
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:10 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout Koichiro Den
` (10 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
A PCI function has only a few BARs, but an NTB device may need more
logical MWs, for example to give queues separate windows. Splitting a
large or resizable BAR avoids spending one BAR per MW.
BAR subrange mappings for that would consume one inbound mapping region
per logical MW. Those regions can be scarce on some PCIe controller
integrations, limiting the number of MWs.
Introduce translation groups whose members use contiguous local
backing, so one inbound mapping can cover the whole group. Add a group
query and atomic group set/clear operations. Singleton groups keep using
the existing per-MW callbacks.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
include/linux/ntb.h | 133 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 128 insertions(+), 5 deletions(-)
diff --git a/include/linux/ntb.h b/include/linux/ntb.h
index ae72caa03859..c21c6495d18f 100644
--- a/include/linux/ntb.h
+++ b/include/linux/ntb.h
@@ -217,6 +217,9 @@ static inline int ntb_ctx_ops_is_valid(const struct ntb_ctx_ops *ops)
* @link_disable: See ntb_link_disable().
* @mw_count: See ntb_mw_count().
* @mw_get_align: See ntb_mw_get_align().
+ * @mw_get_trans_group: See ntb_mw_get_trans_group().
+ * @mw_set_trans_group: See ntb_mw_set_trans_group().
+ * @mw_clear_trans_group: See ntb_mw_clear_trans_group().
* @mw_set_trans: See ntb_mw_set_trans().
* @mw_clear_trans: See ntb_mw_clear_trans().
* @peer_mw_count: See ntb_peer_mw_count().
@@ -275,6 +278,11 @@ struct ntb_dev_ops {
resource_size_t *addr_align,
resource_size_t *size_align,
resource_size_t *size_max);
+ int (*mw_get_trans_group)(struct ntb_dev *ntb, int pidx, int widx,
+ int *gidx, int *gcount);
+ int (*mw_set_trans_group)(struct ntb_dev *ntb, int pidx, int widx,
+ dma_addr_t addr, resource_size_t size);
+ int (*mw_clear_trans_group)(struct ntb_dev *ntb, int pidx, int widx);
int (*mw_set_trans)(struct ntb_dev *ntb, int pidx, int widx,
dma_addr_t addr, resource_size_t size);
int (*mw_clear_trans)(struct ntb_dev *ntb, int pidx, int widx);
@@ -352,6 +360,11 @@ static inline int ntb_dev_ops_is_valid(const struct ntb_dev_ops *ops)
ops->mw_get_align &&
(ops->mw_set_trans ||
ops->peer_mw_set_trans) &&
+ (!ops->mw_set_trans_group ||
+ ops->mw_get_trans_group) &&
+ /* Group set and clear operations must be provided together */
+ !ops->mw_set_trans_group ==
+ !ops->mw_clear_trans_group &&
/* ops->mw_clear_trans && */
ops->peer_mw_count &&
ops->peer_mw_get_addr &&
@@ -836,6 +849,102 @@ static inline int ntb_mw_get_align(struct ntb_dev *ntb, int pidx, int widx,
size_max);
}
+/**
+ * ntb_mw_get_trans_group() - get the translation group for an inbound MW
+ * @ntb: NTB device context.
+ * @pidx: Port index of peer device.
+ * @widx: Memory window index.
+ * @gidx: OUT - first memory window index in the group.
+ * @gcount: OUT - number of memory windows in the group.
+ *
+ * Return the first window index and number of consecutive windows in the
+ * group containing @widx. All translations in a group must be set and cleared
+ * together. If the device operation is not implemented, each window forms a
+ * singleton group.
+ *
+ * Group definitions for @pidx must not change while @ntb is registered. This
+ * function may be called regardless of the link state.
+ *
+ * NULL may be given for either output parameter.
+ *
+ * Return: Zero on success, otherwise a negative error number.
+ */
+static inline int ntb_mw_get_trans_group(struct ntb_dev *ntb, int pidx,
+ int widx, int *gidx, int *gcount)
+{
+ if (!ntb->ops->mw_get_trans_group) {
+ if (gidx)
+ *gidx = widx;
+ if (gcount)
+ *gcount = 1;
+
+ return 0;
+ }
+
+ return ntb->ops->mw_get_trans_group(ntb, pidx, widx, gidx, gcount);
+}
+
+static inline int ntb_mw_check_singleton(struct ntb_dev *ntb, int pidx,
+ int widx)
+{
+ int first, group_count;
+ int ret;
+
+ ret = ntb_mw_get_trans_group(ntb, pidx, widx, &first, &group_count);
+ if (ret)
+ return ret;
+
+ if (first < 0 || group_count <= 0 ||
+ widx < first || widx - first >= group_count)
+ return -EINVAL;
+
+ return group_count == 1 ? 0 : -EOPNOTSUPP;
+}
+
+/**
+ * ntb_mw_set_trans_group() - set all translations in an inbound MW group
+ * @ntb: NTB device context.
+ * @pidx: Port index of peer device.
+ * @widx: First memory window index in the group.
+ * @addr: DMA address of the local memory exposed to the peer.
+ * @size: Size of the local memory exposed to the peer.
+ *
+ * Set the translation for a multi-member group. @widx must be the first MW in
+ * the group. @addr and @size describe one contiguous backing range covering
+ * the complete group. Singleton groups use ntb_mw_set_trans().
+ *
+ * Return: Zero on success, otherwise a negative error number.
+ */
+static inline int
+ntb_mw_set_trans_group(struct ntb_dev *ntb, int pidx, int widx,
+ dma_addr_t addr, resource_size_t size)
+{
+ if (!ntb->ops->mw_set_trans_group)
+ return -EOPNOTSUPP;
+
+ return ntb->ops->mw_set_trans_group(ntb, pidx, widx, addr, size);
+}
+
+/**
+ * ntb_mw_clear_trans_group() - clear all translations in an inbound MW group
+ * @ntb: NTB device context.
+ * @pidx: Port index of peer device.
+ * @widx: First memory window index in the group.
+ *
+ * Clear the translation for a multi-member group. @widx must be the first MW
+ * in the group. Singleton groups use ntb_mw_clear_trans().
+ *
+ * Return: Zero on success, otherwise a negative error number.
+ */
+static inline int ntb_mw_clear_trans_group(struct ntb_dev *ntb, int pidx,
+ int widx)
+{
+ if (!ntb->ops->mw_clear_trans_group)
+ return -EOPNOTSUPP;
+
+ return ntb->ops->mw_clear_trans_group(ntb, pidx, widx);
+}
+
/**
* ntb_mw_set_trans() - set the translation of an inbound memory window
* @ntb: NTB device context.
@@ -851,13 +960,20 @@ static inline int ntb_mw_get_align(struct ntb_dev *ntb, int pidx, int widx,
* of that method.
*
* This method may not be implemented due to the hardware specific memory
- * windows interface.
+ * windows interface. A per-member request for a translation group with
+ * multiple members returns -EOPNOTSUPP.
*
* Return: Zero on success, otherwise an error number.
*/
static inline int ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
dma_addr_t addr, resource_size_t size)
{
+ int ret;
+
+ ret = ntb_mw_check_singleton(ntb, pidx, widx);
+ if (ret)
+ return ret;
+
if (!ntb->ops->mw_set_trans)
return 0;
@@ -872,16 +988,23 @@ static inline int ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
* @widx: Memory window index.
*
* Clear the translation of an inbound memory window. The peer may no longer
- * access local memory through the window.
+ * access local memory through the window. A per-member request for a
+ * translation group with multiple members returns -EOPNOTSUPP.
*
* Return: Zero on success, otherwise an error number.
*/
static inline int ntb_mw_clear_trans(struct ntb_dev *ntb, int pidx, int widx)
{
- if (!ntb->ops->mw_clear_trans)
- return ntb_mw_set_trans(ntb, pidx, widx, 0, 0);
+ int ret;
+
+ ret = ntb_mw_check_singleton(ntb, pidx, widx);
+ if (ret)
+ return ret;
+
+ if (ntb->ops->mw_clear_trans)
+ return ntb->ops->mw_clear_trans(ntb, pidx, widx);
- return ntb->ops->mw_clear_trans(ntb, pidx, widx);
+ return ntb_mw_set_trans(ntb, pidx, widx, 0, 0);
}
/**
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
2026-08-03 18:04 ` [PATCH 01/11] NTB: Add atomic MW translation group operations Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:23 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling Koichiro Den
` (9 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
The legacy control layout cannot describe multiple logical MWs sharing
one BAR. Treat a zero version as legacy; version 1 describes all MWs as
one packed group using a BAR number and aggregate size. Derive the
equal-sized logical MWs from the existing MW count.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/ntb/hw/epf/ntb_hw_epf.c | 102 ++++++++++++++++++++++++++++----
1 file changed, 90 insertions(+), 12 deletions(-)
diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
index c47607d4f8a7..46f37c2dea7b 100644
--- a/drivers/ntb/hw/epf/ntb_hw_epf.c
+++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
@@ -31,18 +31,22 @@
#define NTB_EPF_LINK_STATUS 0x0A
#define LINK_STATUS_UP BIT(0)
-#define NTB_EPF_TOPOLOGY 0x0C
+#define NTB_EPF_CTRL_VERSION 0x0C
+#define NTB_EPF_CTRL_V1 1
#define NTB_EPF_LOWER_ADDR 0x10
#define NTB_EPF_UPPER_ADDR 0x14
#define NTB_EPF_LOWER_SIZE 0x18
#define NTB_EPF_UPPER_SIZE 0x1C
#define NTB_EPF_MW_COUNT 0x20
-#define NTB_EPF_MW1_OFFSET 0x24
+#define NTB_EPF_MW1_OFFSET 0x24 /* v0 only */
#define NTB_EPF_SPAD_OFFSET 0x28
#define NTB_EPF_SPAD_COUNT 0x2C
#define NTB_EPF_DB_ENTRY_SIZE 0x30
#define NTB_EPF_DB_DATA(n) (0x34 + (n) * 4)
#define NTB_EPF_DB_OFFSET(n) (0xB4 + (n) * 4)
+/* v1 only */
+#define NTB_EPF_MW_BAR 0x134
+#define NTB_EPF_MW_GROUP_SIZE 0x138
/*
* Legacy doorbell slot layout when paired with pci-epf-*ntb:
@@ -88,7 +92,8 @@ enum epf_irq_slot {
EPF_IRQ_DB_START,
};
-#define NTB_EPF_MAX_MW_COUNT (NTB_BAR_NUM - BAR_MW1)
+#define NTB_EPF_LEGACY_MAX_MW_COUNT (NTB_BAR_NUM - BAR_MW1)
+#define NTB_EPF_MAX_MW_COUNT 16
struct ntb_epf_dev;
@@ -105,9 +110,12 @@ struct ntb_epf_dev {
const enum pci_barno *barno_map;
+ u32 ctrl_version;
unsigned int mw_count;
unsigned int spad_count;
unsigned int db_count;
+ u32 mw_bar;
+ u32 mw_size;
void __iomem *ctrl_reg;
void __iomem *db_reg;
@@ -171,6 +179,9 @@ static int ntb_epf_mw_to_bar(struct ntb_epf_dev *ndev, int idx)
return -EINVAL;
}
+ if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
+ return ndev->mw_bar;
+
return ndev->barno_map[BAR_MW1 + idx];
}
@@ -211,8 +222,37 @@ static int ntb_epf_mw_get_align(struct ntb_dev *ntb, int pidx, int idx,
if (size_align)
*size_align = 1;
- if (size_max)
- *size_max = pci_resource_len(ndev->ntb.pdev, bar);
+ if (size_max) {
+ if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
+ *size_max = ndev->mw_size;
+ else
+ *size_max = pci_resource_len(ndev->ntb.pdev, bar);
+ }
+
+ return 0;
+}
+
+static int ntb_epf_mw_get_trans_group(struct ntb_dev *ntb, int pidx, int idx,
+ int *gidx, int *gcount)
+{
+ struct ntb_epf_dev *ndev = ntb_ndev(ntb);
+ struct device *dev = ndev->dev;
+
+ if (pidx != NTB_DEF_PEER_IDX) {
+ dev_err(dev, "Unsupported Peer ID %d\n", pidx);
+ return -EINVAL;
+ }
+
+ if (idx < 0 || idx >= ndev->mw_count) {
+ dev_err(dev, "Unsupported Memory Window index %d\n", idx);
+ return -EINVAL;
+ }
+
+ if (gidx)
+ *gidx = ndev->ctrl_version >= NTB_EPF_CTRL_V1 ? 0 : idx;
+ if (gcount)
+ *gcount = ndev->ctrl_version >= NTB_EPF_CTRL_V1 ?
+ ndev->mw_count : 1;
return 0;
}
@@ -520,21 +560,27 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
phys_addr_t *base, resource_size_t *size)
{
struct ntb_epf_dev *ndev = ntb_ndev(ntb);
- u32 offset = 0;
+ resource_size_t offset = 0;
int bar;
- if (idx == 0)
- offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
-
bar = ntb_epf_mw_to_bar(ndev, idx);
if (bar < 0)
return bar;
+ if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
+ offset = (resource_size_t)idx * ndev->mw_size;
+ else if (idx == 0)
+ offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
+
if (base)
*base = pci_resource_start(ndev->ntb.pdev, bar) + offset;
- if (size)
- *size = pci_resource_len(ndev->ntb.pdev, bar) - offset;
+ if (size) {
+ if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
+ *size = ndev->mw_size;
+ else
+ *size = pci_resource_len(ndev->ntb.pdev, bar) - offset;
+ }
return 0;
}
@@ -601,6 +647,7 @@ static const struct ntb_dev_ops ntb_epf_ops = {
.db_vector_count = ntb_epf_db_vector_count,
.db_vector_mask = ntb_epf_db_vector_mask,
.db_set_mask = ntb_epf_db_set_mask,
+ .mw_get_trans_group = ntb_epf_mw_get_trans_group,
.mw_set_trans = ntb_epf_mw_set_trans,
.mw_clear_trans = ntb_epf_mw_clear_trans,
.peer_mw_get_addr = ntb_epf_peer_mw_get_addr,
@@ -629,14 +676,45 @@ static inline void ntb_epf_init_struct(struct ntb_epf_dev *ndev,
static int ntb_epf_init_dev(struct ntb_epf_dev *ndev)
{
struct device *dev = ndev->dev;
+ resource_size_t bar_len;
+ u32 group_size;
int ret;
+ ndev->ctrl_version = readl(ndev->ctrl_reg + NTB_EPF_CTRL_VERSION);
ndev->mw_count = readl(ndev->ctrl_reg + NTB_EPF_MW_COUNT);
- if (ndev->mw_count > NTB_EPF_MAX_MW_COUNT) {
+
+ if (ndev->ctrl_version && ndev->ctrl_version != NTB_EPF_CTRL_V1) {
+ dev_err(dev, "Unsupported control region version: %u\n",
+ ndev->ctrl_version);
+ return -EINVAL;
+ }
+
+ if ((!ndev->ctrl_version &&
+ ndev->mw_count > NTB_EPF_LEGACY_MAX_MW_COUNT) ||
+ ndev->mw_count > NTB_EPF_MAX_MW_COUNT) {
dev_err(dev, "Unsupported MW count: %u\n", ndev->mw_count);
return -EINVAL;
}
+ if (ndev->ctrl_version >= NTB_EPF_CTRL_V1) {
+ ndev->mw_bar = readl(ndev->ctrl_reg + NTB_EPF_MW_BAR);
+ group_size = readl(ndev->ctrl_reg + NTB_EPF_MW_GROUP_SIZE);
+
+ if (ndev->mw_count < 2 || ndev->mw_bar > BAR_5 ||
+ !group_size || group_size % ndev->mw_count) {
+ dev_err(dev, "Invalid packed MW layout\n");
+ return -EINVAL;
+ }
+
+ ndev->mw_size = group_size / ndev->mw_count;
+ bar_len = pci_resource_len(ndev->ntb.pdev, ndev->mw_bar);
+ if (!IS_ALIGNED(ndev->mw_size, SZ_4K) || group_size != bar_len) {
+ dev_err(dev, "Packed MW layout does not match BAR%u\n",
+ ndev->mw_bar);
+ return -EINVAL;
+ }
+ }
+
/* One Link interrupt and rest doorbell interrupt */
ret = ntb_epf_init_isr(ndev, NTB_EPF_MIN_DB_COUNT + 1,
NTB_EPF_MAX_DB_COUNT + 1);
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout
2026-08-03 18:04 ` [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout Koichiro Den
@ 2026-08-03 18:23 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:23 UTC (permalink / raw)
To: Koichiro Den; +Cc: ntb, linux-pci
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Missing `.mw_set_trans_group` callback breaks functionality for the V1 packed memory window layout.
Pre-existing issues:
- [Critical] Untrusted offset from endpoint causes integer underflow, allowing arbitrary host physical memory exposure.
- [High] Off-by-one bounds check allows out-of-bounds access and out-of-bounds physical memory mappings.
- [High] Hardware command return values are silently ignored, masking endpoint failures.
- [High] Concurrent translation configurations race on shared endpoint configuration registers without locking.
- [High] Probe error path leaks PCI regions and I/O mappings.
--
--- Patch [2]: [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout ---
commit 552e0bc5df4566e72805c5d6eadc9d96fa21be65
Author: Koichiro Den <den@valinux.co.jp>
NTB: epf: Parse a versioned packed MW layout
The legacy control layout cannot describe multiple logical MWs sharing
one BAR. Treat a zero version as legacy; version 1 describes all MWs as
one packed group using a BAR number and aggregate size. Derive the
equal-sized logical MWs from the existing MW count.
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index c47607d4f8a7f..46f37c2dea7b2 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -171,6 +179,9 @@ static int ntb_epf_mw_to_bar(struct ntb_epf_dev *ndev, int idx)
> return -EINVAL;
> }
>
> + if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
> + return ndev->mw_bar;
> +
[Severity: High]
This is a pre-existing issue, but does ntb_epf_mw_to_bar() have an off-by-one
bounds check?
If idx is equal to ndev->mw_count, it bypasses the idx > ndev->mw_count check.
In legacy version 0, this accesses index 7 of a 7-element barno_map array.
In version 1, it calculates an offset equal to the entire base address register
size in ntb_epf_peer_mw_get_addr(), resulting in a physical mapping address
exactly past the end of the region.
Could this allow the client to map and access arbitrary adjacent host physical
memory?
> return ndev->barno_map[BAR_MW1 + idx];
> }
[ ... ]
> @@ -520,21 +560,27 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
> phys_addr_t *base, resource_size_t *size)
> {
> struct ntb_epf_dev *ndev = ntb_ndev(ntb);
> - u32 offset = 0;
> + resource_size_t offset = 0;
> int bar;
>
> bar = ntb_epf_mw_to_bar(ndev, idx);
> if (bar < 0)
> return bar;
>
> + if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
> + offset = (resource_size_t)idx * ndev->mw_size;
> + else if (idx == 0)
> + offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
> +
> if (base)
> *base = pci_resource_start(ndev->ntb.pdev, bar) + offset;
>
> - if (size)
> - *size = pci_resource_len(ndev->ntb.pdev, bar) - offset;
> + if (size) {
> + if (ndev->ctrl_version >= NTB_EPF_CTRL_V1)
> + *size = ndev->mw_size;
> + else
> + *size = pci_resource_len(ndev->ntb.pdev, bar) - offset;
> + }
[Severity: High]
This is a pre-existing issue, but could the untrusted offset read from the
endpoint cause an integer underflow here?
The offset is read directly from endpoint-controlled registers and subtracted
from pci_resource_len() without validation. If an untrusted peer endpoint sets
NTB_EPF_MW1_OFFSET to a value larger than the mapped resource size, does the
unsigned subtraction underflow?
This could produce a near-maximum integer value for the mapping size, allowing
an untrusted peer endpoint to map and access arbitrary host physical memory.
> return 0;
> }
[ ... ]
> @@ -601,6 +647,7 @@ static const struct ntb_dev_ops ntb_epf_ops = {
> .db_vector_count = ntb_epf_db_vector_count,
> .db_vector_mask = ntb_epf_db_vector_mask,
> .db_set_mask = ntb_epf_db_set_mask,
> + .mw_get_trans_group = ntb_epf_mw_get_trans_group,
> .mw_set_trans = ntb_epf_mw_set_trans,
> .mw_clear_trans = ntb_epf_mw_clear_trans,
[Severity: High]
Does the missing .mw_set_trans_group callback break the version 1 packed memory
window layout?
Because this layout defines version 1 windows as a group, the framework enforces
that translations must be configured via .mw_set_trans_group, actively rejecting
single .mw_set_trans calls by returning -EOPNOTSUPP via ntb_mw_check_singleton.
By failing to implement and export .mw_set_trans_group in ntb_epf_ops, is it
impossible to configure any version 1 memory windows?
[Severity: High]
This is a pre-existing issue, but do ntb_epf_mw_clear_trans() and
ntb_epf_mw_set_trans() completely discard hardware command return values?
In ntb_epf_mw_clear_trans(), the return value of ntb_epf_send_command()
is never assigned to the ret variable, making the subsequent if (ret)
check dead code.
This masks endpoint hardware failures and falsely informs the
framework that the window was successfully torn down, potentially causing system
faults if the system reuses a window still active on the endpoint.
[Severity: High]
This is a pre-existing issue, but does ntb_epf_mw_set_trans() race on shared
endpoint configuration registers without locking?
The parameter registers like NTB_EPF_LOWER_ADDR are written without holding
ndev->cmd_lock, which is only acquired later inside ntb_epf_send_command().
If multiple processors configure different memory windows concurrently, could
one thread overwrite the parameter registers before another thread sends its
command, leading to incorrectly routed memory windows?
> .peer_mw_get_addr = ntb_epf_peer_mw_get_addr,
[ ... ]
[Severity: High]
This is a pre-existing issue, but does the probe error path leak mapped regions?
Looking at ntb_epf_init_pci(), if it fails after pci_request_regions() succeeds
(for example, during pci_iomap()), the err_pci_regions label disables the device
but omits calling pci_release_regions() and pci_iounmap().
Does this permanently leak the allocated resources?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
2026-08-03 18:04 ` [PATCH 01/11] NTB: Add atomic MW translation group operations Koichiro Den
2026-08-03 18:04 ` [PATCH 02/11] NTB: epf: Parse a versioned packed MW layout Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:25 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks Koichiro Den
` (8 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
Teach pci-epf-vntb to build a packed layout and advertise its logical
MWs as one version 1 translation group. Non-packed configurations keep
the version 0 layout and one MW per BAR.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 255 +++++++++++++++---
1 file changed, 219 insertions(+), 36 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index c3caec927d74..f22459e5a8c6 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -68,6 +68,9 @@ static struct workqueue_struct *kpcintb_workqueue;
#define DB_COUNT_MASK GENMASK(15, 0)
#define MSIX_ENABLE BIT(16)
#define MAX_MW 4
+#define EPF_NTB_MAX_MW 16
+#define EPF_NTB_CTRL_V0 0
+#define EPF_NTB_CTRL_V1 1
/* Limit per-work execution to avoid monopolizing kworker on doorbell storms. */
#define VNTB_PEER_DB_WORK_BUDGET 5
@@ -119,7 +122,7 @@ struct epf_ntb_ctrl {
u32 argument;
u16 command_status;
u16 link_status;
- u32 topology;
+ u32 version;
u64 addr;
u64 size;
u32 num_mws;
@@ -129,17 +132,27 @@ struct epf_ntb_ctrl {
u32 db_entry_size;
u32 db_data[MAX_DB_COUNT];
u32 db_offset[MAX_DB_COUNT];
+ u32 mw_bar;
+ u32 mw_group_size;
} __packed;
+struct epf_ntb_mw_layout {
+ enum pci_barno barno;
+ u64 offset;
+ u64 size;
+};
+
struct epf_ntb {
struct ntb_dev ntb;
struct pci_epf *epf;
struct config_group group;
u32 num_mws;
+ u32 packed_mws;
u32 db_count;
u32 spad_count;
u64 mws_size[MAX_MW];
+ struct epf_ntb_mw_layout mw_layout[EPF_NTB_MAX_MW];
atomic64_t db;
atomic64_t peer_db_pending;
struct work_struct peer_db_work;
@@ -162,8 +175,8 @@ struct epf_ntb {
u32 *epf_db;
- phys_addr_t vpci_mw_phy[MAX_MW];
- void __iomem *vpci_mw_addr[MAX_MW];
+ phys_addr_t vpci_mw_phy[EPF_NTB_MAX_MW];
+ void __iomem *vpci_mw_addr[EPF_NTB_MAX_MW];
struct delayed_work cmd_handler;
};
@@ -199,11 +212,25 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
return 0;
}
+static int epf_ntb_get_mw_group(struct epf_ntb *ntb, unsigned int mw,
+ unsigned int *first, unsigned int *count)
+{
+ if (mw >= ntb->num_mws)
+ return -EINVAL;
+
+ if (first)
+ *first = ntb->packed_mws ? 0 : mw;
+ if (count)
+ *count = ntb->packed_mws ? ntb->num_mws : 1;
+
+ return 0;
+}
+
/**
* epf_ntb_configure_mw() - Configure the Outbound Address Space for VHOST
* to access the memory window of HOST
* @ntb: NTB device that facilitates communication between HOST and VHOST
- * @mw: Index of the memory window (either 0, 1, 2 or 3)
+ * @mw: Index of the memory window
*
* EP Outbound Window
* +--------+ +-----------+
@@ -227,13 +254,23 @@ static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
{
phys_addr_t phys_addr;
u8 func_no, vfunc_no;
+ unsigned int count;
u64 addr, size;
- int ret = 0;
+ int ret;
+
+ ret = epf_ntb_get_mw_group(ntb, mw, NULL, &count);
+ if (ret)
+ return ret;
+ if (count > 1)
+ return -EOPNOTSUPP;
phys_addr = ntb->vpci_mw_phy[mw];
addr = ntb->reg->addr;
size = ntb->reg->size;
+ if (!size || size > ntb->mw_layout[mw].size)
+ return -EINVAL;
+
func_no = ntb->epf->func_no;
vfunc_no = ntb->epf->vfunc_no;
@@ -247,17 +284,30 @@ static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
/**
* epf_ntb_teardown_mw() - Teardown the configured OB ATU
* @ntb: NTB device that facilitates communication between HOST and VHOST
- * @mw: Index of the memory window (either 0, 1, 2 or 3)
+ * @mw: Index of the memory window
*
* Teardown the configured OB ATU configured in epf_ntb_configure_mw() using
* pci_epc_unmap_addr()
+ *
+ * Returns: Zero for success, or an error code in case of failure
*/
-static void epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
+static int epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
{
+ unsigned int count;
+ int ret;
+
+ ret = epf_ntb_get_mw_group(ntb, mw, NULL, &count);
+ if (ret)
+ return ret;
+ if (count > 1)
+ return -EOPNOTSUPP;
+
pci_epc_unmap_addr(ntb->epf->epc,
ntb->epf->func_no,
ntb->epf->vfunc_no,
ntb->vpci_mw_phy[mw]);
+
+ return 0;
}
/**
@@ -316,8 +366,8 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
ctrl->command_status = COMMAND_STATUS_OK;
break;
case COMMAND_TEARDOWN_MW:
- epf_ntb_teardown_mw(ntb, argument);
- ctrl->command_status = COMMAND_STATUS_OK;
+ ret = epf_ntb_teardown_mw(ntb, argument);
+ ctrl->command_status = ret ? COMMAND_STATUS_ERROR : COMMAND_STATUS_OK;
break;
case COMMAND_LINK_UP:
ntb->linkup = true;
@@ -434,6 +484,78 @@ static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
pci_epf_free_space(ntb->epf, ntb->reg, barno, 0);
}
+static int epf_ntb_build_mw_layout(struct epf_ntb *ntb)
+{
+ struct device *dev = &ntb->epf->dev;
+ u64 size;
+ int i;
+
+ if (ntb->packed_mws) {
+ enum pci_barno barno = ntb->epf_ntb_bar[BAR_MW1];
+
+ /*
+ * A packed group requires contiguous backing for the entire BAR.
+ * Per-MW size limits such as max_mw_size are not supported.
+ */
+ if (ntb->packed_mws != ntb->num_mws) {
+ dev_err(dev, "packed_mws must match num_mws\n");
+ return -EINVAL;
+ }
+
+ if (barno < 0) {
+ dev_err(dev, "packed MW has no BAR\n");
+ return -EINVAL;
+ }
+
+ if (!ntb->mws_size[0] ||
+ ntb->mws_size[0] > U32_MAX ||
+ !is_power_of_2(ntb->mws_size[0]) ||
+ ntb->mws_size[0] % ntb->packed_mws) {
+ dev_err(dev, "invalid packed MW size\n");
+ return -EINVAL;
+ }
+
+ size = ntb->mws_size[0] / ntb->packed_mws;
+ if (!IS_ALIGNED(size, SZ_4K)) {
+ dev_err(dev, "invalid packed MW member size\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < ntb->num_mws; i++) {
+ ntb->mw_layout[i].barno = barno;
+ ntb->mw_layout[i].offset = i * size;
+ ntb->mw_layout[i].size = size;
+ }
+
+ return 0;
+ }
+
+ if (ntb->num_mws > MAX_MW) {
+ dev_err(dev, "num_mws=%u requires packed_mws\n", ntb->num_mws);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < ntb->num_mws; i++) {
+ enum pci_barno barno = ntb->epf_ntb_bar[BAR_MW1 + i];
+
+ if (barno < 0) {
+ dev_err(dev, "MW%d has no BAR\n", i + 1);
+ return -EINVAL;
+ }
+
+ if (!ntb->mws_size[i]) {
+ dev_err(dev, "MW%d has no size\n", i + 1);
+ return -EINVAL;
+ }
+
+ ntb->mw_layout[i].barno = barno;
+ ntb->mw_layout[i].offset = 0;
+ ntb->mw_layout[i].size = ntb->mws_size[i];
+ }
+
+ return 0;
+}
+
/**
* epf_ntb_config_spad_bar_alloc() - Allocate memory for config + scratchpad
* region
@@ -474,6 +596,7 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
ntb->reg = base;
ctrl = ntb->reg;
+ ctrl->version = ntb->packed_mws ? EPF_NTB_CTRL_V1 : EPF_NTB_CTRL_V0;
ctrl->spad_offset = ctrl_size;
ctrl->spad_count = spad_count;
@@ -487,6 +610,11 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
ntb->reg->db_offset[i] = 0;
}
+ if (ntb->packed_mws) {
+ ctrl->mw_bar = ntb->mw_layout[0].barno;
+ ctrl->mw_group_size = ntb->mws_size[0];
+ }
+
return 0;
}
@@ -761,16 +889,28 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
u64 size;
enum pci_barno barno;
struct device *dev = &ntb->epf->dev;
+ u64 bar_size[BAR_5 + 1] = {};
+ bool bar_set[BAR_5 + 1] = {};
for (i = 0; i < ntb->num_mws; i++) {
- size = ntb->mws_size[i];
- barno = ntb->epf_ntb_bar[BAR_MW1 + i];
+ size = ntb->mw_layout[i].size;
+ barno = ntb->mw_layout[i].barno;
+ bar_size[barno] = max(bar_size[barno],
+ ntb->mw_layout[i].offset + size);
+ }
+
+ for (i = 0; i < ntb->num_mws; i++) {
+ size = ntb->mw_layout[i].size;
+ barno = ntb->mw_layout[i].barno;
+
+ if (bar_set[barno])
+ goto alloc_vpci_mw;
ntb->epf->bar[barno].barno = barno;
- ntb->epf->bar[barno].size = size;
+ ntb->epf->bar[barno].size = bar_size[barno];
ntb->epf->bar[barno].addr = NULL;
ntb->epf->bar[barno].phys_addr = 0;
- ntb->epf->bar[barno].flags |= upper_32_bits(size) ?
+ ntb->epf->bar[barno].flags |= upper_32_bits(bar_size[barno]) ?
PCI_BASE_ADDRESS_MEM_TYPE_64 :
PCI_BASE_ADDRESS_MEM_TYPE_32;
@@ -782,7 +922,9 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
dev_err(dev, "MW set failed\n");
goto err_alloc_mem;
}
+ bar_set[barno] = true;
+alloc_vpci_mw:
/* Allocate EPC outbound memory windows to vpci vntb device */
ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
&ntb->vpci_mw_phy[i],
@@ -790,17 +932,13 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
if (!ntb->vpci_mw_addr[i]) {
ret = -ENOMEM;
dev_err(dev, "Failed to allocate source address\n");
- goto err_set_bar;
+ i++;
+ goto err_alloc_mem;
}
}
return ret;
-err_set_bar:
- pci_epc_clear_bar(ntb->epf->epc,
- ntb->epf->func_no,
- ntb->epf->vfunc_no,
- &ntb->epf->bar[barno]);
err_alloc_mem:
epf_ntb_mw_bar_clear(ntb, i);
return ret;
@@ -809,24 +947,32 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
/**
* epf_ntb_mw_bar_clear() - Clear Memory window BARs
* @ntb: NTB device that facilitates communication between HOST and VHOST
- * @num_mws: the number of Memory window BARs that to be cleared
+ * @num_mws: Number of logical memory windows to clean up
*/
static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
{
+ bool bar_cleared[BAR_5 + 1] = {};
enum pci_barno barno;
int i;
for (i = 0; i < num_mws; i++) {
- barno = ntb->epf_ntb_bar[BAR_MW1 + i];
- pci_epc_clear_bar(ntb->epf->epc,
- ntb->epf->func_no,
- ntb->epf->vfunc_no,
- &ntb->epf->bar[barno]);
+ barno = ntb->mw_layout[i].barno;
+ if (!bar_cleared[barno]) {
+ pci_epc_clear_bar(ntb->epf->epc,
+ ntb->epf->func_no,
+ ntb->epf->vfunc_no,
+ &ntb->epf->bar[barno]);
+ bar_cleared[barno] = true;
+ }
+
+ if (!ntb->vpci_mw_addr[i])
+ continue;
pci_epc_mem_free_addr(ntb->epf->epc,
ntb->vpci_mw_phy[i],
ntb->vpci_mw_addr[i],
- ntb->mws_size[i]);
+ ntb->mw_layout[i].size);
+ ntb->vpci_mw_addr[i] = NULL;
}
}
@@ -916,16 +1062,21 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
}
}
- /* These are optional BARs which don't impact NTB functionality */
- for (bar = BAR_MW1, i = 1; i < num_mws; bar++, i++) {
- barno = epf_ntb_find_bar(ntb, epc_features, bar, barno);
- if (barno < 0) {
- ntb->num_mws = i;
- dev_dbg(dev, "BAR not available for > MW%d\n", i + 1);
+ if (!ntb->packed_mws) {
+ /* These are optional BARs which don't impact NTB functionality */
+ for (bar = BAR_MW1, i = 1;
+ i < num_mws && bar <= BAR_MW4; bar++, i++) {
+ barno = epf_ntb_find_bar(ntb, epc_features, bar, barno);
+ if (barno < 0) {
+ ntb->num_mws = i;
+ dev_dbg(dev, "BAR not available for > MW%d\n",
+ i + 1);
+ break;
+ }
}
}
- return 0;
+ return epf_ntb_build_mw_layout(ntb);
}
/**
@@ -1352,6 +1503,28 @@ static int vntb_epf_mw_count(struct ntb_dev *ntb, int pidx)
return ndev->num_mws;
}
+static int vntb_epf_mw_get_trans_group(struct ntb_dev *ndev, int pidx,
+ int idx, int *gidx, int *gcount)
+{
+ struct epf_ntb *ntb = ntb_ndev(ndev);
+ unsigned int first, count;
+ int ret;
+
+ if (pidx != NTB_DEF_PEER_IDX || idx < 0 || idx >= ntb->num_mws)
+ return -EINVAL;
+
+ ret = epf_ntb_get_mw_group(ntb, idx, &first, &count);
+ if (ret)
+ return ret;
+
+ if (gidx)
+ *gidx = first;
+ if (gcount)
+ *gcount = count;
+
+ return 0;
+}
+
static int vntb_epf_spad_count(struct ntb_dev *ntb)
{
return ntb_ndev(ntb)->spad_count;
@@ -1415,13 +1588,22 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
dma_addr_t addr, resource_size_t size)
{
struct epf_ntb *ntb = ntb_ndev(ndev);
+ struct epf_ntb_mw_layout *mw;
struct pci_epf_bar *epf_bar;
enum pci_barno barno;
int ret;
struct device *dev;
dev = &ntb->ntb.dev;
- barno = ntb->epf_ntb_bar[BAR_MW1 + idx];
+ if (pidx != NTB_DEF_PEER_IDX ||
+ idx < 0 || idx >= ntb->num_mws)
+ return -EINVAL;
+
+ mw = &ntb->mw_layout[idx];
+ if (size > mw->size)
+ return -EINVAL;
+
+ barno = mw->barno;
epf_bar = &ntb->epf->bar[barno];
epf_bar->phys_addr = addr;
epf_bar->barno = barno;
@@ -1450,7 +1632,7 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
*base = ntb->vpci_mw_phy[idx];
if (size)
- *size = ntb->mws_size[idx];
+ *size = ntb->mw_layout[idx].size;
return 0;
}
@@ -1608,7 +1790,7 @@ static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
*size_align = 1;
if (size_max)
- *size_max = ntb->mws_size[idx];
+ *size_max = ntb->mw_layout[idx].size;
return 0;
}
@@ -1656,6 +1838,7 @@ static const struct ntb_dev_ops vntb_epf_ops = {
.db_vector_count = vntb_epf_db_vector_count,
.db_vector_mask = vntb_epf_db_vector_mask,
.db_set_mask = vntb_epf_db_set_mask,
+ .mw_get_trans_group = vntb_epf_mw_get_trans_group,
.mw_set_trans = vntb_epf_mw_set_trans,
.mw_clear_trans = vntb_epf_mw_clear_trans,
.peer_mw_get_addr = vntb_epf_peer_mw_get_addr,
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling
2026-08-03 18:04 ` [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling Koichiro Den
@ 2026-08-03 18:25 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:25 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci, ntb
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The packed_mws feature is entirely unreachable as it cannot be configured by userspace.
- [Medium] Outbound ATU mapping is completely broken for packed memory windows due to explicit rejection.
- [Medium] Inbound translations for packed configurations are unusable and dynamically corrupt the shared BAR.
--
--- Patch [3]: [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling ---
commit b1065455da6e47589efee078ad0c8d01095b3f1a
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Add packed MW layout handling
Teach pci-epf-vntb to build a packed layout and advertise its logical
MWs as one version 1 translation group. Non-packed configurations keep
the version 0 layout and one MW per BAR.
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index c3caec927d748..f22459e5a8c69 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
[ ... ]
> @@ -129,17 +132,27 @@ struct epf_ntb_ctrl {
> u32 db_entry_size;
> u32 db_data[MAX_DB_COUNT];
> u32 db_offset[MAX_DB_COUNT];
> + u32 mw_bar;
> + u32 mw_group_size;
> } __packed;
>
> +struct epf_ntb_mw_layout {
> + enum pci_barno barno;
> + u64 offset;
> + u64 size;
> +};
> +
> struct epf_ntb {
> struct ntb_dev ntb;
> struct pci_epf *epf;
> struct config_group group;
>
> u32 num_mws;
> + u32 packed_mws;
[Severity: Medium]
Will userspace be able to configure packed_mws?
It appears that there is no EPF_NTB_W(packed_mws) macro or a corresponding
entry in the epf_ntb_attrs[] configfs array, which means this field might
not be writable by userspace.
Additionally, looking at epf_ntb_num_mws_store(), it restricts num_mws to
MAX_MW (4), so the new EPF_NTB_MAX_MW (16) limit for packed configurations
cannot be reached.
[ ... ]
> @@ -227,13 +254,23 @@ static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
> {
> phys_addr_t phys_addr;
> u8 func_no, vfunc_no;
> + unsigned int count;
> u64 addr, size;
> - int ret = 0;
> + int ret;
> +
> + ret = epf_ntb_get_mw_group(ntb, mw, NULL, &count);
> + if (ret)
> + return ret;
> + if (count > 1)
> + return -EOPNOTSUPP;
[Severity: Medium]
How will outbound memory windows be mapped for packed layouts?
If count > 1 is explicitly rejected here with -EOPNOTSUPP, and there is no
COMMAND_CONFIGURE_MW_GROUP handler in epf_ntb_cmd_handler() to manage packed
layouts, will the NTB host always receive a COMMAND_STATUS_ERROR when
attempting to map an outbound grouped window?
[ ... ]
> @@ -1656,6 +1838,7 @@ static const struct ntb_dev_ops vntb_epf_ops = {
> .db_vector_count = vntb_epf_db_vector_count,
> .db_vector_mask = vntb_epf_db_vector_mask,
> .db_set_mask = vntb_epf_db_set_mask,
> + .mw_get_trans_group = vntb_epf_mw_get_trans_group,
[Severity: Medium]
Are the .mw_set_trans_group and .mw_clear_trans_group callbacks missing?
Without the group callbacks, the NTB subsystem falls back to using
.mw_set_trans (vntb_epf_mw_set_trans), which locklessly overwrites
epf_bar->phys_addr and epf_bar->size with a single segment's attributes.
Because all segments of a packed MW share the same BAR, won't calling this
for each segment continuously overwrite the entire BAR's inbound translation
and corrupt the mappings for all other windows in the group?
> .mw_set_trans = vntb_epf_mw_set_trans,
> .mw_clear_trans = vntb_epf_mw_clear_trans,
> .peer_mw_get_addr = vntb_epf_peer_mw_get_addr,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (2 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 03/11] PCI: endpoint: pci-epf-vntb: Add packed MW layout handling Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:25 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space Koichiro Den
` (7 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
All RC-to-EP MWs in a packed group share one inbound BAR. Setting each
MW separately would keep replacing that BAR and could expose a partial
group.
Add group callbacks that program the shared BAR once for the complete
backing range. The EPC API has no operation to disable only the inbound
translation without also clearing the host-assigned BAR address. Keep
clear as a no-op, matching the existing per-MW behavior.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index f22459e5a8c6..0c72b907c289 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -1622,6 +1622,31 @@ static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
return 0;
}
+static int
+vntb_epf_mw_set_trans_group(struct ntb_dev *ndev, int pidx, int widx,
+ dma_addr_t addr, resource_size_t size)
+{
+ struct epf_ntb *ntb = ntb_ndev(ndev);
+ struct pci_epf_bar *epf_bar;
+
+ if (pidx != NTB_DEF_PEER_IDX || !ntb->packed_mws || widx)
+ return -EINVAL;
+ if (!IS_ALIGNED(addr, SZ_4K) || size != ntb->mws_size[0])
+ return -EINVAL;
+
+ epf_bar = &ntb->epf->bar[ntb->mw_layout[0].barno];
+ epf_bar->phys_addr = addr;
+
+ return pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
+ ntb->epf->vfunc_no, epf_bar);
+}
+
+static int vntb_epf_mw_clear_trans_group(struct ntb_dev *ndev, int pidx,
+ int widx)
+{
+ return 0;
+}
+
static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
phys_addr_t *base, resource_size_t *size)
{
@@ -1839,6 +1864,8 @@ static const struct ntb_dev_ops vntb_epf_ops = {
.db_vector_mask = vntb_epf_db_vector_mask,
.db_set_mask = vntb_epf_db_set_mask,
.mw_get_trans_group = vntb_epf_mw_get_trans_group,
+ .mw_set_trans_group = vntb_epf_mw_set_trans_group,
+ .mw_clear_trans_group = vntb_epf_mw_clear_trans_group,
.mw_set_trans = vntb_epf_mw_set_trans,
.mw_clear_trans = vntb_epf_mw_clear_trans,
.peer_mw_get_addr = vntb_epf_peer_mw_get_addr,
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
2026-08-03 18:04 ` [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks Koichiro Den
@ 2026-08-03 18:25 ` sashiko-bot
2026-08-04 1:41 ` Koichiro Den
0 siblings, 1 reply; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:25 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci, ntb
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] DMA Use-After-Free due to `vntb_epf_mw_clear_trans_group()` and `vntb_epf_mw_clear_trans()` returning success (0) without actually disabling the inbound hardware translations.
- [High] The `vntb_epf_mw_set_trans()` function incorrectly hardcodes `func_no` and `vfunc_no` to 0 when invoking `pci_epc_set_bar()`, whereas other EPF APIs (including the newly added group API) correctly use the endpoint's configured function numbers.
- [High] The `vntb_pci_driver` structure is missing a `.remove` callback, causing NTB device leaks and use-after-free conditions when the endpoint function is unbound.
--
--- Patch [4]: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks ---
commit 419429d9534b2c856c11cb4a4a4107e84a87fe8e
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
All RC-to-EP MWs in a packed group share one inbound BAR. Setting each
MW separately would keep replacing that BAR and could expose a partial
group. Add group callbacks that program the shared BAR once for the
complete backing range.
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index f22459e5a8c69..0c72b907c2896 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1622,6 +1622,31 @@ static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
> return 0;
> }
[Severity: High]
This isn't a bug introduced by this patch, but while looking at this context,
I noticed that the existing vntb_epf_mw_set_trans() hardcodes the function
numbers to 0:
drivers/pci/endpoint/functions/pci-epf-vntb.c:vntb_epf_mw_set_trans() {
...
ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
...
}
Does this incorrectly configure the BAR for function 0 when the virtual NTB
device is bound to a non-zero endpoint function or virtual function?
If the device is configured via configfs with a non-zero function, could this
lead to silent failures in memory window translation or corrupt the ATU
configuration of function 0?
> +
> +static int
> +vntb_epf_mw_set_trans_group(struct ntb_dev *ndev, int pidx, int widx,
> + dma_addr_t addr, resource_size_t size)
> +{
[ ... ]
> + return pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> + ntb->epf->vfunc_no, epf_bar);
> +}
> +
> +static int vntb_epf_mw_clear_trans_group(struct ntb_dev *ndev, int pidx,
> + int widx)
> +{
> + return 0;
> +}
[Severity: Critical]
This is a pre-existing issue perpetuated here by the new group API, but does
returning 0 without actually disabling the inbound hardware translations create a
DMA use-after-free window?
When an NTB client unmaps a memory window group and frees the backing memory,
the hardware BAR mapping will remain active.
If the local host frees this memory, won't the endpoint hardware continue
translating incoming PCIe accesses to the freed physical address, allowing a
remote peer to access arbitrary new allocations on the endpoint?
[Severity: High]
This is another pre-existing issue, but I noticed the vntb_pci_driver structure
is missing a .remove callback:
drivers/pci/endpoint/functions/pci-epf-vntb.c:
static struct pci_driver vntb_pci_driver = {
.name = "pci-vntb",
.id_table = pci_vntb_table,
.probe = pci_vntb_probe,
};
Without a .remove callback, ntb_unregister_device() is never called when the
EPF device is unbound via configfs.
Does this cause NTB device leaks and driver state use-after-free?
If the endpoint function is unbound, won't the NTB clients remain active and
bound to a device whose backing EPF structures, DMA masks, and hardware
mappings have been destroyed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=4
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
2026-08-03 18:25 ` sashiko-bot
@ 2026-08-04 1:41 ` Koichiro Den
0 siblings, 0 replies; 26+ messages in thread
From: Koichiro Den @ 2026-08-04 1:41 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-pci, ntb
On Mon, Aug 03, 2026 at 06:25:28PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] DMA Use-After-Free due to `vntb_epf_mw_clear_trans_group()` and `vntb_epf_mw_clear_trans()` returning success (0) without actually disabling the inbound hardware translations.
To be clear, vntb_epf_mw_clear_trans_group() is intentionally left as a no-op,
matching the existing the existing vntb_epf_mw_clear_trans() behavior. This
series does not try to fix that pre-existing limitation.
See the "About existing inbound MW clear behavior" note in the cover letter.
Koichiro
> - [High] The `vntb_epf_mw_set_trans()` function incorrectly hardcodes `func_no` and `vfunc_no` to 0 when invoking `pci_epc_set_bar()`, whereas other EPF APIs (including the newly added group API) correctly use the endpoint's configured function numbers.
> - [High] The `vntb_pci_driver` structure is missing a `.remove` callback, causing NTB device leaks and use-after-free conditions when the endpoint function is unbound.
> --
>
> --- Patch [4]: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks ---
> commit 419429d9534b2c856c11cb4a4a4107e84a87fe8e
> Author: Koichiro Den <den@valinux.co.jp>
>
> PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
>
> All RC-to-EP MWs in a packed group share one inbound BAR. Setting each
> MW separately would keep replacing that BAR and could expose a partial
> group. Add group callbacks that program the shared BAR once for the
> complete backing range.
>
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index f22459e5a8c69..0c72b907c2896 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -1622,6 +1622,31 @@ static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
> > return 0;
> > }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while looking at this context,
> I noticed that the existing vntb_epf_mw_set_trans() hardcodes the function
> numbers to 0:
>
> drivers/pci/endpoint/functions/pci-epf-vntb.c:vntb_epf_mw_set_trans() {
> ...
> ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
> ...
> }
>
> Does this incorrectly configure the BAR for function 0 when the virtual NTB
> device is bound to a non-zero endpoint function or virtual function?
>
> If the device is configured via configfs with a non-zero function, could this
> lead to silent failures in memory window translation or corrupt the ATU
> configuration of function 0?
>
> > +
> > +static int
> > +vntb_epf_mw_set_trans_group(struct ntb_dev *ndev, int pidx, int widx,
> > + dma_addr_t addr, resource_size_t size)
> > +{
> [ ... ]
> > + return pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> > + ntb->epf->vfunc_no, epf_bar);
> > +}
> > +
> > +static int vntb_epf_mw_clear_trans_group(struct ntb_dev *ndev, int pidx,
> > + int widx)
> > +{
> > + return 0;
> > +}
>
> [Severity: Critical]
> This is a pre-existing issue perpetuated here by the new group API, but does
> returning 0 without actually disabling the inbound hardware translations create a
> DMA use-after-free window?
>
> When an NTB client unmaps a memory window group and frees the backing memory,
> the hardware BAR mapping will remain active.
>
> If the local host frees this memory, won't the endpoint hardware continue
> translating incoming PCIe accesses to the freed physical address, allowing a
> remote peer to access arbitrary new allocations on the endpoint?
>
> [Severity: High]
> This is another pre-existing issue, but I noticed the vntb_pci_driver structure
> is missing a .remove callback:
>
> drivers/pci/endpoint/functions/pci-epf-vntb.c:
> static struct pci_driver vntb_pci_driver = {
> .name = "pci-vntb",
> .id_table = pci_vntb_table,
> .probe = pci_vntb_probe,
> };
>
> Without a .remove callback, ntb_unregister_device() is never called when the
> EPF device is unbound via configfs.
>
> Does this cause NTB device leaks and driver state use-after-free?
>
> If the endpoint function is unbound, won't the NTB clients remain active and
> bound to a device whose backing EPF structures, DMA masks, and hardware
> mappings have been destroyed?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=4
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (3 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:32 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands Koichiro Den
` (6 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
A packed MW group can use one outbound iATU region only when its EPC
address range is contiguous.
Allocate one EPC address range for the group and expose each logical MW
as a subrange. Singleton MWs keep separate allocations.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 26 +++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 0c72b907c289..aa8d304fe861 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -226,6 +226,15 @@ static int epf_ntb_get_mw_group(struct epf_ntb *ntb, unsigned int mw,
return 0;
}
+static u64 epf_ntb_mw_group_size(struct epf_ntb *ntb, unsigned int first,
+ unsigned int count)
+{
+ unsigned int last = first + count - 1;
+
+ return ntb->mw_layout[last].offset - ntb->mw_layout[first].offset +
+ ntb->mw_layout[last].size;
+}
+
/**
* epf_ntb_configure_mw() - Configure the Outbound Address Space for VHOST
* to access the memory window of HOST
@@ -891,6 +900,8 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
struct device *dev = &ntb->epf->dev;
u64 bar_size[BAR_5 + 1] = {};
bool bar_set[BAR_5 + 1] = {};
+ unsigned int count;
+ unsigned int j;
for (i = 0; i < ntb->num_mws; i++) {
size = ntb->mw_layout[i].size;
@@ -900,7 +911,6 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
}
for (i = 0; i < ntb->num_mws; i++) {
- size = ntb->mw_layout[i].size;
barno = ntb->mw_layout[i].barno;
if (bar_set[barno])
@@ -926,6 +936,9 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
alloc_vpci_mw:
/* Allocate EPC outbound memory windows to vpci vntb device */
+ count = ntb->packed_mws ? ntb->packed_mws : 1;
+ size = epf_ntb_mw_group_size(ntb, i, count);
+ /* Only the group head owns the allocation handle. */
ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
&ntb->vpci_mw_phy[i],
size);
@@ -935,6 +948,13 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
i++;
goto err_alloc_mem;
}
+
+ for (j = 1; j < count; j++)
+ ntb->vpci_mw_phy[i + j] = ntb->vpci_mw_phy[i] +
+ ntb->mw_layout[i + j].offset -
+ ntb->mw_layout[i].offset;
+
+ i += count - 1;
}
return ret;
@@ -953,6 +973,7 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
{
bool bar_cleared[BAR_5 + 1] = {};
enum pci_barno barno;
+ unsigned int count;
int i;
for (i = 0; i < num_mws; i++) {
@@ -968,10 +989,11 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
if (!ntb->vpci_mw_addr[i])
continue;
+ count = ntb->packed_mws ? ntb->packed_mws : 1;
pci_epc_mem_free_addr(ntb->epf->epc,
ntb->vpci_mw_phy[i],
ntb->vpci_mw_addr[i],
- ntb->mw_layout[i].size);
+ epf_ntb_mw_group_size(ntb, i, count));
ntb->vpci_mw_addr[i] = NULL;
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space
2026-08-03 18:04 ` [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space Koichiro Den
@ 2026-08-03 18:32 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:32 UTC (permalink / raw)
To: Koichiro Den; +Cc: ntb, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Packed outbound MW space is allocated but can never be mapped to the host, rendering it unusable.
--
--- Patch [5]: [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space ---
commit 1f91a957de194db5f5786ea1932b0222be01c3a2
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space
A packed MW group can use one outbound iATU region only when its EPC
address range is contiguous.
Allocate one EPC address range for the group and expose each logical MW
as a subrange. Singleton MWs keep separate allocations.
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 0c72b907c2896..aa8d304fe861f 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
[ ... ]
> @@ -926,6 +936,9 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
>
> alloc_vpci_mw:
> /* Allocate EPC outbound memory windows to vpci vntb device */
> + count = ntb->packed_mws ? ntb->packed_mws : 1;
> + size = epf_ntb_mw_group_size(ntb, i, count);
> + /* Only the group head owns the allocation handle. */
> ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
> &ntb->vpci_mw_phy[i],
> size);
[Severity: High]
Will the newly allocated packed MW space ever be mapped to the host, or does
it remain unusable?
In epf_ntb_configure_mw() (which handles VHOST CMD_CONFIGURE_MW requests from
the host), the driver explicitly rejects packed groups:
drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_configure_mw() {
...
ret = epf_ntb_get_mw_group(ntb, mw, NULL, &count);
if (ret)
return ret;
if (count > 1)
return -EOPNOTSUPP;
...
}
Since mapping of packed groups returns -EOPNOTSUPP without calling
pci_epc_map_addr(), won't the configuration fail when requested by the host?
If so, does the memory allocated here go unused, preventing host memory access
for packed MW groups?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=5
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (4 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 05/11] PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:28 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 07/11] NTB: epf: Implement MW group translation callbacks Koichiro Den
` (5 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
Packed logical MWs share one contiguous EPC address range. Their
outbound translations can therefore use one mapping as well.
Add commands to map or unmap a complete group in one operation. Keep the
existing commands for singleton MWs.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index aa8d304fe861..526cc31d9435 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -56,6 +56,8 @@ static struct workqueue_struct *kpcintb_workqueue;
#define COMMAND_TEARDOWN_MW 4
#define COMMAND_LINK_UP 5
#define COMMAND_LINK_DOWN 6
+#define COMMAND_CONFIGURE_MW_GROUP 7
+#define COMMAND_TEARDOWN_MW_GROUP 8
#define COMMAND_STATUS_OK 1
#define COMMAND_STATUS_ERROR 2
@@ -319,6 +321,50 @@ static int epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
return 0;
}
+static int epf_ntb_configure_mw_group(struct epf_ntb *ntb, u32 mw)
+{
+ unsigned int first, count;
+ u64 addr, size;
+ int ret;
+
+ ret = epf_ntb_get_mw_group(ntb, mw, &first, &count);
+ if (ret)
+ return ret;
+ if (count <= 1)
+ return -EOPNOTSUPP;
+ if (mw != first)
+ return -EINVAL;
+
+ addr = ntb->reg->addr;
+ size = ntb->reg->size;
+ if (!IS_ALIGNED(addr, SZ_4K) ||
+ size != epf_ntb_mw_group_size(ntb, first, count))
+ return -EINVAL;
+
+ return pci_epc_map_addr(ntb->epf->epc, ntb->epf->func_no,
+ ntb->epf->vfunc_no, ntb->vpci_mw_phy[first],
+ addr, size);
+}
+
+static int epf_ntb_teardown_mw_group(struct epf_ntb *ntb, u32 mw)
+{
+ unsigned int first, count;
+ int ret;
+
+ ret = epf_ntb_get_mw_group(ntb, mw, &first, &count);
+ if (ret)
+ return ret;
+ if (count <= 1)
+ return -EOPNOTSUPP;
+ if (mw != first)
+ return -EINVAL;
+
+ pci_epc_unmap_addr(ntb->epf->epc, ntb->epf->func_no,
+ ntb->epf->vfunc_no, ntb->vpci_mw_phy[first]);
+
+ return 0;
+}
+
/**
* epf_ntb_cmd_handler() - Handle commands provided by the NTB HOST
* @work: work_struct for the epf_ntb_epc
@@ -378,6 +424,14 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
ret = epf_ntb_teardown_mw(ntb, argument);
ctrl->command_status = ret ? COMMAND_STATUS_ERROR : COMMAND_STATUS_OK;
break;
+ case COMMAND_CONFIGURE_MW_GROUP:
+ ret = epf_ntb_configure_mw_group(ntb, argument);
+ ctrl->command_status = ret ? COMMAND_STATUS_ERROR : COMMAND_STATUS_OK;
+ break;
+ case COMMAND_TEARDOWN_MW_GROUP:
+ ret = epf_ntb_teardown_mw_group(ntb, argument);
+ ctrl->command_status = ret ? COMMAND_STATUS_ERROR : COMMAND_STATUS_OK;
+ break;
case COMMAND_LINK_UP:
ntb->linkup = true;
ret = epf_ntb_link_up(ntb, true);
@@ -388,6 +442,8 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
goto reset_handler;
case COMMAND_LINK_DOWN:
ntb->linkup = false;
+ if (ntb->packed_mws)
+ epf_ntb_teardown_mw_group(ntb, 0);
ret = epf_ntb_link_up(ntb, false);
if (ret < 0)
ctrl->command_status = COMMAND_STATUS_ERROR;
@@ -976,6 +1032,9 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
unsigned int count;
int i;
+ if (ntb->packed_mws)
+ epf_ntb_teardown_mw_group(ntb, 0);
+
for (i = 0; i < num_mws; i++) {
barno = ntb->mw_layout[i].barno;
if (!bar_cleared[barno]) {
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands
2026-08-03 18:04 ` [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands Koichiro Den
@ 2026-08-03 18:28 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:28 UTC (permalink / raw)
To: Koichiro Den; +Cc: ntb, linux-pci
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The cleanup path `epf_ntb_mw_bar_clear()` unconditionally calls `epf_ntb_teardown_mw_group(ntb, 0)` for packed MWs, leading to unmapping of physical address 0 if initialization fails.
- [High] The automatic ATU mapping teardown silently fails when `packed_mws == 1`, leading to ATU resource leaks and stale memory mappings.
Pre-existing issues:
- [High] Failure to unmap outbound ATU mappings for singleton (non-packed) memory windows during driver unbind or error cleanup leaves stale DMA translations active.
- [High] The command handler `epf_ntb_cmd_handler` reads `ctrl->command` and then `ctrl->argument` without a read memory barrier, risking processing of stale arguments.
--
--- Patch [6]: [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands ---
commit 6b0d64af683bb36f33f200f34cffed00c1c6d648
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Add outbound MW group commands
Packed logical MWs share one contiguous EPC address range. Their
outbound translations can therefore use one mapping as well.
Add commands to map or unmap a complete group in one operation. Keep the
existing commands for singleton MWs.
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index aa8d304fe861f..526cc31d94351 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
[ ... ]
> @@ -319,6 +321,50 @@ static int epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
[ ... ]
> +static int epf_ntb_teardown_mw_group(struct epf_ntb *ntb, u32 mw)
> +{
> + unsigned int first, count;
> + int ret;
> +
> + ret = epf_ntb_get_mw_group(ntb, mw, &first, &count);
> + if (ret)
> + return ret;
> + if (count <= 1)
> + return -EOPNOTSUPP;
[Severity: High]
Does returning -EOPNOTSUPP here cause an ATU resource leak when a single
packed memory window is configured?
When packed_mws == 1, this returns -EOPNOTSUPP, which is then silently ignored
by callers like epf_ntb_mw_bar_clear() and COMMAND_LINK_DOWN.
This leaves the ATU hardware mappings active while the underlying physical EPC
memory is freed, which could lead to memory corruption if the physical address
is reallocated.
> + if (mw != first)
> + return -EINVAL;
> +
> + pci_epc_unmap_addr(ntb->epf->epc, ntb->epf->func_no,
> + ntb->epf->vfunc_no, ntb->vpci_mw_phy[first]);
> +
> + return 0;
> +}
[ ... ]
> @@ -378,6 +424,14 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
[Severity: High]
This is a pre-existing issue, but does epf_ntb_cmd_handler() risk processing
stale arguments due to missing memory barriers?
The handler reads ctrl->command, checks it, and then reads ctrl->argument from
DMA-coherent memory.
Without a read memory barrier (e.g., smp_rmb()), control dependencies do not
prevent speculative reads on architectures with weak memory ordering.
Could the CPU speculatively read a stale argument before the command is fully
resolved?
> ret = epf_ntb_teardown_mw(ntb, argument);
> ctrl->command_status = ret ? COMMAND_STATUS_ERROR : COMMAND_STATUS_OK;
> break;
[ ... ]
> @@ -976,6 +1032,9 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
> unsigned int count;
> int i;
>
> + if (ntb->packed_mws)
> + epf_ntb_teardown_mw_group(ntb, 0);
> +
[Severity: High]
Is it possible to unmap physical address 0 if initialization fails?
If pci_epc_set_bar() fails for i=0 during epf_ntb_mw_bar_init(), it branches
to the error path and calls epf_ntb_mw_bar_clear(ntb, 0).
Because ntb->packed_mws is true, it unconditionally calls
epf_ntb_teardown_mw_group(ntb, 0).
Since the memory allocation was never reached for i=0, ntb->vpci_mw_phy[0] is
still 0, causing pci_epc_unmap_addr() to operate on uninitialized physical
address 0.
> for (i = 0; i < num_mws; i++) {
> barno = ntb->mw_layout[i].barno;
[Severity: High]
This is a pre-existing issue, but does this loop fail to unmap outbound ATU
mappings for standard (non-packed) memory windows?
During driver unbind or error cleanup, this loop frees the outbound EPC memory,
but it lacks a call to pci_epc_unmap_addr() for unpacked memory windows.
The physical EPC memory is freed while the ATU mapping remains active in
hardware, which could lead to silent memory corruption if re-allocated to
another endpoint function.
> if (!bar_cleared[barno]) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=6
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 07/11] NTB: epf: Implement MW group translation callbacks
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (5 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 06/11] PCI: endpoint: pci-epf-vntb: Add outbound MW group commands Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:29 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 08/11] NTB: perf: Reject grouped memory windows Koichiro Den
` (4 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
Connect the NTB group callbacks to the version 1 group commands so
ntb_hw_epf can program packed MWs. Singleton MWs keep using the legacy
commands.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/ntb/hw/epf/ntb_hw_epf.c | 41 +++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
index 46f37c2dea7b..8b00550eea04 100644
--- a/drivers/ntb/hw/epf/ntb_hw_epf.c
+++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
@@ -20,6 +20,8 @@
#define CMD_TEARDOWN_MW 4
#define CMD_LINK_UP 5
#define CMD_LINK_DOWN 6
+#define CMD_CONFIGURE_MW_GROUP 7
+#define CMD_TEARDOWN_MW_GROUP 8
#define NTB_EPF_ARGUMENT 0x4
#define MSIX_ENABLE BIT(16)
@@ -509,6 +511,43 @@ static int ntb_epf_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
return 0;
}
+static int
+ntb_epf_mw_set_trans_group(struct ntb_dev *ntb, int pidx, int widx,
+ dma_addr_t addr, resource_size_t size)
+{
+ struct ntb_epf_dev *ndev = ntb_ndev(ntb);
+ resource_size_t group_size;
+
+ if (ndev->ctrl_version < NTB_EPF_CTRL_V1)
+ return -EOPNOTSUPP;
+ if (pidx != NTB_DEF_PEER_IDX || widx)
+ return -EINVAL;
+
+ group_size = (resource_size_t)ndev->mw_size * ndev->mw_count;
+ if (!IS_ALIGNED(addr, SZ_4K) || size != group_size)
+ return -EINVAL;
+
+ writel(lower_32_bits(addr), ndev->ctrl_reg + NTB_EPF_LOWER_ADDR);
+ writel(upper_32_bits(addr), ndev->ctrl_reg + NTB_EPF_UPPER_ADDR);
+ writel(lower_32_bits(size), ndev->ctrl_reg + NTB_EPF_LOWER_SIZE);
+ writel(upper_32_bits(size), ndev->ctrl_reg + NTB_EPF_UPPER_SIZE);
+
+ return ntb_epf_send_command(ndev, CMD_CONFIGURE_MW_GROUP, widx);
+}
+
+static int ntb_epf_mw_clear_trans_group(struct ntb_dev *ntb, int pidx,
+ int widx)
+{
+ struct ntb_epf_dev *ndev = ntb_ndev(ntb);
+
+ if (ndev->ctrl_version < NTB_EPF_CTRL_V1)
+ return -EOPNOTSUPP;
+ if (pidx != NTB_DEF_PEER_IDX || widx)
+ return -EINVAL;
+
+ return ntb_epf_send_command(ndev, CMD_TEARDOWN_MW_GROUP, widx);
+}
+
static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
dma_addr_t addr, resource_size_t size)
{
@@ -648,6 +687,8 @@ static const struct ntb_dev_ops ntb_epf_ops = {
.db_vector_mask = ntb_epf_db_vector_mask,
.db_set_mask = ntb_epf_db_set_mask,
.mw_get_trans_group = ntb_epf_mw_get_trans_group,
+ .mw_set_trans_group = ntb_epf_mw_set_trans_group,
+ .mw_clear_trans_group = ntb_epf_mw_clear_trans_group,
.mw_set_trans = ntb_epf_mw_set_trans,
.mw_clear_trans = ntb_epf_mw_clear_trans,
.peer_mw_get_addr = ntb_epf_peer_mw_get_addr,
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 07/11] NTB: epf: Implement MW group translation callbacks
2026-08-03 18:04 ` [PATCH 07/11] NTB: epf: Implement MW group translation callbacks Koichiro Den
@ 2026-08-03 18:29 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:29 UTC (permalink / raw)
To: Koichiro Den; +Cc: ntb, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Unprotected concurrent modification of shared hardware control registers before acquiring cmd_lock.
Pre-existing issues:
- [High] The return values of `ntb_epf_send_command` are silently ignored in legacy MW operations.
--
--- Patch [7]: [PATCH 07/11] NTB: epf: Implement MW group translation callbacks ---
commit 1b85afed97e44da7b70fd9dff32c501ab6f91e2d
Author: Koichiro Den <den@valinux.co.jp>
NTB: epf: Implement MW group translation callbacks
This commit connects the NTB group callbacks to the version 1 group commands
so ntb_hw_epf can program packed MWs. Singleton MWs keep using the legacy
commands.
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index 46f37c2dea7b2..8b00550eea04d 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -509,6 +511,43 @@ static int ntb_epf_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
> return 0;
> }
>
> +static int
> +ntb_epf_mw_set_trans_group(struct ntb_dev *ntb, int pidx, int widx,
> + dma_addr_t addr, resource_size_t size)
> +{
[ ... ]
> + writel(lower_32_bits(addr), ndev->ctrl_reg + NTB_EPF_LOWER_ADDR);
> + writel(upper_32_bits(addr), ndev->ctrl_reg + NTB_EPF_UPPER_ADDR);
> + writel(lower_32_bits(size), ndev->ctrl_reg + NTB_EPF_LOWER_SIZE);
> + writel(upper_32_bits(size), ndev->ctrl_reg + NTB_EPF_UPPER_SIZE);
[Severity: High]
Can concurrent writes to these registers corrupt the hardware state?
Looking at ntb_epf_mw_set_trans_group(), the parameter registers are modified
outside the protection of ndev->cmd_lock, which is only acquired later inside
ntb_epf_send_command().
If a user or client driver concurrently configures different memory windows,
would the register writes interleave, potentially granting the peer device
DMA access to unintended memory regions?
This identical pattern also exists in ntb_epf_mw_set_trans().
> +
> + return ntb_epf_send_command(ndev, CMD_CONFIGURE_MW_GROUP, widx);
> +}
[ ... ]
> @@ -648,6 +687,8 @@ static const struct ntb_dev_ops ntb_epf_ops = {
> .db_vector_mask = ntb_epf_db_vector_mask,
> .db_set_mask = ntb_epf_db_set_mask,
> .mw_get_trans_group = ntb_epf_mw_get_trans_group,
> + .mw_set_trans_group = ntb_epf_mw_set_trans_group,
> + .mw_clear_trans_group = ntb_epf_mw_clear_trans_group,
> .mw_set_trans = ntb_epf_mw_set_trans,
> .mw_clear_trans = ntb_epf_mw_clear_trans,
[Severity: High]
This is a pre-existing issue, but do the legacy transaction functions mask
hardware failures?
Looking at ntb_epf_mw_clear_trans() in drivers/ntb/hw/epf/ntb_hw_epf.c:
ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx);
if (ret)
dev_err(dev, "Failed to teardown memory window\n");
The return value of ntb_epf_send_command() isn't captured, so ret remains
zero and the error check becomes dead code.
Could this hide a teardown failure, causing the local host to free memory
while the peer host retains active DMA access to it?
Similarly, ntb_epf_mw_set_trans() discards the return value of
ntb_epf_send_command() and unconditionally returns 0.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=7
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 08/11] NTB: perf: Reject grouped memory windows
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (6 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 07/11] NTB: epf: Implement MW group translation callbacks Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:25 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 09/11] NTB/msi: Require a singleton memory window Koichiro Den
` (3 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
NTB perf configures one MW per peer through the per-MW translation API.
It cannot configure one member of a larger translation group.
Reject such layouts at probe. Otherwise, link setup cannot complete and
a performance test waits indefinitely.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/ntb/test/ntb_perf.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index dfd175f79e8f..50bfbec1e251 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -1436,7 +1436,7 @@ static int perf_setup_peer_mw(struct perf_peer *peer)
static int perf_init_peers(struct perf_ctx *perf)
{
struct perf_peer *peer;
- int pidx, lport, ret;
+ int first, count, pidx, lport, ret;
lport = ntb_port_number(perf->ntb);
perf->gidx = -1;
@@ -1469,7 +1469,19 @@ static int perf_init_peers(struct perf_ctx *perf)
}
for (pidx = 0; pidx < perf->pcnt; pidx++) {
- ret = perf_setup_peer_mw(&perf->peers[pidx]);
+ peer = &perf->peers[pidx];
+ ret = ntb_mw_get_trans_group(perf->ntb, peer->pidx,
+ peer->gidx, &first, &count);
+ if (ret)
+ return ret;
+ if (first != peer->gidx || count != 1) {
+ dev_err(&perf->ntb->dev,
+ "Peer %d MW %d is not a standalone window\n",
+ peer->pidx, peer->gidx);
+ return -EOPNOTSUPP;
+ }
+
+ ret = perf_setup_peer_mw(peer);
if (ret)
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 08/11] NTB: perf: Reject grouped memory windows
2026-08-03 18:04 ` [PATCH 08/11] NTB: perf: Reject grouped memory windows Koichiro Den
@ 2026-08-03 18:25 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:25 UTC (permalink / raw)
To: Koichiro Den; +Cc: ntb, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Mismatch in memory window indices (`perf->gidx` vs `peer->gidx`) breaks test communication on multi-port configurations.
- [High] Multiple test threads executing concurrently map memory and overwrite a shared `peer->dma_dst_addr`, leading to DMA mapping leaks, IOMMU faults, and double-unmapping.
--
--- Patch [8]: [PATCH 08/11] NTB: perf: Reject grouped memory windows ---
commit 8a37bed871c2fb90c1fc72578c46b05070e8ad53
Author: Koichiro Den <den@valinux.co.jp>
NTB: perf: Reject grouped memory windows
NTB perf configures one MW per peer through the per-MW translation API.
It cannot configure one member of a larger translation group.
Reject such layouts at probe.
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index dfd175f79e8f0..50bfbec1e251e 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
[ ... ]
> @@ -1469,7 +1469,19 @@ static int perf_init_peers(struct perf_ctx *perf)
> }
>
> for (pidx = 0; pidx < perf->pcnt; pidx++) {
> - ret = perf_setup_peer_mw(&perf->peers[pidx]);
> + peer = &perf->peers[pidx];
> + ret = ntb_mw_get_trans_group(perf->ntb, peer->pidx,
> + peer->gidx, &first, &count);
> + if (ret)
> + return ret;
> + if (first != peer->gidx || count != 1) {
> + dev_err(&perf->ntb->dev,
> + "Peer %d MW %d is not a standalone window\n",
> + peer->pidx, peer->gidx);
> + return -EOPNOTSUPP;
> + }
> +
> + ret = perf_setup_peer_mw(peer);
[Severity: High]
This is a pre-existing issue, but does perf_setup_peer_mw() incorrectly mix
index variables on multi-port configurations?
Looking at perf_setup_peer_mw(), it maps the physical address of the
outbound window using the local port's index (perf->gidx):
ret = ntb_peer_mw_get_addr(perf->ntb, perf->gidx, &phys_addr,
&peer->outbuf_size);
However, perf_setup_outbuf() later configures the outbound translation using
the peer's index (peer->gidx):
ret = ntb_peer_mw_set_trans(perf->ntb, peer->pidx, peer->gidx,
peer->outbuf_xlat, peer->outbuf_size);
When perf->gidx and peer->gidx differ, this mismatch causes writes to the
mapped memory to go to an unconfigured window.
The same index mismatch occurs for inbound windows between ntb_mw_get_align()
and ntb_mw_set_trans().
Could we use a consistent index?
[Severity: High]
This is a pre-existing issue, but can multiple concurrent test threads
overwrite peer->dma_dst_addr and leak DMA allocations?
When a user configures multiple test threads, they all execute
perf_init_test() concurrently without serialization.
Because they share the same perf->test_peer structure, they each call
dma_map_resource() and overwrite peer->dma_dst_addr:
drivers/ntb/test/ntb_perf.c:perf_init_test() {
...
peer->dma_dst_addr =
dma_map_resource(pthr->dma_chan->device->dev,
peer->out_phys_addr, peer->outbuf_size,
DMA_FROM_DEVICE, 0);
...
}
Later, threads may read an IOVA mapped for a completely different DMA device
in perf_copy_chunk():
dst_dma_addr = peer->dma_dst_addr + (dst_vaddr - vbase);
During teardown, all threads call dma_unmap_resource() on the exact same
pointer in perf_clear_test(), causing a double-unmap:
dma_unmap_resource(pthr->dma_chan->device->dev,
pthr->perf->test_peer->dma_dst_addr,
pthr->perf->test_peer->outbuf_size,
DMA_FROM_DEVICE, 0);
Does this sequence need additional locking or per-thread DMA mapping tracking?
> if (ret)
> return ret;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=8
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/11] NTB/msi: Require a singleton memory window
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (7 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 08/11] NTB: perf: Reject grouped memory windows Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:23 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups Koichiro Den
` (2 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
NTB MSI reserves the last MW for each peer and programs it
independently. A member of a larger translation group cannot be reserved
on its own.
Reject such layouts during initialization. This lets ntb_transport keep
the whole group for data and fall back to doorbells.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/ntb/msi.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/ntb/msi.c b/drivers/ntb/msi.c
index 6817d504c12a..029edcf40078 100644
--- a/drivers/ntb/msi.c
+++ b/drivers/ntb/msi.c
@@ -24,7 +24,8 @@ struct ntb_msi {
* the peer memory windows.
*
* This function reserves the last N outbound memory windows (where N
- * is the number of peers).
+ * is the number of peers). The corresponding inbound memory windows must
+ * each form a singleton translation group.
*
* Return: Zero on success, otherwise a negative error number.
*/
@@ -33,6 +34,7 @@ int ntb_msi_init(struct ntb_dev *ntb,
{
phys_addr_t mw_phys_addr;
resource_size_t mw_size;
+ int first, count;
int peer_widx;
int peers;
int ret;
@@ -42,6 +44,21 @@ int ntb_msi_init(struct ntb_dev *ntb,
if (peers <= 0)
return -EINVAL;
+ if (ntb->ops->mw_get_trans_group) {
+ for (i = 0; i < peers; i++) {
+ peer_widx = ntb_peer_highest_mw_idx(ntb, i);
+ if (peer_widx < 0)
+ return peer_widx;
+
+ ret = ntb_mw_get_trans_group(ntb, i, peer_widx, &first,
+ &count);
+ if (ret)
+ return ret;
+ if (first != peer_widx || count != 1)
+ return -EOPNOTSUPP;
+ }
+ }
+
ntb->msi = devm_kzalloc(&ntb->dev, struct_size(ntb->msi, peer_mws, peers),
GFP_KERNEL);
if (!ntb->msi)
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 09/11] NTB/msi: Require a singleton memory window
2026-08-03 18:04 ` [PATCH 09/11] NTB/msi: Require a singleton memory window Koichiro Den
@ 2026-08-03 18:23 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:23 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci, ntb
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Error Handling Bug: incorrect memory window index cleared on rollback
- [High] Integer Underflow / Out-of-Bounds Read: Negative index passed to hardware driver
--
--- Patch [9]: [PATCH 09/11] NTB/msi: Require a singleton memory window ---
commit ed42871f4aa60b4cc0fd8192c14055c9e382384b
Author: Koichiro Den <den@valinux.co.jp>
NTB/msi: Require a singleton memory window
This commit adds a check in ntb_msi_init() to reject NTB MSI layouts
where the outbound memory window does not form a singleton translation
group. This ensures the transport driver can retain non-singleton groups
for data use.
> diff --git a/drivers/ntb/msi.c b/drivers/ntb/msi.c
> index 6817d504c12ac..029edcf400784 100644
> --- a/drivers/ntb/msi.c
> +++ b/drivers/ntb/msi.c
[ ... ]
> @@ -42,6 +44,21 @@ int ntb_msi_init(struct ntb_dev *ntb,
> if (peers <= 0)
> return -EINVAL;
>
> + if (ntb->ops->mw_get_trans_group) {
> + for (i = 0; i < peers; i++) {
> + peer_widx = ntb_peer_highest_mw_idx(ntb, i);
> + if (peer_widx < 0)
> + return peer_widx;
> +
> + ret = ntb_mw_get_trans_group(ntb, i, peer_widx, &first,
> + &count);
> + if (ret)
> + return ret;
> + if (first != peer_widx || count != 1)
> + return -EOPNOTSUPP;
> + }
> + }
> +
> ntb->msi = devm_kzalloc(&ntb->dev, struct_size(ntb->msi, peer_mws, peers),
> GFP_KERNEL);
> if (!ntb->msi)
[Severity: High]
This is a pre-existing issue, but does this code properly prevent a negative
index from being passed to the hardware driver?
Further down in drivers/ntb/msi.c:ntb_msi_init():
for (i = 0; i < peers; i++) {
peer_widx = ntb_peer_mw_count(ntb) - 1 - i;
ret = ntb_peer_mw_get_addr(ntb, peer_widx, &mw_phys_addr,
&mw_size);
If the hardware exposes fewer outbound memory windows (ntb_peer_mw_count(ntb))
than there are peers (ntb_peer_port_count(ntb)), won't the subtraction
underflow and make peer_widx negative?
Hardware drivers might fail to validate this negative index, resulting in
out-of-bounds array accesses and likely causing a kernel panic during device
probe.
[Severity: High]
This is also a pre-existing issue, but does the error handling in
ntb_msi_setup_mws() clear the correct memory window index on rollback?
In drivers/ntb/msi.c:ntb_msi_setup_mws():
error_out:
for (i = 0; i < peer; i++) {
peer_widx = ntb_peer_highest_mw_idx(ntb, peer);
if (peer_widx < 0)
continue;
ntb_mw_clear_trans(ntb, i, peer_widx);
}
The unroll loop queries ntb_peer_highest_mw_idx(ntb, peer) using the failing
peer index instead of the loop iterator i.
If peers have different memory window counts or indices, won't this leave an
active memory window mapping on the NTB hardware for previously configured
peers, or potentially clear the wrong window?
If the driver later reuses the uncleared memory window for regular data
transfers, remote memory writes could be interpreted as MSIs, leading to
hardware state corruption and spurious interrupts.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=9
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (8 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 09/11] NTB/msi: Require a singleton memory window Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:40 ` sashiko-bot
2026-08-03 18:04 ` [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs Koichiro Den
2026-08-03 23:00 ` [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Randy Dunlap
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
ntb_transport currently allocates and programs each MW on its own. MWs
in a translation group instead need one contiguous allocation and must
be set or cleared as one unit.
Query each group, allocate its backing once, and use the group set and
clear operations. Reject a scratchpad limit that would split a group.
Singleton MWs keep the existing path.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
drivers/ntb/ntb_transport.c | 127 +++++++++++++++++++++++++++++++-----
1 file changed, 109 insertions(+), 18 deletions(-)
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index f59f926d4bfa..2c7728898890 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -56,6 +56,7 @@
#include <linux/interrupt.h>
#include <linux/kthread.h>
#include <linux/module.h>
+#include <linux/overflow.h>
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/seq_file.h>
@@ -757,22 +758,65 @@ static void ntb_transport_msi_desc_changed(void *data)
ntb_peer_db_set(nt->ndev, nt->msi_db_mask);
}
+static int ntb_mw_get_group(struct ntb_transport_ctx *nt, unsigned int widx,
+ unsigned int *first, unsigned int *count)
+{
+ int gidx, gcount;
+ int rc;
+
+ rc = ntb_mw_get_trans_group(nt->ndev, PIDX, widx, &gidx, &gcount);
+ if (rc)
+ return rc;
+ if (gidx < 0 || gcount <= 0)
+ return -EINVAL;
+
+ *first = gidx;
+ *count = gcount;
+ if (*first >= nt->mw_count || *count > nt->mw_count - *first ||
+ widx < *first || widx >= *first + *count)
+ return -EINVAL;
+
+ return 0;
+}
+
static void ntb_free_mw(struct ntb_transport_ctx *nt, int num_mw)
{
struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
struct device *dma_dev = ntb_get_dma_dev(nt->ndev);
+ dma_addr_t original_dma_addr;
+ void *alloc_addr;
+ size_t alloc_size;
+ unsigned int first, count;
+ unsigned int i;
+ int rc;
if (!mw->virt_addr)
return;
- ntb_mw_clear_trans(nt->ndev, PIDX, num_mw);
- dma_free_attrs(dma_dev, mw->alloc_size, mw->alloc_addr,
- mw->original_dma_addr, DMA_ATTR_FORCE_CONTIGUOUS);
- mw->xlat_size = 0;
- mw->buff_size = 0;
- mw->alloc_size = 0;
- mw->alloc_addr = NULL;
- mw->virt_addr = NULL;
+ rc = ntb_mw_get_group(nt, num_mw, &first, &count);
+ if (rc || num_mw != first)
+ return;
+
+ if (count > 1)
+ ntb_mw_clear_trans_group(nt->ndev, PIDX, first);
+ else
+ ntb_mw_clear_trans(nt->ndev, PIDX, first);
+
+ alloc_addr = mw->alloc_addr;
+ alloc_size = mw->alloc_size;
+ original_dma_addr = mw->original_dma_addr;
+ for (i = first; i < first + count; i++) {
+ mw = &nt->mw_vec[i];
+ mw->xlat_size = 0;
+ mw->buff_size = 0;
+ mw->alloc_size = 0;
+ mw->alloc_addr = NULL;
+ mw->virt_addr = NULL;
+ mw->dma_addr = 0;
+ }
+
+ dma_free_attrs(dma_dev, alloc_size, alloc_addr, original_dma_addr,
+ DMA_ATTR_FORCE_CONTIGUOUS);
}
static int ntb_alloc_mw_buffer(struct ntb_transport_mw *mw,
@@ -838,9 +882,11 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
{
struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
struct device *dma_dev = ntb_get_dma_dev(nt->ndev);
- size_t xlat_size, buff_size;
+ size_t xlat_size, buff_size, group_size;
resource_size_t xlat_align;
resource_size_t xlat_align_size;
+ unsigned int first, count;
+ unsigned int i;
int rc;
if (!size)
@@ -861,15 +907,30 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
if (mw->buff_size)
ntb_free_mw(nt, num_mw);
+ rc = ntb_mw_get_group(nt, num_mw, &first, &count);
+ if (rc)
+ return rc;
+ if (count > 1) {
+ if (first != num_mw)
+ return -EINVAL;
+ if (xlat_size != buff_size)
+ return -EINVAL;
+ if (check_mul_overflow(xlat_size, count, &group_size))
+ return -EOVERFLOW;
+ } else {
+ group_size = buff_size;
+ }
+
/* Alloc memory for receiving data. Must be aligned */
- mw->xlat_size = xlat_size;
- mw->buff_size = buff_size;
- mw->alloc_size = buff_size;
+ mw->buff_size = group_size;
+ mw->alloc_size = group_size;
rc = ntb_alloc_mw_buffer(mw, dma_dev, xlat_align);
if (rc) {
- mw->alloc_size *= 2;
- rc = ntb_alloc_mw_buffer(mw, dma_dev, xlat_align);
+ if (check_mul_overflow(mw->alloc_size, 2, &mw->alloc_size))
+ rc = -EOVERFLOW;
+ else
+ rc = ntb_alloc_mw_buffer(mw, dma_dev, xlat_align);
if (rc) {
dev_err(dma_dev,
"Unable to alloc aligned MW buff\n");
@@ -880,15 +941,28 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
}
}
+ for (i = 0; i < count; i++) {
+ mw = &nt->mw_vec[first + i];
+ mw->xlat_size = xlat_size;
+ mw->buff_size = buff_size;
+ mw->virt_addr = (u8 *)nt->mw_vec[first].virt_addr +
+ i * xlat_size;
+ mw->dma_addr = nt->mw_vec[first].dma_addr + i * xlat_size;
+ }
+
/* Notify HW the memory location of the receive buffer */
- rc = ntb_mw_set_trans(nt->ndev, PIDX, num_mw, mw->dma_addr,
- mw->xlat_size);
+ if (count > 1)
+ rc = ntb_mw_set_trans_group(nt->ndev, PIDX, first,
+ nt->mw_vec[first].dma_addr,
+ group_size);
+ else
+ rc = ntb_mw_set_trans(nt->ndev, PIDX, num_mw, mw->dma_addr,
+ mw->xlat_size);
if (rc) {
dev_err(dma_dev, "Unable to set mw%d translation", num_mw);
- ntb_free_mw(nt, num_mw);
+ ntb_free_mw(nt, first);
return -EIO;
}
-
return 0;
}
@@ -1238,6 +1312,7 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
struct ntb_transport_ctx *nt;
struct ntb_transport_mw *mw;
unsigned int mw_count, qp_count, spad_count, max_mw_count_for_spads;
+ int group_first, group_count;
u64 qp_bitmap;
int node;
int rc, i;
@@ -1291,6 +1366,22 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
max_mw_count_for_spads = (spad_count - MW0_SZ_HIGH) / 2;
nt->mw_count = min(mw_count, max_mw_count_for_spads);
+ if (nt->mw_count) {
+ rc = ntb_mw_get_trans_group(ndev, PIDX, nt->mw_count - 1,
+ &group_first, &group_count);
+ if (rc)
+ goto err;
+
+ if (group_first < 0 || group_count <= 0 ||
+ (unsigned int)group_first >= nt->mw_count ||
+ (unsigned int)group_count !=
+ nt->mw_count - (unsigned int)group_first) {
+ dev_err(&ndev->dev,
+ "Scratchpad limit splits an MW translation group\n");
+ rc = -EOPNOTSUPP;
+ goto err;
+ }
+ }
nt->msi_spad_offset = nt->mw_count * 2 + MW0_SZ_HIGH;
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups
2026-08-03 18:04 ` [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups Koichiro Den
@ 2026-08-03 18:40 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:40 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci, ntb
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing work cancellation in probe error path leads to Use-After-Free.
--
--- Patch [10]: [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups ---
commit 92a31c981341f2c994839699e7b4fa179c125291
Author: Koichiro Den <den@valinux.co.jp>
NTB: ntb_transport: Use atomic MW translation groups
This commit modifies the ntb_transport driver to allocate and program MWs
in a translation group as one contiguous unit. It queries each group,
allocates its backing once, and uses group set and clear operations.
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f59f926d4bfaa..2c7728898890c 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
[ ... ]
> @@ -1291,6 +1366,22 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>
> max_mw_count_for_spads = (spad_count - MW0_SZ_HIGH) / 2;
> nt->mw_count = min(mw_count, max_mw_count_for_spads);
> + if (nt->mw_count) {
> + rc = ntb_mw_get_trans_group(ndev, PIDX, nt->mw_count - 1,
> + &group_first, &group_count);
> + if (rc)
> + goto err;
> +
> + if (group_first < 0 || group_count <= 0 ||
> + (unsigned int)group_first >= nt->mw_count ||
> + (unsigned int)group_count !=
> + nt->mw_count - (unsigned int)group_first) {
> + dev_err(&ndev->dev,
> + "Scratchpad limit splits an MW translation group\n");
> + rc = -EOPNOTSUPP;
> + goto err;
> + }
> + }
>
> nt->msi_spad_offset = nt->mw_count * 2 + MW0_SZ_HIGH;
>
[Severity: High]
This is a pre-existing issue, but does the probe error path need to cancel
link_work?
Looking at ntb_transport_probe(), ntb_set_ctx() registers device callbacks:
rc = ntb_set_ctx(ndev, nt, &ntb_transport_ops);
if (rc)
goto err2;
INIT_LIST_HEAD(&nt->client_devs);
rc = ntb_bus_init(nt);
if (rc)
goto err3;
If an interrupt fires immediately after ntb_set_ctx() registers the
callbacks, the link_event callback could queue nt->link_work on the system
workqueue.
If ntb_bus_init() subsequently fails, the error path jumps to err3 and
frees the context without explicitly canceling the work via
cancel_delayed_work_sync():
err3:
ntb_clear_ctx(ndev);
err2:
kfree(nt->qp_vec);
err1:
while (i--) {
mw = &nt->mw_vec[i];
iounmap(mw->vbase);
}
kfree(nt->mw_vec);
err:
kfree(nt);
Could this leave link_work queued on freed memory, leading to a
use-after-free when the work queue executes it?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=10
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (9 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups Koichiro Den
@ 2026-08-03 18:04 ` Koichiro Den
2026-08-03 18:40 ` sashiko-bot
2026-08-03 23:00 ` [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Randy Dunlap
11 siblings, 1 reply; 26+ messages in thread
From: Koichiro Den @ 2026-08-03 18:04 UTC (permalink / raw)
To: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
The packed MW data path is now complete. Add packed_mws to enable it and
allow up to 16 logical MWs when packing is selected.
Use mw1 as the aggregate BAR size and restrict the other MW size and BAR
attributes while packing is enabled.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Documentation/PCI/endpoint/pci-vntb-howto.rst | 16 ++++-
drivers/pci/endpoint/functions/pci-epf-vntb.c | 60 ++++++++++++++++++-
2 files changed, 74 insertions(+), 2 deletions(-)
diff --git a/Documentation/PCI/endpoint/pci-vntb-howto.rst b/Documentation/PCI/endpoint/pci-vntb-howto.rst
index 3679f5c30254..8c74a39f1aee 100644
--- a/Documentation/PCI/endpoint/pci-vntb-howto.rst
+++ b/Documentation/PCI/endpoint/pci-vntb-howto.rst
@@ -92,7 +92,7 @@ attributes that can be configured by the user::
# ls functions/pci_epf_vntb/func1/pci_epf_vntb.0/
ctrl_bar db_count mw1_bar mw2_bar mw3_bar mw4_bar spad_count
db_bar mw1 mw2 mw3 mw4 num_mws vbus_number
- vntb_vid vntb_pid
+ vntb_vid vntb_pid packed_mws
A sample configuration for NTB function is given below::
@@ -105,6 +105,20 @@ By default, each construct is assigned a BAR, as needed and in order.
Should a specific BAR setup be required by the platform, BAR may be assigned
to each construct using the related ``XYZ_bar`` entry.
+Without packing, ``num_mws`` is limited to four and each memory window uses
+its corresponding ``mwN`` size and ``mwN_bar``. To expose 2, 4, 8, or 16
+logical memory windows in one BAR, set ``packed_mws`` to the same value as
+``num_mws``. In that mode, ``mw1`` is the aggregate BAR size and must be a
+power of two no larger than 2 GiB. It is divided equally
+into 4 KiB-aligned logical windows, and ``mw1_bar`` is their shared BAR. The
+``mw2`` through ``mw4`` entries report the logical window size when in range
+and reject writes.
+
+Packed MWs require an NTB client that uses MW translation group operations;
+``ntb_transport`` supports them. Per-MW size limiting through its
+``max_mw_size`` parameter is not supported. ``ntb_transport`` also needs four
+control scratchpads plus two scratchpads per logical MW.
+
A sample configuration for virtual NTB driver for virtual PCI bus::
# echo 0x1957 > functions/pci_epf_vntb/func1/pci_epf_vntb.0/vntb_vid
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 526cc31d9435..2f6af0306563 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -1295,6 +1295,7 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item, \
struct config_group *group = to_config_group(item); \
struct epf_ntb *ntb = to_epf_ntb(group); \
struct device *dev = &ntb->epf->dev; \
+ u32 packed_mws; \
int win_no, idx; \
\
if (sscanf(#_name, "mw%d", &win_no) != 1) \
@@ -1306,6 +1307,14 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item, \
win_no, ntb->num_mws); \
return -ERANGE; \
} \
+ packed_mws = ntb->packed_mws; \
+ if (packed_mws && idx > 0) { \
+ u64 size = ntb->mws_size[0]; \
+ \
+ if (size % packed_mws) \
+ return -EINVAL; \
+ return sprintf(page, "%llu\n", size / packed_mws); \
+ } \
idx = array_index_nospec(idx, ntb->num_mws); \
return sprintf(page, "%llu\n", ntb->mws_size[idx]); \
}
@@ -1337,6 +1346,8 @@ static ssize_t epf_ntb_##_name##_store(struct config_item *item, \
win_no, ntb->num_mws); \
return -ERANGE; \
} \
+ if (ntb->packed_mws && idx > 0) \
+ return -EINVAL; \
idx = array_index_nospec(idx, ntb->num_mws); \
ntb->mws_size[idx] = val; \
\
@@ -1372,6 +1383,9 @@ static ssize_t epf_ntb_##_name##_store(struct config_item *item, \
if (val < NO_BAR || val > BAR_5) \
return -EINVAL; \
\
+ if (ntb->packed_mws && _id >= BAR_MW2) \
+ return -EINVAL; \
+ \
ntb->epf_ntb_bar[_id] = val; \
\
return len; \
@@ -1392,7 +1406,10 @@ static ssize_t epf_ntb_num_mws_store(struct config_item *item,
if (ret)
return ret;
- if (val > MAX_MW)
+ if (val > EPF_NTB_MAX_MW)
+ return -EINVAL;
+
+ if (ntb->packed_mws && val != ntb->packed_mws)
return -EINVAL;
ntb->num_mws = val;
@@ -1400,6 +1417,44 @@ static ssize_t epf_ntb_num_mws_store(struct config_item *item,
return len;
}
+static ssize_t epf_ntb_packed_mws_store(struct config_item *item,
+ const char *page, size_t len)
+{
+ struct config_group *group = to_config_group(item);
+ struct epf_ntb *ntb = to_epf_ntb(group);
+ u32 val;
+ int ret;
+ int i;
+
+ if (epf_ntb_epc_attached(ntb))
+ return -EOPNOTSUPP;
+
+ ret = kstrtou32(page, 0, &val);
+ if (ret)
+ return ret;
+
+ if (val > EPF_NTB_MAX_MW ||
+ (val && (val < 2 || !is_power_of_2(val))))
+ return -EINVAL;
+
+ if (val && ntb->num_mws && val != ntb->num_mws)
+ return -EINVAL;
+
+ if (val) {
+ for (i = 1; i < MAX_MW; i++)
+ if (ntb->mws_size[i])
+ return -EINVAL;
+
+ for (i = BAR_MW2; i <= BAR_MW4; i++)
+ if (ntb->epf_ntb_bar[i] != NO_BAR)
+ return -EINVAL;
+ }
+
+ ntb->packed_mws = val;
+
+ return len;
+}
+
static ssize_t epf_ntb_db_count_store(struct config_item *item,
const char *page, size_t len)
{
@@ -1427,6 +1482,7 @@ EPF_NTB_R(spad_count)
EPF_NTB_W(spad_count)
EPF_NTB_R(db_count)
EPF_NTB_R(num_mws)
+EPF_NTB_R(packed_mws)
EPF_NTB_R(vbus_number)
EPF_NTB_W(vbus_number)
EPF_NTB_R(vntb_pid)
@@ -1457,6 +1513,7 @@ EPF_NTB_BAR_W(mw4_bar, BAR_MW4)
CONFIGFS_ATTR(epf_ntb_, spad_count);
CONFIGFS_ATTR(epf_ntb_, db_count);
CONFIGFS_ATTR(epf_ntb_, num_mws);
+CONFIGFS_ATTR(epf_ntb_, packed_mws);
CONFIGFS_ATTR(epf_ntb_, mw1);
CONFIGFS_ATTR(epf_ntb_, mw2);
CONFIGFS_ATTR(epf_ntb_, mw3);
@@ -1475,6 +1532,7 @@ static struct configfs_attribute *epf_ntb_attrs[] = {
&epf_ntb_attr_spad_count,
&epf_ntb_attr_db_count,
&epf_ntb_attr_num_mws,
+ &epf_ntb_attr_packed_mws,
&epf_ntb_attr_mw1,
&epf_ntb_attr_mw2,
&epf_ntb_attr_mw3,
--
2.51.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs
2026-08-03 18:04 ` [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs Koichiro Den
@ 2026-08-03 18:40 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-08-03 18:40 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci, ntb
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] vntb_epf_mw_set_trans() does not reject calls when packed_mws is enabled, allowing legacy NTB clients to corrupt the shared BAR configuration.
- [Medium] Data race on `ntb->packed_mws` in configfs `show` methods can lead to theoretical divide-by-zero.
- [Low] Using native division and modulo operators on a 64-bit integer will cause a link-time error on 32-bit architectures.
Pre-existing issues:
- [High] The loop initializing optional BARs incorrectly starts at BAR_MW1 instead of BAR_MW2, leaving BAR_MW4 uninitialized when num_mws is 4.
--
--- Patch [11]: [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs ---
commit ff5e63e72a02a10c0fca1af20a09901153b50a18
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs
This patch adds support for exposing packed memory windows via configfs,
enabling up to 16 logical memory windows when packing is selected. It
utilizes mw1 as the aggregate BAR size and restricts other window sizes
and BAR attributes when packing is active.
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
[ ... ]
> @@ -1295,6 +1295,7 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item, \
> struct config_group *group = to_config_group(item); \
> struct epf_ntb *ntb = to_epf_ntb(group); \
> struct device *dev = &ntb->epf->dev; \
> + u32 packed_mws; \
> int win_no, idx; \
> \
> if (sscanf(#_name, "mw%d", &win_no) != 1) \
> @@ -1306,6 +1307,14 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item, \
> win_no, ntb->num_mws); \
> return -ERANGE; \
> } \
> + packed_mws = ntb->packed_mws; \
> + if (packed_mws && idx > 0) { \
> + u64 size = ntb->mws_size[0]; \
> + \
> + if (size % packed_mws) \
> + return -EINVAL; \
> + return sprintf(page, "%llu\n", size / packed_mws); \
[Severity: Medium]
Could the compiler re-fetch ntb->packed_mws from memory during the modulo
or division operations? If a concurrent configfs thread writes 0 to
packed_mws between the initial check and the division, might this result in
a divide-by-zero exception? Would using READ_ONCE() here be appropriate?
[Severity: Low]
Does using the native modulo and division operators on a 64-bit integer
(size) and a 32-bit integer (packed_mws) cause a link-time error on 32-bit
architectures due to missing symbols? Should this use div64_u64() or
do_div() instead? This same pattern also appears to be present in
epf_ntb_build_mw_layout().
> + } \
> idx = array_index_nospec(idx, ntb->num_mws); \
> return sprintf(page, "%llu\n", ntb->mws_size[idx]); \
> }
[ ... ]
> @@ -1400,6 +1417,44 @@ static ssize_t epf_ntb_num_mws_store(struct config_item *item,
> return len;
> }
>
> +static ssize_t epf_ntb_packed_mws_store(struct config_item *item,
> + const char *page, size_t len)
> +{
> + struct config_group *group = to_config_group(item);
> + struct epf_ntb *ntb = to_epf_ntb(group);
> + u32 val;
> + int ret;
> + int i;
> +
> + if (epf_ntb_epc_attached(ntb))
> + return -EOPNOTSUPP;
> +
> + ret = kstrtou32(page, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val > EPF_NTB_MAX_MW ||
> + (val && (val < 2 || !is_power_of_2(val))))
> + return -EINVAL;
> +
> + if (val && ntb->num_mws && val != ntb->num_mws)
> + return -EINVAL;
> +
> + if (val) {
> + for (i = 1; i < MAX_MW; i++)
> + if (ntb->mws_size[i])
> + return -EINVAL;
> +
> + for (i = BAR_MW2; i <= BAR_MW4; i++)
> + if (ntb->epf_ntb_bar[i] != NO_BAR)
> + return -EINVAL;
> + }
> +
> + ntb->packed_mws = val;
> +
> + return len;
> +}
[Severity: High]
When packed_mws is enabled, multiple logical memory windows share a single
physical PCI BAR. Does vntb_epf_mw_set_trans() need a check to reject calls
if packed_mws is set? If a legacy NTB client driver calls
vntb_epf_mw_set_trans(), could it overwrite the shared epf_bar->size with a
smaller logical window size and reconfigure the physical BAR, breaking the
configuration for all shared windows?
[Severity: High]
This is a pre-existing issue, but there appears to be a logic error in
epf_ntb_init_epc_bar(). The automatic BAR assignment loop for optional
memory windows starts iterating from BAR_MW1 with i=1 instead of BAR_MW2:
drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_init_epc_bar() {
...
for (bar = BAR_MW1, i = 1;
i < num_mws && bar <= BAR_MW4; bar++, i++) {
barno = epf_ntb_find_bar(ntb, epc_features, bar, barno);
...
}
Since BAR_MW1 was already assigned earlier in the function, this redundant
check consumes an iteration. When num_mws is 4, doesn't the loop terminate
when bar reaches BAR_MW3, leaving BAR_MW4 uninitialized and causing
automatic BAR assignment to fail?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=11
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows
2026-08-03 18:04 [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Koichiro Den
` (10 preceding siblings ...)
2026-08-03 18:04 ` [PATCH 11/11] PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs Koichiro Den
@ 2026-08-03 23:00 ` Randy Dunlap
2026-08-04 0:55 ` Koichiro Den
11 siblings, 1 reply; 26+ messages in thread
From: Randy Dunlap @ 2026-08-03 23:00 UTC (permalink / raw)
To: Koichiro Den, Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet
Cc: linux-pci, linux-doc, linux-kernel, ntb
On 8/3/26 11:04 AM, Koichiro Den wrote:
> Hi,
>
> vNTB currently spends one BAR per MW. BAR count therefore limits the
> number of MWs, even when one BAR has room for several. A large BAR,
> especially a Resizable BAR, can instead be split into logical MWs and
> leave more BARs for config and doorbells.
>
> This series adds packed_mws, which exposes up to 16 equal-sized logical
> MWs in one BAR. ntb_transport uses the new group operations to configure
> them atomically.
>
> The immediate use case is increasing the number of ntb_transport queues.
> The resulting performance gain is shown in the Testing section.
>
> Based on the latest pci/endpoint.
>
>
> Why not BAR subrange mappings?
> ==============================
>
> An earlier proposal used mwN_offset and BAR subrange mappings to place
> independently backed MWs in one BAR:
>
> https://lore.kernel.org/r/20260312165005.1148676-11-den@valinux.co.jp/
>
> BAR subrange mapping is currently implemented only by DWC, and that
> design still needs one hardware mapping per MW in each direction. This
> series instead requires one contiguous backing range and maps the whole
> group once in each direction. The initial implementation intentionally
> trades arbitrary MW placement for a constant mapping count as the MW
> count grows. More flexible layouts can be added when a concrete use
> case requires them.
>
Hi,
To be clear, this is totally outside of the scope of any PCI or NTB
standards or specs? or did I overlook that part of the discussion?
Thanks.
>
> Testing
> =======
>
> The packed data path was tested with ntb_transport and ntb_netdev. An
> `iperf3 -ub0 -l 65482 -P 2` UDP saturation run produced the following
> results when only one Resizable BAR was available for MWs:
>
> Configuration Receiver throughput
> ---------------------------------------------- -------------------
> num_mws=1, packed_mws unset (default 0) 607 Mbits/sec
> num_mws=2, packed_mws=2, combined channels=2 1.19 Gbits/sec
>
> Meanwhile, I realized that the in-tree ntb_test.sh does not fully pass
> with the pci-epf-vntb/ntb_hw_epf setup on the base tree. I had to apply
> a separate collection of local PCI endpoint and NTB fixes before this
> setup could be used for regression testing. These fixes address
> pre-existing issues and are not prerequisites of the packed MW
> implementation.
>
> With packed_mws unset and ntb_msi_tests disabled, ntb_test.sh reported
> the same pass/unsupported results before and after this series. One run
> follows:
>
> $ sudo ./ntb_test.sh -r ${EP} -m 1048576 -p 28 0000:01:00.0 0001:10:00.0
> Starting ntb_tool tests...
> Running port tests on: 0000:01:00.0 / 0001:10:00.0
> Local port 0 with index 0 on remote host
> Peer port 0 with index 0 on local host
> Passed
> Running link tests on: 0000:01:00.0 / 0001:10:00.0
> Passed
> Running link tests on: 0001:10:00.0 / 0000:01:00.0
> Passed
> Running db tests on: 0000:01:00.0 / 0001:10:00.0
> Passed
> Running db tests on: 0001:10:00.0 / 0000:01:00.0
> Passed
> Running spad tests on: 0000:01:00.0 / 0001:10:00.0
> Passed
> Running spad tests on: 0001:10:00.0 / 0000:01:00.0
> Passed
> Running msg tests on: 0000:01:00.0 / 0001:10:00.0
> Unsupported
> Running msg tests on: 0001:10:00.0 / 0000:01:00.0
> Unsupported
> Running mw0 tests on: 0000:01:00.0 / 0001:10:00.0
> Passed
> Running mw0 tests on: 0001:10:00.0 / 0000:01:00.0
> Passed
>
> Starting ntb_pingpong tests...
> Running ping pong tests on: 0000:01:00.0 / 0001:10:00.0
> Passed
>
> Starting ntb_perf tests...
> Running local perf test without DMA
> Peer 0 test statistics:
> 0: copied 268435456 bytes in 137063 usecs, 1958 MBytes/s
> Passed
> Running remote perf test without DMA
> Peer 0 test statistics:
> 0: copied 268435456 bytes in 139188 usecs, 1928 MBytes/s
> Passed
>
> For comparison, the before-series run reported 1997 MBytes/s locally
> and 1950 MBytes/s remotely. No obvious regression was seen in this
> single-run comparison.
>
>
> Control layout compatibility
> ============================
>
> The endpoint selects the control layout. Non-packed pci-epf-vntb keeps
> the version 0 layout. packed_mws selects version 1. "Old" and "new"
> below mean ntb_hw_epf without and with this series.
>
> EP configuration Old ntb_hw_epf New ntb_hw_epf
> ---------------- ------------------------ ------------------------
> non-packed (v0) Supported Supported
> packed (v1) Not supported [1] Supported
>
> [1] The old driver accepts two or four MWs but looks for a separate BAR
> for each MW. It rejects larger MW counts at probe.
>
>
> Packed MW compatibility with in-tree NTB clients
> ================================================
>
> The in-tree clients handle a packed MW group as follows:
>
> Client Behavior
> -------------- ------------------------------------------------------
> ntb_transport Uses group operations. Without a singleton MW, MSI
> mode falls back to doorbells.
> ntb_perf Rejects a non-singleton MW group at probe.
> ntb_tool Loads normally. Per-MW inbound translation setup
> returns -EOPNOTSUPP for a non-singleton group.
> Doorbell and scratchpad access is unaffected.
> ntb_msi_test Fails probe unless the MW reserved for MSI is a
> singleton.
> ntb_pingpong Does not use MWs and is unaffected.
>
> Further client support can be added separately when needed. A final
> singleton MW on another BAR can also be added later if MSI mode is needed
> with packed MWs.
>
>
> Notes
> =====
>
> * About pci-epf-ntb:
>
> pci-epf-ntb remains on the legacy version 0 layout; this series does
> not add version 1 layout support to it. A pci-epf-ntb bridge should
> therefore keep its existing behavior even when its nodes run a mix of
> old and new kernels. I do not have hardware for the pci-epf-ntb bridge
> topology.
>
> * About existing inbound MW clear behavior:
>
> pci-epf-vntb already leaves the inbound BAR mapping in place when an
> MW translation is cleared. This series keeps that behavior for packed
> MWs. This limitation is independent of packing.
>
> pci_epc_clear_bar() is not suitable here because it can clear the BAR
> address assigned by the host. An EPF can instead restore a persistent
> fallback mapping with pci_epc_set_bar(), as pci-epf-test does:
>
> https://lore.kernel.org/r/20250908161942.534799-2-cassel@kernel.org/
>
> pci-epf-vntb has no such fallback mapping today. Adding one, or an EPC
> operation to unmap only the inbound translation, should be separate
> work.
>
> Best regards,
> Koichiro
>
>
> Koichiro Den (11):
> NTB: Add atomic MW translation group operations
> NTB: epf: Parse a versioned packed MW layout
> PCI: endpoint: pci-epf-vntb: Add packed MW layout handling
> PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
> PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space
> PCI: endpoint: pci-epf-vntb: Add outbound MW group commands
> NTB: epf: Implement MW group translation callbacks
> NTB: perf: Reject grouped memory windows
> NTB/msi: Require a singleton memory window
> NTB: ntb_transport: Use atomic MW translation groups
> PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs
>
> Documentation/PCI/endpoint/pci-vntb-howto.rst | 16 +-
> drivers/ntb/hw/epf/ntb_hw_epf.c | 143 +++++-
> drivers/ntb/msi.c | 19 +-
> drivers/ntb/ntb_transport.c | 127 +++++-
> drivers/ntb/test/ntb_perf.c | 16 +-
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 423 ++++++++++++++++--
> include/linux/ntb.h | 133 +++++-
> 7 files changed, 801 insertions(+), 76 deletions(-)
>
--
~Randy
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows
2026-08-03 23:00 ` [PATCH 00/11] PCI/NTB: endpoint: packed vNTB memory windows Randy Dunlap
@ 2026-08-04 0:55 ` Koichiro Den
0 siblings, 0 replies; 26+ messages in thread
From: Koichiro Den @ 2026-08-04 0:55 UTC (permalink / raw)
To: Randy Dunlap
Cc: Manivannan Sadhasivam, Frank Li, Niklas Cassel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jonathan Corbet, Shuah Khan, Jon Mason, Dave Jiang, Allen Hubbe,
Jerome Brunet, linux-pci, linux-doc, linux-kernel, ntb
On Mon, Aug 03, 2026 at 04:00:45PM -0700, Randy Dunlap wrote:
>
>
> On 8/3/26 11:04 AM, Koichiro Den wrote:
> > Hi,
> >
> > vNTB currently spends one BAR per MW. BAR count therefore limits the
> > number of MWs, even when one BAR has room for several. A large BAR,
> > especially a Resizable BAR, can instead be split into logical MWs and
> > leave more BARs for config and doorbells.
> >
> > This series adds packed_mws, which exposes up to 16 equal-sized logical
> > MWs in one BAR. ntb_transport uses the new group operations to configure
> > them atomically.
> >
> > The immediate use case is increasing the number of ntb_transport queues.
> > The resulting performance gain is shown in the Testing section.
> >
> > Based on the latest pci/endpoint.
> >
> >
> > Why not BAR subrange mappings?
> > ==============================
> >
> > An earlier proposal used mwN_offset and BAR subrange mappings to place
> > independently backed MWs in one BAR:
> >
> > https://lore.kernel.org/r/20260312165005.1148676-11-den@valinux.co.jp/
> >
> > BAR subrange mapping is currently implemented only by DWC, and that
> > design still needs one hardware mapping per MW in each direction. This
> > series instead requires one contiguous backing range and maps the whole
> > group once in each direction. The initial implementation intentionally
> > trades arbitrary MW placement for a constant mapping count as the MW
> > count grows. More flexible layouts can be added when a concrete use
> > case requires them.
> >
>
> Hi,
> To be clear, this is totally outside of the scope of any PCI or NTB
> standards or specs? or did I overlook that part of the discussion?
> Thanks.
Hi Randy,
No, you didn't overlook anything. This isn't part of any PCI or NTB spec.
This series just proposes a new 'packed_mws' configfs knob for the
vNTB/ntb_hw_epf pair. PCI still sees a normal BAR.
Thanks,
Koichiro
>
> >
> > Testing
> > =======
> >
> > The packed data path was tested with ntb_transport and ntb_netdev. An
> > `iperf3 -ub0 -l 65482 -P 2` UDP saturation run produced the following
> > results when only one Resizable BAR was available for MWs:
> >
> > Configuration Receiver throughput
> > ---------------------------------------------- -------------------
> > num_mws=1, packed_mws unset (default 0) 607 Mbits/sec
> > num_mws=2, packed_mws=2, combined channels=2 1.19 Gbits/sec
> >
> > Meanwhile, I realized that the in-tree ntb_test.sh does not fully pass
> > with the pci-epf-vntb/ntb_hw_epf setup on the base tree. I had to apply
> > a separate collection of local PCI endpoint and NTB fixes before this
> > setup could be used for regression testing. These fixes address
> > pre-existing issues and are not prerequisites of the packed MW
> > implementation.
> >
> > With packed_mws unset and ntb_msi_tests disabled, ntb_test.sh reported
> > the same pass/unsupported results before and after this series. One run
> > follows:
> >
> > $ sudo ./ntb_test.sh -r ${EP} -m 1048576 -p 28 0000:01:00.0 0001:10:00.0
> > Starting ntb_tool tests...
> > Running port tests on: 0000:01:00.0 / 0001:10:00.0
> > Local port 0 with index 0 on remote host
> > Peer port 0 with index 0 on local host
> > Passed
> > Running link tests on: 0000:01:00.0 / 0001:10:00.0
> > Passed
> > Running link tests on: 0001:10:00.0 / 0000:01:00.0
> > Passed
> > Running db tests on: 0000:01:00.0 / 0001:10:00.0
> > Passed
> > Running db tests on: 0001:10:00.0 / 0000:01:00.0
> > Passed
> > Running spad tests on: 0000:01:00.0 / 0001:10:00.0
> > Passed
> > Running spad tests on: 0001:10:00.0 / 0000:01:00.0
> > Passed
> > Running msg tests on: 0000:01:00.0 / 0001:10:00.0
> > Unsupported
> > Running msg tests on: 0001:10:00.0 / 0000:01:00.0
> > Unsupported
> > Running mw0 tests on: 0000:01:00.0 / 0001:10:00.0
> > Passed
> > Running mw0 tests on: 0001:10:00.0 / 0000:01:00.0
> > Passed
> >
> > Starting ntb_pingpong tests...
> > Running ping pong tests on: 0000:01:00.0 / 0001:10:00.0
> > Passed
> >
> > Starting ntb_perf tests...
> > Running local perf test without DMA
> > Peer 0 test statistics:
> > 0: copied 268435456 bytes in 137063 usecs, 1958 MBytes/s
> > Passed
> > Running remote perf test without DMA
> > Peer 0 test statistics:
> > 0: copied 268435456 bytes in 139188 usecs, 1928 MBytes/s
> > Passed
> >
> > For comparison, the before-series run reported 1997 MBytes/s locally
> > and 1950 MBytes/s remotely. No obvious regression was seen in this
> > single-run comparison.
> >
> >
> > Control layout compatibility
> > ============================
> >
> > The endpoint selects the control layout. Non-packed pci-epf-vntb keeps
> > the version 0 layout. packed_mws selects version 1. "Old" and "new"
> > below mean ntb_hw_epf without and with this series.
> >
> > EP configuration Old ntb_hw_epf New ntb_hw_epf
> > ---------------- ------------------------ ------------------------
> > non-packed (v0) Supported Supported
> > packed (v1) Not supported [1] Supported
> >
> > [1] The old driver accepts two or four MWs but looks for a separate BAR
> > for each MW. It rejects larger MW counts at probe.
> >
> >
> > Packed MW compatibility with in-tree NTB clients
> > ================================================
> >
> > The in-tree clients handle a packed MW group as follows:
> >
> > Client Behavior
> > -------------- ------------------------------------------------------
> > ntb_transport Uses group operations. Without a singleton MW, MSI
> > mode falls back to doorbells.
> > ntb_perf Rejects a non-singleton MW group at probe.
> > ntb_tool Loads normally. Per-MW inbound translation setup
> > returns -EOPNOTSUPP for a non-singleton group.
> > Doorbell and scratchpad access is unaffected.
> > ntb_msi_test Fails probe unless the MW reserved for MSI is a
> > singleton.
> > ntb_pingpong Does not use MWs and is unaffected.
> >
> > Further client support can be added separately when needed. A final
> > singleton MW on another BAR can also be added later if MSI mode is needed
> > with packed MWs.
> >
> >
> > Notes
> > =====
> >
> > * About pci-epf-ntb:
> >
> > pci-epf-ntb remains on the legacy version 0 layout; this series does
> > not add version 1 layout support to it. A pci-epf-ntb bridge should
> > therefore keep its existing behavior even when its nodes run a mix of
> > old and new kernels. I do not have hardware for the pci-epf-ntb bridge
> > topology.
> >
> > * About existing inbound MW clear behavior:
> >
> > pci-epf-vntb already leaves the inbound BAR mapping in place when an
> > MW translation is cleared. This series keeps that behavior for packed
> > MWs. This limitation is independent of packing.
> >
> > pci_epc_clear_bar() is not suitable here because it can clear the BAR
> > address assigned by the host. An EPF can instead restore a persistent
> > fallback mapping with pci_epc_set_bar(), as pci-epf-test does:
> >
> > https://lore.kernel.org/r/20250908161942.534799-2-cassel@kernel.org/
> >
> > pci-epf-vntb has no such fallback mapping today. Adding one, or an EPC
> > operation to unmap only the inbound translation, should be separate
> > work.
> >
> > Best regards,
> > Koichiro
> >
> >
> > Koichiro Den (11):
> > NTB: Add atomic MW translation group operations
> > NTB: epf: Parse a versioned packed MW layout
> > PCI: endpoint: pci-epf-vntb: Add packed MW layout handling
> > PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks
> > PCI: endpoint: pci-epf-vntb: Allocate packed outbound MW space
> > PCI: endpoint: pci-epf-vntb: Add outbound MW group commands
> > NTB: epf: Implement MW group translation callbacks
> > NTB: perf: Reject grouped memory windows
> > NTB/msi: Require a singleton memory window
> > NTB: ntb_transport: Use atomic MW translation groups
> > PCI: endpoint: pci-epf-vntb: Expose packed MWs through configfs
> >
> > Documentation/PCI/endpoint/pci-vntb-howto.rst | 16 +-
> > drivers/ntb/hw/epf/ntb_hw_epf.c | 143 +++++-
> > drivers/ntb/msi.c | 19 +-
> > drivers/ntb/ntb_transport.c | 127 +++++-
> > drivers/ntb/test/ntb_perf.c | 16 +-
> > drivers/pci/endpoint/functions/pci-epf-vntb.c | 423 ++++++++++++++++--
> > include/linux/ntb.h | 133 +++++-
> > 7 files changed, 801 insertions(+), 76 deletions(-)
> >
>
> --
> ~Randy
>
^ permalink raw reply [flat|nested] 26+ messages in thread