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

[PATCH] 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 the HOST.

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 this waiter on the HOST:

  echo N > /sys/kernel/debug/ntb_tool/<H-device>/peer0/link_event

While it is blocked, disable the link on the VHOST:

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

Without this patch, the second command succeeds, but peer0/link on the
HOST remains Y and the waiter does not return.

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. Notify the other side only when this effective state
changes. Serialize the two paths because HOST commands run from delayed
work while the VHOST callbacks may run concurrently.

With this patch, peer0/link on the HOST changes to N and the waiter
returns.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Note: This is a standalone fix, but it conflicts with the following
series:

  [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA
  https://lore.kernel.org/r/20260830151948.3547577-1-den@valinux.co.jp/

That series still has pending issues and will need more work. I will base
v3 on this fix and make the new link-event IRQ honor the MSI/MSI-X
selection made by the HOST.
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 82 +++++++++++++++----
 1 file changed, 65 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index fba65abfb6b2..51200223f53c 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_linkup;
+	bool vhost_linkup;
 
 	/*
 	 * True when doorbells are interrupt-driven (MSI or embedded), false
@@ -178,24 +182,42 @@ 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_linkup && ntb->vhost_linkup)
+		link_status |= LINK_STATUS_UP;
+	else
+		link_status &= ~LINK_STATUS_UP;
+
+	WRITE_ONCE(ntb->reg->link_status, link_status);
+}
+
 /**
- * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
+ * epf_ntb_link_up() - Update the HOST link state
  * @ntb: NTB device that facilitates communication between HOST and VHOST
- * @link_up: true or false indicating Link is UP or Down
+ * @link_up: true when the HOST has enabled the link
  *
- * Once NTB function in HOST invoke ntb_link_enable(),
- * this NTB function driver will trigger a link event to VHOST.
- *
- * Returns: Zero for success, or an error code in case of failure
+ * Returns: Zero for success
  */
 static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
 {
-	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_linkup != link_up && ntb->vhost_linkup;
+		ntb->host_linkup = link_up;
+		epf_ntb_update_link(ntb);
+	}
+
+	if (notify) {
+		/* Publish link status before completing the HOST command. */
+		dma_wmb();
+		ntb_link_event(&ntb->ntb);
+	}
 
-	ntb_link_event(&ntb->ntb);
 	return 0;
 }
 
@@ -320,7 +342,6 @@ 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);
 		if (ret < 0)
 			ctrl->command_status = COMMAND_STATUS_ERROR;
@@ -328,7 +349,6 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 			ctrl->command_status = COMMAND_STATUS_OK;
 		goto reset_handler;
 	case COMMAND_LINK_DOWN:
-		ntb->linkup = false;
 		ret = epf_ntb_link_up(ntb, false);
 		if (ret < 0)
 			ctrl->command_status = COMMAND_STATUS_ERROR;
@@ -1456,11 +1476,34 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
 	return 0;
 }
 
+static int vntb_epf_set_link(struct epf_ntb *ntb, bool link_up)
+{
+	struct pci_epf *epf = ntb->epf;
+	bool notify;
+	int ret;
+
+	scoped_guard(mutex, &ntb->link_lock) {
+		notify = ntb->vhost_linkup != link_up && ntb->host_linkup;
+		ntb->vhost_linkup = link_up;
+		epf_ntb_update_link(ntb);
+	}
+
+	if (!notify)
+		return 0;
+
+	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;
+}
+
 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(ntb_ndev(ntb), true);
 }
 
 static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
@@ -1620,7 +1663,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)
@@ -1638,7 +1681,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(ntb_ndev(ntb), false);
 }
 
 static struct device *vntb_epf_get_dma_dev(struct ntb_dev *ndev)
@@ -1750,6 +1793,10 @@ static int epf_ntb_bind(struct pci_epf *epf)
 		goto err_bar_alloc;
 	}
 
+	ntb->host_linkup = false;
+	ntb->vhost_linkup = false;
+	ntb->reg->link_status = 0;
+
 	ret = epf_ntb_epc_init(ntb);
 	if (ret) {
 		dev_err(dev, "Failed to initialize EPC\n");
@@ -1833,6 +1880,7 @@ static int epf_ntb_probe(struct pci_epf *epf,
 	epf->header = &epf_ntb_header;
 	ntb->epf = epf;
 	ntb->vbus_number = 0xff;
+	mutex_init(&ntb->link_lock);
 
 	INIT_WORK(&ntb->peer_db_work, vntb_epf_peer_db_work);
 	disable_work(&ntb->peer_db_work);
-- 
2.51.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The driver fails to mutually notify the local NTB client or HOST of effective link state changes, potentially causing clients waiting for link events to hang.

Pre-existing issues:
- [High] Missing endianness conversions for PCIe shared memory.
--

commit ca462c18f2f26ec109681df8452341853193267f
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Track link state from both sides

This patch tracks the link state of both HOST and VHOST sides of a virtual
NTB device separately. It reports the effective link as up only when both
sides enable it and notifies the other side when this effective state changes.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index c3caec927d748..8af3dcdd893c4 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c

[ ... ]

> @@ -178,24 +182,42 @@ 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);

[Severity: High]
This isn't a bug introduced by this patch, but the patch propagates a
pre-existing structural flaw by adding more native endian accesses to
ntb->reg->link_status.

Will this cause the endpoint function driver to read and write corrupted link
status over the PCIe bus on big-endian architectures? The PCIe bus is
inherently little-endian, so it seems this needs cpu_to_le16() or
le16_to_cpu() conversions to avoid breaking device functionality globally on
big-endian endpoint architectures.

> +
> +	/* The link is usable only after both sides have enabled it. */
> +	if (ntb->host_linkup && ntb->vhost_linkup)
> +		link_status |= LINK_STATUS_UP;
> +	else
> +		link_status &= ~LINK_STATUS_UP;
> +
> +	WRITE_ONCE(ntb->reg->link_status, link_status);
> +}
> +
>  /**
> - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
> + * epf_ntb_link_up() - Update the HOST link state
>   * @ntb: NTB device that facilitates communication between HOST and VHOST
> - * @link_up: true or false indicating Link is UP or Down
> + * @link_up: true when the HOST has enabled the link
>   *
> - * Once NTB function in HOST invoke ntb_link_enable(),
> - * this NTB function driver will trigger a link event to VHOST.
> - *
> - * Returns: Zero for success, or an error code in case of failure
> + * Returns: Zero for success
>   */
>  static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
>  {
> -	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_linkup != link_up && ntb->vhost_linkup;
> +		ntb->host_linkup = link_up;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (notify) {
> +		/* Publish link status before completing the HOST command. */
> +		dma_wmb();
> +		ntb_link_event(&ntb->ntb);
> +	}

[Severity: High]
Does this code miss raising an IRQ to notify the HOST?

When the HOST changes the effective state via epf_ntb_link_up(), this notifies
the VHOST but fails to raise an IRQ for the HOST. Could this cause NTB clients
waiting for a link event to hang indefinitely because the event is never
delivered to the side that initiated the change?

>  
> -	ntb_link_event(&ntb->ntb);
>  	return 0;
>  }

[ ... ]

> @@ -1455,11 +1475,34 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
>  	return 0;
>  }
>  
> +static int vntb_epf_set_link(struct epf_ntb *ntb, bool link_up)
> +{
> +	struct pci_epf *epf = ntb->epf;
> +	bool notify;
> +	int ret;
> +
> +	scoped_guard(mutex, &ntb->link_lock) {
> +		notify = ntb->vhost_linkup != link_up && ntb->host_linkup;
> +		ntb->vhost_linkup = link_up;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (!notify)
> +		return 0;
> +
> +	ret = pci_epc_raise_irq(epf->epc, epf->func_no, epf->vfunc_no,
> +				PCI_IRQ_MSI, EPF_IRQ_LINK + 1);

[Severity: High]
Does this code miss calling ntb_link_event(&ntb->ntb) to notify the local
VHOST?

When the VHOST changes the effective link state via vntb_epf_set_link(), the
driver raises an IRQ for the HOST but fails to notify the local VHOST clients.
Similar to epf_ntb_link_up(), could this cause NTB clients blocking on a link
event to hang indefinitely?

> +	if (ret)
> +		dev_err(&epf->dev, "Failed to raise link event IRQ: %d\n", ret);
> +
> +	return ret;
> +}
> +

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

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

[Koichiro Den]

On Fri, Sep 04, 2026 at 07:13:21AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] The driver fails to mutually notify the local NTB client or HOST of effective link state changes, potentially causing clients waiting for link events to hang.

Good catch. This is a pre-existing issue rather than a regression, but my patch
only fixes half of it. I'll cover the remaining notification cases in v2.

Best regards,
Koichiro

> 
> Pre-existing issues:
> - [High] Missing endianness conversions for PCIe shared memory.
> --
> 
> commit ca462c18f2f26ec109681df8452341853193267f
> Author: Koichiro Den <den@valinux.co.jp>
> PCI: endpoint: pci-epf-vntb: Track link state from both sides
> 
> This patch tracks the link state of both HOST and VHOST sides of a virtual
> NTB device separately. It reports the effective link as up only when both
> sides enable it and notifies the other side when this effective state changes.
> 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index c3caec927d748..8af3dcdd893c4 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> 
> [ ... ]
> 
> > @@ -178,24 +182,42 @@ 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);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but the patch propagates a
> pre-existing structural flaw by adding more native endian accesses to
> ntb->reg->link_status.
> 
> Will this cause the endpoint function driver to read and write corrupted link
> status over the PCIe bus on big-endian architectures? The PCIe bus is
> inherently little-endian, so it seems this needs cpu_to_le16() or
> le16_to_cpu() conversions to avoid breaking device functionality globally on
> big-endian endpoint architectures.
> 
> > +
> > +	/* The link is usable only after both sides have enabled it. */
> > +	if (ntb->host_linkup && ntb->vhost_linkup)
> > +		link_status |= LINK_STATUS_UP;
> > +	else
> > +		link_status &= ~LINK_STATUS_UP;
> > +
> > +	WRITE_ONCE(ntb->reg->link_status, link_status);
> > +}
> > +
> >  /**
> > - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
> > + * epf_ntb_link_up() - Update the HOST link state
> >   * @ntb: NTB device that facilitates communication between HOST and VHOST
> > - * @link_up: true or false indicating Link is UP or Down
> > + * @link_up: true when the HOST has enabled the link
> >   *
> > - * Once NTB function in HOST invoke ntb_link_enable(),
> > - * this NTB function driver will trigger a link event to VHOST.
> > - *
> > - * Returns: Zero for success, or an error code in case of failure
> > + * Returns: Zero for success
> >   */
> >  static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
> >  {
> > -	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_linkup != link_up && ntb->vhost_linkup;
> > +		ntb->host_linkup = link_up;
> > +		epf_ntb_update_link(ntb);
> > +	}
> > +
> > +	if (notify) {
> > +		/* Publish link status before completing the HOST command. */
> > +		dma_wmb();
> > +		ntb_link_event(&ntb->ntb);
> > +	}
> 
> [Severity: High]
> Does this code miss raising an IRQ to notify the HOST?
> 
> When the HOST changes the effective state via epf_ntb_link_up(), this notifies
> the VHOST but fails to raise an IRQ for the HOST. Could this cause NTB clients
> waiting for a link event to hang indefinitely because the event is never
> delivered to the side that initiated the change?
> 
> >  
> > -	ntb_link_event(&ntb->ntb);
> >  	return 0;
> >  }
> 
> [ ... ]
> 
> > @@ -1455,11 +1475,34 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
> >  	return 0;
> >  }
> >  
> > +static int vntb_epf_set_link(struct epf_ntb *ntb, bool link_up)
> > +{
> > +	struct pci_epf *epf = ntb->epf;
> > +	bool notify;
> > +	int ret;
> > +
> > +	scoped_guard(mutex, &ntb->link_lock) {
> > +		notify = ntb->vhost_linkup != link_up && ntb->host_linkup;
> > +		ntb->vhost_linkup = link_up;
> > +		epf_ntb_update_link(ntb);
> > +	}
> > +
> > +	if (!notify)
> > +		return 0;
> > +
> > +	ret = pci_epc_raise_irq(epf->epc, epf->func_no, epf->vfunc_no,
> > +				PCI_IRQ_MSI, EPF_IRQ_LINK + 1);
> 
> [Severity: High]
> Does this code miss calling ntb_link_event(&ntb->ntb) to notify the local
> VHOST?
> 
> When the VHOST changes the effective link state via vntb_epf_set_link(), the
> driver raises an IRQ for the HOST but fails to notify the local VHOST clients.
> Similar to epf_ntb_link_up(), could this cause NTB clients blocking on a link
> event to hang indefinitely?
> 
> > +	if (ret)
> > +		dev_err(&epf->dev, "Failed to raise link event IRQ: %d\n", ret);
> > +
> > +	return ret;
> > +}
> > +
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260904065335.3059625-1-den@valinux.co.jp?part=1

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

[Frank Li]

On Fri, Sep 04, 2026 at 03:53:35PM +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 the HOST.
>
> 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 this waiter on the HOST:
>
>   echo N > /sys/kernel/debug/ntb_tool/<H-device>/peer0/link_event
>
> While it is blocked, disable the link on the VHOST:
>
>   echo N > /sys/kernel/debug/ntb_tool/<V-device>/link
>
> Without this patch, the second command succeeds, but peer0/link on the
> HOST remains Y and the waiter does not return.
>
> 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. Notify the other side only when this effective state
> changes. Serialize the two paths because HOST commands run from delayed
> work while the VHOST callbacks may run concurrently.
>
> With this patch, peer0/link on the HOST changes to N and the waiter
> returns.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Note: This is a standalone fix, but it conflicts with the following
> series:
>
>   [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA
>   https://lore.kernel.org/r/20260830151948.3547577-1-den@valinux.co.jp/
>
> That series still has pending issues and will need more work. I will base
> v3 on this fix and make the new link-event IRQ honor the MSI/MSI-X
> selection made by the HOST.
> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 82 +++++++++++++++----
>  1 file changed, 65 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index fba65abfb6b2..51200223f53c 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_linkup;
> +	bool vhost_linkup;
>
>  	/*
>  	 * True when doorbells are interrupt-driven (MSI or embedded), false
> @@ -178,24 +182,42 @@ 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_linkup && ntb->vhost_linkup)
> +		link_status |= LINK_STATUS_UP;
> +	else
> +		link_status &= ~LINK_STATUS_UP;
> +
> +	WRITE_ONCE(ntb->reg->link_status, link_status);
> +}
> +
>  /**
> - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
> + * epf_ntb_link_up() - Update the HOST link state
>   * @ntb: NTB device that facilitates communication between HOST and VHOST
> - * @link_up: true or false indicating Link is UP or Down
> + * @link_up: true when the HOST has enabled the link
>   *
> - * Once NTB function in HOST invoke ntb_link_enable(),
> - * this NTB function driver will trigger a link event to VHOST.
> - *
> - * Returns: Zero for success, or an error code in case of failure
> + * Returns: Zero for success
>   */
>  static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
>  {
> -	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_linkup != link_up && ntb->vhost_linkup;
> +		ntb->host_linkup = link_up;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (notify) {
> +		/* Publish link status before completing the HOST command. */
> +		dma_wmb();
> +		ntb_link_event(&ntb->ntb);
> +	}
>
> -	ntb_link_event(&ntb->ntb);
>  	return 0;
>  }
>
> @@ -320,7 +342,6 @@ 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);
>  		if (ret < 0)
>  			ctrl->command_status = COMMAND_STATUS_ERROR;
> @@ -328,7 +349,6 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>  			ctrl->command_status = COMMAND_STATUS_OK;
>  		goto reset_handler;
>  	case COMMAND_LINK_DOWN:
> -		ntb->linkup = false;
>  		ret = epf_ntb_link_up(ntb, false);
>  		if (ret < 0)
>  			ctrl->command_status = COMMAND_STATUS_ERROR;
> @@ -1456,11 +1476,34 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
>  	return 0;
>  }
>
> +static int vntb_epf_set_link(struct epf_ntb *ntb, bool link_up)
> +{
> +	struct pci_epf *epf = ntb->epf;
> +	bool notify;
> +	int ret;
> +
> +	scoped_guard(mutex, &ntb->link_lock) {
> +		notify = ntb->vhost_linkup != link_up && ntb->host_linkup;
> +		ntb->vhost_linkup = link_up;
> +		epf_ntb_update_link(ntb);
> +	}
> +
> +	if (!notify)
> +		return 0;
> +
> +	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;
> +}
> +
>  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(ntb_ndev(ntb), true);
>  }
>
>  static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
> @@ -1620,7 +1663,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)
> @@ -1638,7 +1681,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(ntb_ndev(ntb), false);
>  }
>
>  static struct device *vntb_epf_get_dma_dev(struct ntb_dev *ndev)
> @@ -1750,6 +1793,10 @@ static int epf_ntb_bind(struct pci_epf *epf)
>  		goto err_bar_alloc;
>  	}
>
> +	ntb->host_linkup = false;
> +	ntb->vhost_linkup = false;
> +	ntb->reg->link_status = 0;
> +
>  	ret = epf_ntb_epc_init(ntb);
>  	if (ret) {
>  		dev_err(dev, "Failed to initialize EPC\n");
> @@ -1833,6 +1880,7 @@ static int epf_ntb_probe(struct pci_epf *epf,
>  	epf->header = &epf_ntb_header;
>  	ntb->epf = epf;
>  	ntb->vbus_number = 0xff;
> +	mutex_init(&ntb->link_lock);

devm_mutex_init();

others look good.

Frank
>
>  	INIT_WORK(&ntb->peer_db_work, vntb_epf_peer_db_work);
>  	disable_work(&ntb->peer_db_work);
> --
> 2.51.0
>

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

[Koichiro Den]

On Fri, Sep 04, 2026 at 10:05:19AM -0500, Frank Li wrote:
> On Fri, Sep 04, 2026 at 03:53:35PM +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 the HOST.
> >
> > 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 this waiter on the HOST:
> >
> >   echo N > /sys/kernel/debug/ntb_tool/<H-device>/peer0/link_event
> >
> > While it is blocked, disable the link on the VHOST:
> >
> >   echo N > /sys/kernel/debug/ntb_tool/<V-device>/link
> >
> > Without this patch, the second command succeeds, but peer0/link on the
> > HOST remains Y and the waiter does not return.
> >
> > 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. Notify the other side only when this effective state
> > changes. Serialize the two paths because HOST commands run from delayed
> > work while the VHOST callbacks may run concurrently.
> >
> > With this patch, peer0/link on the HOST changes to N and the waiter
> > returns.
> >
> > Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > Note: This is a standalone fix, but it conflicts with the following
> > series:
> >
> >   [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA
> >   https://lore.kernel.org/r/20260830151948.3547577-1-den@valinux.co.jp/
> >
> > That series still has pending issues and will need more work. I will base
> > v3 on this fix and make the new link-event IRQ honor the MSI/MSI-X
> > selection made by the HOST.
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 82 +++++++++++++++----
> >  1 file changed, 65 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index fba65abfb6b2..51200223f53c 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_linkup;
> > +	bool vhost_linkup;
> >
> >  	/*
> >  	 * True when doorbells are interrupt-driven (MSI or embedded), false
> > @@ -178,24 +182,42 @@ 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_linkup && ntb->vhost_linkup)
> > +		link_status |= LINK_STATUS_UP;
> > +	else
> > +		link_status &= ~LINK_STATUS_UP;
> > +
> > +	WRITE_ONCE(ntb->reg->link_status, link_status);
> > +}
> > +
> >  /**
> > - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST)
> > + * epf_ntb_link_up() - Update the HOST link state
> >   * @ntb: NTB device that facilitates communication between HOST and VHOST
> > - * @link_up: true or false indicating Link is UP or Down
> > + * @link_up: true when the HOST has enabled the link
> >   *
> > - * Once NTB function in HOST invoke ntb_link_enable(),
> > - * this NTB function driver will trigger a link event to VHOST.
> > - *
> > - * Returns: Zero for success, or an error code in case of failure
> > + * Returns: Zero for success
> >   */
> >  static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
> >  {
> > -	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_linkup != link_up && ntb->vhost_linkup;
> > +		ntb->host_linkup = link_up;
> > +		epf_ntb_update_link(ntb);
> > +	}
> > +
> > +	if (notify) {
> > +		/* Publish link status before completing the HOST command. */
> > +		dma_wmb();
> > +		ntb_link_event(&ntb->ntb);
> > +	}
> >
> > -	ntb_link_event(&ntb->ntb);
> >  	return 0;
> >  }
> >
> > @@ -320,7 +342,6 @@ 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);
> >  		if (ret < 0)
> >  			ctrl->command_status = COMMAND_STATUS_ERROR;
> > @@ -328,7 +349,6 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> >  			ctrl->command_status = COMMAND_STATUS_OK;
> >  		goto reset_handler;
> >  	case COMMAND_LINK_DOWN:
> > -		ntb->linkup = false;
> >  		ret = epf_ntb_link_up(ntb, false);
> >  		if (ret < 0)
> >  			ctrl->command_status = COMMAND_STATUS_ERROR;
> > @@ -1456,11 +1476,34 @@ static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
> >  	return 0;
> >  }
> >
> > +static int vntb_epf_set_link(struct epf_ntb *ntb, bool link_up)
> > +{
> > +	struct pci_epf *epf = ntb->epf;
> > +	bool notify;
> > +	int ret;
> > +
> > +	scoped_guard(mutex, &ntb->link_lock) {
> > +		notify = ntb->vhost_linkup != link_up && ntb->host_linkup;
> > +		ntb->vhost_linkup = link_up;
> > +		epf_ntb_update_link(ntb);
> > +	}
> > +
> > +	if (!notify)
> > +		return 0;
> > +
> > +	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;
> > +}
> > +
> >  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(ntb_ndev(ntb), true);
> >  }
> >
> >  static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
> > @@ -1620,7 +1663,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)
> > @@ -1638,7 +1681,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(ntb_ndev(ntb), false);
> >  }
> >
> >  static struct device *vntb_epf_get_dma_dev(struct ntb_dev *ndev)
> > @@ -1750,6 +1793,10 @@ static int epf_ntb_bind(struct pci_epf *epf)
> >  		goto err_bar_alloc;
> >  	}
> >
> > +	ntb->host_linkup = false;
> > +	ntb->vhost_linkup = false;
> > +	ntb->reg->link_status = 0;
> > +
> >  	ret = epf_ntb_epc_init(ntb);
> >  	if (ret) {
> >  		dev_err(dev, "Failed to initialize EPC\n");
> > @@ -1833,6 +1880,7 @@ static int epf_ntb_probe(struct pci_epf *epf,
> >  	epf->header = &epf_ntb_header;
> >  	ntb->epf = epf;
> >  	ntb->vbus_number = 0xff;
> > +	mutex_init(&ntb->link_lock);
> 
> devm_mutex_init();

Thanks for pointing that out. I will use it in v2.

Also, Sashiko's feedback seems valid. I have tested the updated patch with
ntb_tool again and confirmed that when the VHOST brings the link down, the
link_event waiters on both sides return. I will send v2 shortly.

Best regards,
Koichiro

> 
> others look good.
> 
> Frank
> >
> >  	INIT_WORK(&ntb->peer_db_work, vntb_epf_peer_db_work);
> >  	disable_work(&ntb->peer_db_work);
> > --
> > 2.51.0
> >