* [PATCH v2 0/2] NTB: epf: serialize MW setup and report reliable errors @ 2026-11-01 13:57 Avi Weiss 2026-11-01 13:57 ` [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes Avi Weiss 2026-11-01 13:57 ` [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable Avi Weiss 0 siblings, 2 replies; 5+ messages in thread From: Avi Weiss @ 2026-11-01 13:57 UTC (permalink / raw) To: Jon Mason Cc: Dave Jiang, Allen Hubbe, Koichiro Den, Frank Li, ntb, linux-kernel, Avi Weiss This series fixes two related NTB EPF memory-window issues. The first patch serializes shared MW parameter writes with command submission, preventing concurrent setups from using each other's DMA addresses. The second patch propagates definite endpoint command errors while completion status can still be associated with the current command. Changes since v1: - koichiro pointed out that an endpoint may complete CONFIGURE_MW after the host times out. Propagating -ETIMEDOUT can cause clients to free DMA memory while the endpoint is still configuring the mapping. Returning success only for the timed-out command avoids that immediate regression, but was insufficient in the end b/c a late error from command A may be observed during command B. Returning that stale error can cause B's buffer to be freed before B completes. The second patch records whether any command has timed out and preserves the original MW return behavior thereafter. The first patch also prevents a pre-timeout regression: without parameter serialization, propagating an explicit rejection from one setup can release memory referenced by another concurrent setup. v1: https://lists.openwall.net/linux-kernel/2026/10/27/2144 Review: https://lists.openwall.net/linux-kernel/2026/10/28/466 Avi Weiss (2): NTB: epf: serialize memory window parameter writes NTB: epf: propagate MW command errors while status is reliable drivers/ntb/hw/epf/ntb_hw_epf.c | 37 ++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes 2026-11-01 13:57 [PATCH v2 0/2] NTB: epf: serialize MW setup and report reliable errors Avi Weiss @ 2026-11-01 13:57 ` Avi Weiss 2026-11-01 15:53 ` Koichiro Den 2026-11-01 13:57 ` [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable Avi Weiss 1 sibling, 1 reply; 5+ messages in thread From: Avi Weiss @ 2026-11-01 13:57 UTC (permalink / raw) To: Jon Mason Cc: Dave Jiang, Allen Hubbe, Koichiro Den, Frank Li, ntb, linux-kernel, Avi Weiss ntb_epf_mw_set_trans() writes the address and size to shared control registers before ntb_epf_send_command() acquires cmd_lock. Concurrent memory window setup requests can overwrite each other's parameters, causing one window to use another's DMA address. Hold cmd_lock across the parameter writes and command submission. Use a locked command helper to preserve the existing serialization for other command callers. Fixes: 812ce2f8d14e ("NTB: Add support for EPF PCI Non-Transparent Bridge") Signed-off-by: Avi Weiss <thnkslprpt@gmail.com> --- drivers/ntb/hw/epf/ntb_hw_epf.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c index c47607d4f8a7f..da7fbe1800656 100644 --- a/drivers/ntb/hw/epf/ntb_hw_epf.c +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c @@ -123,15 +123,14 @@ struct ntb_epf_dev { #define ntb_ndev(__ntb) container_of(__ntb, struct ntb_epf_dev, ntb) -static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, - u32 argument) +static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, + u32 argument) { ktime_t timeout; bool timedout; int ret = 0; u32 status; - mutex_lock(&ndev->cmd_lock); writel(argument, ndev->ctrl_reg + NTB_EPF_ARGUMENT); writel(command, ndev->ctrl_reg + NTB_EPF_COMMAND); @@ -157,6 +156,17 @@ static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, } writew(0, ndev->ctrl_reg + NTB_EPF_CMD_STATUS); + + return ret; +} + +static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, + u32 argument) +{ + int ret; + + mutex_lock(&ndev->cmd_lock); + ret = ntb_epf_send_command_locked(ndev, command, argument); mutex_unlock(&ndev->cmd_lock); return ret; @@ -494,11 +504,13 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, return -EINVAL; } + mutex_lock(&ndev->cmd_lock); 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); - ntb_epf_send_command(ndev, CMD_CONFIGURE_MW, idx); + ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); + mutex_unlock(&ndev->cmd_lock); return 0; } -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes 2026-11-01 13:57 ` [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes Avi Weiss @ 2026-11-01 15:53 ` Koichiro Den 0 siblings, 0 replies; 5+ messages in thread From: Koichiro Den @ 2026-11-01 15:53 UTC (permalink / raw) To: Avi Weiss; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, Frank Li, ntb, linux-kernel On Sun, Nov 01, 2026 at 03:57:05PM +0200, Avi Weiss wrote: > ntb_epf_mw_set_trans() writes the address and size to shared control > registers before ntb_epf_send_command() acquires cmd_lock. Concurrent > memory window setup requests can overwrite each other's parameters, > causing one window to use another's DMA address. > > Hold cmd_lock across the parameter writes and command submission. > Use a locked command helper to preserve the existing serialization > for other command callers. > > Fixes: 812ce2f8d14e ("NTB: Add support for EPF PCI Non-Transparent Bridge") > Signed-off-by: Avi Weiss <thnkslprpt@gmail.com> Reviewed-by: Koichiro Den <den@valinux.co.jp> > --- > drivers/ntb/hw/epf/ntb_hw_epf.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c > index c47607d4f8a7f..da7fbe1800656 100644 > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c > @@ -123,15 +123,14 @@ struct ntb_epf_dev { > > #define ntb_ndev(__ntb) container_of(__ntb, struct ntb_epf_dev, ntb) > > -static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, > - u32 argument) > +static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, > + u32 argument) > { > ktime_t timeout; > bool timedout; > int ret = 0; > u32 status; > > - mutex_lock(&ndev->cmd_lock); > writel(argument, ndev->ctrl_reg + NTB_EPF_ARGUMENT); > writel(command, ndev->ctrl_reg + NTB_EPF_COMMAND); > > @@ -157,6 +156,17 @@ static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, > } > > writew(0, ndev->ctrl_reg + NTB_EPF_CMD_STATUS); > + > + return ret; > +} > + > +static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, > + u32 argument) > +{ > + int ret; > + > + mutex_lock(&ndev->cmd_lock); > + ret = ntb_epf_send_command_locked(ndev, command, argument); > mutex_unlock(&ndev->cmd_lock); > > return ret; > @@ -494,11 +504,13 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > return -EINVAL; > } > > + mutex_lock(&ndev->cmd_lock); > 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); > - ntb_epf_send_command(ndev, CMD_CONFIGURE_MW, idx); > + ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); > + mutex_unlock(&ndev->cmd_lock); > > return 0; > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable 2026-11-01 13:57 [PATCH v2 0/2] NTB: epf: serialize MW setup and report reliable errors Avi Weiss 2026-11-01 13:57 ` [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes Avi Weiss @ 2026-11-01 13:57 ` Avi Weiss 2026-11-01 15:56 ` Koichiro Den 1 sibling, 1 reply; 5+ messages in thread From: Avi Weiss @ 2026-11-01 13:57 UTC (permalink / raw) To: Jon Mason Cc: Dave Jiang, Allen Hubbe, Koichiro Den, Frank Li, ntb, linux-kernel, Avi Weiss ntb_epf_mw_set_trans() and ntb_epf_mw_clear_trans() discard command errors even when the endpoint explicitly rejects a request. A timed-out command may still complete on the endpoint. Propagating that timeout can cause callers to free DMA buffers before the mapping completes. A late completion may also be mistaken for the result of a subsequent command. Record the first command timeout and propagate MW command errors only while completion status remains reliable. Preserve the original MW success-return behavior after a timeout to avoid releasing buffers based on ambiguous results. Fixes: 812ce2f8d14e ("NTB: Add support for EPF PCI Non-Transparent Bridge") Signed-off-by: Avi Weiss <thnkslprpt@gmail.com> --- drivers/ntb/hw/epf/ntb_hw_epf.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c index da7fbe1800656..455bcefc7c13e 100644 --- a/drivers/ntb/hw/epf/ntb_hw_epf.c +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c @@ -102,6 +102,7 @@ struct ntb_epf_dev { struct device *dev; /* Mutex to protect providing commands to NTB EPF */ struct mutex cmd_lock; + bool cmd_status_unreliable; const enum pci_barno *barno_map; @@ -148,6 +149,7 @@ static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, break; if (WARN_ON(timedout)) { + ndev->cmd_status_unreliable = true; ret = -ETIMEDOUT; break; } @@ -156,6 +158,13 @@ static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, } writew(0, ndev->ctrl_reg + NTB_EPF_CMD_STATUS); + /* + * After a timeout, a status may refer to an earlier command. + * MW callers may free buffers on error, so retain old semantics. + */ + if (ndev->cmd_status_unreliable && + (command == CMD_CONFIGURE_MW || command == CMD_TEARDOWN_MW)) + ret = 0; return ret; } @@ -485,7 +494,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, struct ntb_epf_dev *ndev = ntb_ndev(ntb); struct device *dev = ndev->dev; resource_size_t mw_size; - int bar; + int bar, ret; if (pidx != NTB_DEF_PEER_IDX) { dev_err(dev, "Unsupported Peer ID %d\n", pidx); @@ -509,19 +518,19 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, 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); - ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); + ret = ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); mutex_unlock(&ndev->cmd_lock); - return 0; + return ret; } static int ntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx) { struct ntb_epf_dev *ndev = ntb_ndev(ntb); struct device *dev = ndev->dev; - int ret = 0; + int ret; - ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx); + ret = ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx); if (ret) dev_err(dev, "Failed to teardown memory window\n"); -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable 2026-11-01 13:57 ` [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable Avi Weiss @ 2026-11-01 15:56 ` Koichiro Den 0 siblings, 0 replies; 5+ messages in thread From: Koichiro Den @ 2026-11-01 15:56 UTC (permalink / raw) To: Avi Weiss; +Cc: Jon Mason, Dave Jiang, Allen Hubbe, Frank Li, ntb, linux-kernel On Sun, Nov 01, 2026 at 03:57:06PM +0200, Avi Weiss wrote: > ntb_epf_mw_set_trans() and ntb_epf_mw_clear_trans() discard command > errors even when the endpoint explicitly rejects a request. > > A timed-out command may still complete on the endpoint. Propagating > that timeout can cause callers to free DMA buffers before the mapping > completes. A late completion may also be mistaken for the result of a > subsequent command. > > Record the first command timeout and propagate MW command errors only > while completion status remains reliable. Preserve the original MW > success-return behavior after a timeout to avoid releasing buffers > based on ambiguous results. > > Fixes: 812ce2f8d14e ("NTB: Add support for EPF PCI Non-Transparent Bridge") > Signed-off-by: Avi Weiss <thnkslprpt@gmail.com> LGTM. I think this is a nice improvement, and the points flagged by Sashiko seem to be pre-existing issues. Reviewed-by: Koichiro Den <den@valinux.co.jp> Thanks, Koichiro > --- > drivers/ntb/hw/epf/ntb_hw_epf.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c > index da7fbe1800656..455bcefc7c13e 100644 > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c > @@ -102,6 +102,7 @@ struct ntb_epf_dev { > struct device *dev; > /* Mutex to protect providing commands to NTB EPF */ > struct mutex cmd_lock; > + bool cmd_status_unreliable; > > const enum pci_barno *barno_map; > > @@ -148,6 +149,7 @@ static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, > break; > > if (WARN_ON(timedout)) { > + ndev->cmd_status_unreliable = true; > ret = -ETIMEDOUT; > break; > } > @@ -156,6 +158,13 @@ static int ntb_epf_send_command_locked(struct ntb_epf_dev *ndev, u32 command, > } > > writew(0, ndev->ctrl_reg + NTB_EPF_CMD_STATUS); > + /* > + * After a timeout, a status may refer to an earlier command. > + * MW callers may free buffers on error, so retain old semantics. > + */ > + if (ndev->cmd_status_unreliable && > + (command == CMD_CONFIGURE_MW || command == CMD_TEARDOWN_MW)) > + ret = 0; > > return ret; > } > @@ -485,7 +494,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > struct ntb_epf_dev *ndev = ntb_ndev(ntb); > struct device *dev = ndev->dev; > resource_size_t mw_size; > - int bar; > + int bar, ret; > > if (pidx != NTB_DEF_PEER_IDX) { > dev_err(dev, "Unsupported Peer ID %d\n", pidx); > @@ -509,19 +518,19 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > 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); > - ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); > + ret = ntb_epf_send_command_locked(ndev, CMD_CONFIGURE_MW, idx); > mutex_unlock(&ndev->cmd_lock); > > - return 0; > + return ret; > } > > static int ntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx) > { > struct ntb_epf_dev *ndev = ntb_ndev(ntb); > struct device *dev = ndev->dev; > - int ret = 0; > + int ret; > > - ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx); > + ret = ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx); > if (ret) > dev_err(dev, "Failed to teardown memory window\n"); > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-01 15:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-01 13:57 [PATCH v2 0/2] NTB: epf: serialize MW setup and report reliable errors Avi Weiss 2026-11-01 13:57 ` [PATCH v2 1/2] NTB: epf: serialize memory window parameter writes Avi Weiss 2026-11-01 15:53 ` Koichiro Den 2026-11-01 13:57 ` [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable Avi Weiss 2026-11-01 15:56 ` Koichiro Den
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®