Re: [PATCH v5 2/3] xen/pci: introduce PF<->VF links

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.10.2024 18:50, Stewart Hildebrand wrote:
> On 10/16/24 05:52, Jan Beulich wrote:
>> On 11.10.2024 17:27, Stewart Hildebrand wrote:
>>> Add links between a VF's struct pci_dev and its associated PF struct
>>> pci_dev. Move the calls to pci_get_pdev()/pci_add_device() down to avoid
>>> dropping and re-acquiring the pcidevs_lock().
>>>
>>> During PF removal, unlink VF from PF and mark the VF broken. As before,
>>> VFs may exist without a corresponding PF, although now only with
>>> pdev->broken = true. If the PF is removed and re-added, dom0 is expected
>>> to also remove and re-add the VFs.
>>
>> Right, or else the VF struct instance would remain orphaned the way you've
>> implemented this. Question is whether it is a reasonable assumption that a
>> Dom0 which failed to remove the VFs during PF removal might later
>> "remember" that it still needs to report VFs removed. I for one doubt that.
> 
> This matches my observations: you're right, it most likely won't. I had
> just written it in a misleading way in the commit message. Re-adding
> should not occur until after VFs and PF have been removed in non-buggy
> scenarios.
> 
> A PF driver that fails to disable SR-IOV (i.e remove VFs) during PF
> removal is buggy, and would lead to issues inside dom0 as well due to
> the orphaned/stale VF left behind in Linux dom0 itself. As mentioned in
> patch #1, Linux warns about this:
> 
> [  100.000000]  0000:01:00.0: driver left SR-IOV enabled after remove
> 
> With the PF<->VF links in place, we now also have the ability to detect
> and warn about this potentially buggy condition in Xen. I have so far
> only observed VF-then-PF removal order in non-buggy scenarios. My only
> hesitation with adding such a warning in Xen is that I don't know
> whether removing in PF-then-VF order is legitimate.

My take here is that it's not legitimate. VFs with no PF is physically
impossible, and hence also ought to be logically invalid. It's the PF
that surfaces (materializes) the VFs, after all.

Jan