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 <[email protected]> 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/[email protected]?part=18
