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

[Frank Li <Frank.li@oss.nxp.com>]

On Sat, Sep 05, 2026 at 01:20:49AM +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.
>
> Order a link-status change caused by a HOST command before publishing
> command completion.
>
> 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")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v2:
>   - Notify both NTB clients when the effective link state changes. (Sashiko)
>   - Use devm_mutex_init(). (Frank)
>   - Update the commit message + reproducer accordingly.
>
...
>  /**
> - * 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
> - *
> - * Once NTB function in HOST invoke ntb_link_enable(),
> - * this NTB function driver will trigger a link event to VHOST.
> + * @link_up: 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)
>  {
> -	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();

why need dma_wmb() here?  suppose there are writel() at
pci_epc_raise_irq(), or barrier should be inside pci_epc_raise_irq().

Frank