mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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

* [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 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

* 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®