Re: [PATCH V2 18/20] accel/amdxdna: Link SR-IOV VFs for power management sequencing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PM device link between PF and VF is lost on VF driver unbind
--

commit e5da3c2c28df6a2bd8639c6fb55cd8abc88ea7e5
Author: David Zhang <yidong.zhang@amd.com>

accel/amdxdna: Link SR-IOV VFs for power management sequencing

This commit adds PM device links between Physical Function (PF) supplier
and Virtual Function (VF) consumers via device_link_add() upon SR-IOV
enablement. The intent is to ensure the PM core enforces proper sequencing
between the PF and VFs.

> diff --git a/drivers/accel/amdxdna/aie4_sriov.c b/drivers/accel/amdxdna/aie4_sriov.c
> index 0eea28f626763..3cad0526087db 100644
> --- a/drivers/accel/amdxdna/aie4_sriov.c
> +++ b/drivers/accel/amdxdna/aie4_sriov.c

[ ... ]

> @@ -56,9 +56,74 @@ int aie4_sriov_stop(struct amdxdna_dev_hdl *ndev)

[ ... ]

> +static int aie4_link_vf(struct amdxdna_dev *xdna, struct pci_dev *pdev_vf)
> +{
> +	struct pci_dev *pdev_pf = to_pci_dev(xdna->ddev.dev);
> +	struct device_link *link;
> +
> +	/*
> +	 * The link is removed when the VF driver unbinds. To rebind VF
> +	 * drivers, re-enable SR-IOV via sriov_numvfs (0, then N).
> +	 */
> +	link = device_link_add(&pdev_vf->dev,   /* consumer = VF */
> +			       &pdev_pf->dev,   /* supplier = PF */
> +			       DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
Does this flag choice break subsequent binds like vfio-pci passthrough?

Using DL_FLAG_AUTOREMOVE_CONSUMER causes the PM device link between the PF
and VF to be permanently deleted when the VF driver is unbound via sysfs.
If the PF is then allowed to enter runtime suspend and the VF is later
rebound to a driver like vfio-pci for virtualization passthrough, the PM
core will not automatically resume the PF. The VF driver will then probe
while the PF hardware is suspended, which can lead to a system crash or
hang.

While the comment notes that rebinding is not supported, any root user
or virtualization management daemon (like libvirt) can still trigger
this standard SR-IOV passthrough use-case because the driver does not set
suppress_bind_attrs = true to block sysfs unbind/bind operations.

> +	if (!link) {
> +		XDNA_ERR(xdna, "Failed to link VF %s", pci_name(pdev_vf));
> +		return -EINVAL;
> +	}
> +
> +	XDNA_DBG(xdna, "Linked VF %s", pci_name(pdev_vf));
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006042230.547807-1-yidong.zhang@amd.com?part=18