Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs
Samiullah Khawaja <[email protected]>
| Newsgroups | org.kernel.vger.linux-pci,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Fri, Aug 14, 2026 at 08:37:37AM -0600, Alex Williamson wrote: >On Thu, 13 Aug 2026 23:22:33 +0000 >Samiullah Khawaja <[email protected]> wrote: > >> > [snip] >> >+ if (pci_num_vf(dev) > 0) { >> >+ pci_dev_unlock(dev); >> >+ return -ENOTTY; >> >+ } >> >> I am wondering whether we should return EAGAIN from here, since this >> function is used by vfio_pci_core_enable() during open and it doesn't >> fail the open on ENOTTY. Basically whether we should allow the user to >> reopen the device if the reset was skipped previously? In the previous >> instance of open, the device was setup with vfio/iommufd and the vfio fd >> was abruptly closed and the reset was skipped. But the device went back >> to the IOMMU default domain and that is probably an Identity domain. If >> we allow the device to be opened and re-enable busmaster without a >> reset, is there a chance that device would continue to DMA based on its >> previous setup/context? Probably unlikely? >> >> Note this is different from the current vfio-pci behaviour where device >> is always reset during close. >> >> Maybe thinking with too much paranoia about it :D. > >Devices are only ever opened into a user owned domain, the IOMMU >context switch happens before this and regardless of the reset. Close >also disables bus-master regardless of reset, so there's no risk of Yes the close side makes sense, I was more concerned about the reopen. >ongoing DMA if the device is placed into an identity domain between >close and re-open. > >Actually, I think -ENOTTY is a leftover from a previous iteration where >this test was pushed into the individual reset methods. -ENOTTY allows >continuing to the next reset method. With the test guarding all the >reset methods in this version, we should probably use -EBUSY. -EAGAIN >would conflate the try-lock contention error, which is actually a usage >race, versus the PF is not in a state to handle the request. This sounds good to me. > >If the user owns the PF, as evidenced by them being able to get to >vfio_pci_core_enable(), and reset is blocked by the SR-IOV state of the >PF, I think there are arguments both that the user implicitly opted in >to the best-effort reset, as well as a use case that allows the PF >driver to fail and re-open the PF demands this behavior. That is fair. Thanks for clarifying. Sami