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

[sashiko-bot@kernel.org]

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