[PATCH v4] PCI: endpoint: pci-epf-vntb: Track link state from both sides

[PATCH v4] PCI: endpoint: pci-epf-vntb: Track link state from both sides

[Koichiro Den]

The control-region link status is currently updated only by
COMMAND_LINK_UP and COMMAND_LINK_DOWN from the HOST. The virtual
NTB link callbacks are empty. Consequently, the HOST can see the link as
up before the VHOST has enabled it, and ntb_link_disable() on the VHOST
leaves LINK_STATUS_UP set without notifying either NTB client.

ntb_netdev can hide this device-level state bug because ntb_transport uses
a separate per-QP LINK_DOWN_FLAG message. Use ntb_tool to observe the
device link state directly:

After bringing both sides up, start one waiter on each side:

  HOST# echo N > /sys/kernel/debug/ntb_tool/<H-device>/peer0/link_event
  VHOST# echo N > /sys/kernel/debug/ntb_tool/<V-device>/peer0/link_event

While both waiters are blocked, disable the link on the VHOST:

  VHOST# echo N > /sys/kernel/debug/ntb_tool/<V-device>/link

Without this patch, the link-disable command succeeds, but peer0/link
remains Y on both sides and neither waiter returns.

Track HOST and VHOST enablement separately and report the effective link
as up only when both sides have enabled it, as pci-epf-ntb does for its
two physical hosts. Both NTB clients observe this effective state, so
notify both sides only when it changes, including the side whose request
completes the transition. Serialize the two paths because HOST commands
run from delayed work while the VHOST callbacks may run concurrently.

With this patch, peer0/link changes to N on both sides and both waiters
return.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v4:
  - No functional changes, but:
    * Rename link state flags and helpers for clarity. (Mani)
    * Make the *_set_link_enabled() return patterns symmetric.
  - Add Frank's Reviewed-by tag from v3.
  - Retest using ntb_tool.
  - Link to v3: https://lore.kernel.org/r/20261014071448.2473208-1-den@valinux.co.jp/

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 98 +++++++++++++++----
 1 file changed, 78 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 07ba338732d4..bebdd93a067c 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -41,6 +41,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/slab.h>
 
 #include <linux/pci-ep-msi.h>
@@ -147,7 +148,10 @@ struct epf_ntb {
 	u16 vntb_pid;
 	u16 vntb_vid;
 
-	bool linkup;
+	/* Serialize HOST and VHOST link state changes. */
+	struct mutex link_lock;
+	bool host_link_enabled;
+	bool vhost_link_enabled;
 
 	/*
 	 * True when doorbells are interrupt-driven (MSI or embedded), false
@@ -183,24 +187,55 @@ static struct pci_epf_header epf_ntb_header = {
 	.interrupt_pin	= PCI_INTERRUPT_INTA,
 };
 
+static void epf_ntb_update_link(struct epf_ntb *ntb)
+{
+	u16 link_status = READ_ONCE(ntb->reg->link_status);
+
+	/* The link is usable only after both sides have enabled it. */
+	if (ntb->host_link_enabled && ntb->vhost_link_enabled)
+		link_status |= LINK_STATUS_UP;
+	else
+		link_status &= ~LINK_STATUS_UP;
+
+	WRITE_ONCE(ntb->reg->link_status, link_status);
+}
+
+static int epf_ntb_notify_link(struct ntb_dev *ndev)
+{
+	struct pci_epf *epf = ntb_ndev(ndev)->epf;
+	int ret;
+
+	ntb_link_event(ndev);
+
+	ret = pci_epc_raise_irq(epf->epc, epf->func_no, epf->vfunc_no,
+				PCI_IRQ_MSI, EPF_IRQ_LINK + 1);
+	if (ret)
+		dev_err(&epf->dev, "Failed to raise link event IRQ: %d\n", ret);
+
+	return ret;
+}
+
 /**
- * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
+ * epf_ntb_set_link_enabled() - Set HOST link enablement
  * @ntb: NTB device that facilitates communication between HOST and VHOST
- * @link_up: true or false indicating Link is UP or Down
- *
- * Once NTB function in HOST invoke ntb_link_enable(),
- * this NTB function driver will trigger a link event to VHOST.
+ * @enabled: true when the HOST has enabled the link
  *
  * Returns: Zero for success, or an error code in case of failure
  */
-static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
+static int epf_ntb_set_link_enabled(struct epf_ntb *ntb, bool enabled)
 {
-	if (link_up)
-		ntb->reg->link_status |= LINK_STATUS_UP;
-	else
-		ntb->reg->link_status &= ~LINK_STATUS_UP;
+	bool notify;
+
+	scoped_guard(mutex, &ntb->link_lock) {
+		notify = ntb->host_link_enabled != enabled &&
+			 ntb->vhost_link_enabled;
+		ntb->host_link_enabled = enabled;
+		epf_ntb_update_link(ntb);
+	}
+
+	if (notify)
+		return epf_ntb_notify_link(ntb->ntb);
 
-	ntb_link_event(ntb->ntb);
 	return 0;
 }
 
@@ -325,16 +360,14 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 		ctrl->command_status = COMMAND_STATUS_OK;
 		break;
 	case COMMAND_LINK_UP:
-		ntb->linkup = true;
-		ret = epf_ntb_link_up(ntb, true);
+		ret = epf_ntb_set_link_enabled(ntb, true);
 		if (ret < 0)
 			ctrl->command_status = COMMAND_STATUS_ERROR;
 		else
 			ctrl->command_status = COMMAND_STATUS_OK;
 		goto reset_handler;
 	case COMMAND_LINK_DOWN:
-		ntb->linkup = false;
-		ret = epf_ntb_link_up(ntb, false);
+		ret = epf_ntb_set_link_enabled(ntb, false);
 		if (ret < 0)
 			ctrl->command_status = COMMAND_STATUS_ERROR;
 		else
@@ -1472,11 +1505,29 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
 	return 0;
 }
 
+static int vntb_epf_set_link_enabled(struct ntb_dev *ndev, bool enabled)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	bool notify;
+
+	scoped_guard(mutex, &ntb->link_lock) {
+		notify = ntb->vhost_link_enabled != enabled &&
+			 ntb->host_link_enabled;
+		ntb->vhost_link_enabled = enabled;
+		epf_ntb_update_link(ntb);
+	}
+
+	if (notify)
+		return epf_ntb_notify_link(ndev);
+
+	return 0;
+}
+
 static int vntb_epf_link_enable(struct ntb_dev *ntb,
 			enum ntb_speed max_speed,
 			enum ntb_width max_width)
 {
-	return 0;
+	return vntb_epf_set_link_enabled(ntb, true);
 }
 
 static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
@@ -1636,7 +1687,7 @@ static u64 vntb_epf_link_is_up(struct ntb_dev *ndev,
 {
 	struct epf_ntb *ntb = ntb_ndev(ndev);
 
-	return ntb->reg->link_status;
+	return READ_ONCE(ntb->reg->link_status);
 }
 
 static int vntb_epf_db_clear_mask(struct ntb_dev *ndev, u64 db_bits)
@@ -1654,7 +1705,7 @@ static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
 
 static int vntb_epf_link_disable(struct ntb_dev *ntb)
 {
-	return 0;
+	return vntb_epf_set_link_enabled(ntb, false);
 }
 
 static struct device *vntb_epf_get_dma_dev(struct ntb_dev *ndev)
@@ -1797,6 +1848,10 @@ static int epf_ntb_bind(struct pci_epf *epf)
 		goto err_bar_alloc;
 	}
 
+	ntb->host_link_enabled = false;
+	ntb->vhost_link_enabled = false;
+	ntb->reg->link_status = 0;
+
 	ret = epf_ntb_epc_init(ntb);
 	if (ret) {
 		dev_err(dev, "Failed to initialize EPC\n");
@@ -1882,7 +1937,7 @@ static int epf_ntb_probe(struct pci_epf *epf,
 {
 	struct epf_ntb *ntb;
 	struct device *dev;
-	int i;
+	int ret, i;
 
 	dev = &epf->dev;
 
@@ -1893,6 +1948,9 @@ static int epf_ntb_probe(struct pci_epf *epf,
 	epf->header = &epf_ntb_header;
 	ntb->epf = epf;
 	ntb->vbus_number = 0xff;
+	ret = devm_mutex_init(dev, &ntb->link_lock);
+	if (ret)
+		return ret;
 
 	INIT_WORK(&ntb->peer_db_work, vntb_epf_peer_db_work);
 	disable_work(&ntb->peer_db_work);

base-commit: 5cdbd6d3a3a147fa9975e35f2602cbec9a72a932
-- 
2.54.0

Re: [PATCH v4] PCI: endpoint: pci-epf-vntb: Track link state from both sides

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: state desync on link notification failure
--

commit d2ad15704832c4f850d567ba49113c3cd0055435
Author: Koichiro Den <den@valinux.co.jp>

PCI: endpoint: pci-epf-vntb: Track link state from both sides

This commit introduces separate tracking for HOST and VHOST link states to
ensure the effective link is reported as up only when both sides have
enabled it. It ensures both NTB clients observe the proper state by updating
and notifying when transitions complete.

Link: https://lore.kernel.org/r/20261014071448.2473208-1-den@valinux.co.jp/

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 07ba338732d4e..bebdd93a067c0 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
[ ... ]
> +static int epf_ntb_set_link_enabled(struct epf_ntb *ntb, bool enabled)
>  {
> -	if (link_up)
> -		ntb->reg->link_status |= LINK_STATUS_UP;
> -	else
> -		ntb->reg->link_status &= ~LINK_STATUS_UP;
> +	bool notify;
> +
> +	scoped_guard(mutex, &ntb->link_lock) {
> +		notify = ntb->host_link_enabled != enabled &&
> +			 ntb->vhost_link_enabled;
> +		ntb->host_link_enabled = enabled;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (notify)
> +		return epf_ntb_notify_link(ntb->ntb);

[Severity: Medium]
If epf_ntb_notify_link() fails (for example, if pci_epc_raise_irq()
returns an error), does the link state become permanently desynchronized?

The host link state is committed inside the scoped_guard, but if the
notification fails, the state is not rolled back. If the enable command is
subsequently retried, the notify flag will evaluate to false because
ntb->host_link_enabled already matches the requested state. This would cause
the function to return 0 without ever attempting to raise the interrupt
again.

[ ... ]
> @@ -1472,11 +1505,29 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
[ ... ]
> +static int vntb_epf_set_link_enabled(struct ntb_dev *ndev, bool enabled)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	bool notify;
> +
> +	scoped_guard(mutex, &ntb->link_lock) {
> +		notify = ntb->vhost_link_enabled != enabled &&
> +			 ntb->host_link_enabled;
> +		ntb->vhost_link_enabled = enabled;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (notify)
> +		return epf_ntb_notify_link(ndev);

[Severity: Medium]
Similar to epf_ntb_set_link_enabled() above, could a failure in
epf_ntb_notify_link() leave the VHOST link state permanently broken?

If the IRQ fails, the state update remains committed, meaning any
subsequent attempt to enable the link will skip the notification step and
silently succeed without notifying the remote side.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015142716.3452900-1-den@valinux.co.jp?part=1

Re: [PATCH v4] PCI: endpoint: pci-epf-vntb: Track link state from both sides

[Manivannan Sadhasivam]

On Thu, Oct 15, 2026 at 11:27:16PM +0900, Koichiro Den wrote:
> The control-region link status is currently updated only by
> COMMAND_LINK_UP and COMMAND_LINK_DOWN from the HOST. The virtual
> NTB link callbacks are empty. Consequently, the HOST can see the link as
> up before the VHOST has enabled it, and ntb_link_disable() on the VHOST
> leaves LINK_STATUS_UP set without notifying either NTB client.
> 
> ntb_netdev can hide this device-level state bug because ntb_transport uses
> a separate per-QP LINK_DOWN_FLAG message. Use ntb_tool to observe the
> device link state directly:
> 
> After bringing both sides up, start one waiter on each side:
> 
>   HOST# echo N > /sys/kernel/debug/ntb_tool/<H-device>/peer0/link_event
>   VHOST# echo N > /sys/kernel/debug/ntb_tool/<V-device>/peer0/link_event
> 
> While both waiters are blocked, disable the link on the VHOST:
> 
>   VHOST# echo N > /sys/kernel/debug/ntb_tool/<V-device>/link
> 
> Without this patch, the link-disable command succeeds, but peer0/link
> remains Y on both sides and neither waiter returns.
> 
> Track HOST and VHOST enablement separately and report the effective link
> as up only when both sides have enabled it, as pci-epf-ntb does for its
> two physical hosts. Both NTB clients observe this effective state, so
> notify both sides only when it changes, including the side whose request
> completes the transition. Serialize the two paths because HOST commands
> run from delayed work while the VHOST callbacks may run concurrently.
> 
> With this patch, peer0/link changes to N on both sides and both waiters
> return.
> 
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v4:
>   - No functional changes, but:
>     * Rename link state flags and helpers for clarity. (Mani)
>     * Make the *_set_link_enabled() return patterns symmetric.
>   - Add Frank's Reviewed-by tag from v3.
>   - Retest using ntb_tool.
>   - Link to v3: https://lore.kernel.org/r/20261014071448.2473208-1-den@valinux.co.jp/
> 
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 98 +++++++++++++++----
>  1 file changed, 78 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 07ba338732d4..bebdd93a067c 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -41,6 +41,7 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/slab.h>
>  
>  #include <linux/pci-ep-msi.h>
> @@ -147,7 +148,10 @@ struct epf_ntb {
>  	u16 vntb_pid;
>  	u16 vntb_vid;
>  
> -	bool linkup;
> +	/* Serialize HOST and VHOST link state changes. */
> +	struct mutex link_lock;
> +	bool host_link_enabled;
> +	bool vhost_link_enabled;
>  
>  	/*
>  	 * True when doorbells are interrupt-driven (MSI or embedded), false
> @@ -183,24 +187,55 @@ static struct pci_epf_header epf_ntb_header = {
>  	.interrupt_pin	= PCI_INTERRUPT_INTA,
>  };
>  
> +static void epf_ntb_update_link(struct epf_ntb *ntb)
> +{
> +	u16 link_status = READ_ONCE(ntb->reg->link_status);
> +

I believe this READ_ONCE here is just kept for the sake of symmetry. Since the
writer is inside this function only and this function is always called with
link_lock being held, there is no way the compiler can optimize this read. But
it is OK, I'm not complaining :)

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4] PCI: endpoint: pci-epf-vntb: Track link state from both sides

[Manivannan Sadhasivam]

On Thu, 15 Oct 2026 23:27:16 +0900, Koichiro Den wrote:
> The control-region link status is currently updated only by
> COMMAND_LINK_UP and COMMAND_LINK_DOWN from the HOST. The virtual
> NTB link callbacks are empty. Consequently, the HOST can see the link as
> up before the VHOST has enabled it, and ntb_link_disable() on the VHOST
> leaves LINK_STATUS_UP set without notifying either NTB client.
> 
> ntb_netdev can hide this device-level state bug because ntb_transport uses
> a separate per-QP LINK_DOWN_FLAG message. Use ntb_tool to observe the
> device link state directly:
> 
> [...]

Applied, thanks!

[1/1] PCI: endpoint: pci-epf-vntb: Track link state from both sides
      commit: f9eabc48327fae33cf0a02a8f312c24505dc413b

Best regards,
-- 
மணிவண்ணன் சதாசிவம்