Re: [PATCH v2 2/2] NTB: epf: propagate MW command errors while status is reliable

[Koichiro Den]

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@xxxxxxxxx>

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@xxxxxxxxxxxxx>

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
>