On Wed, Jan 22, 2025 at 12:14:02PM -0600, Nishanth Aravamudan wrote:
> On Mon, Jan 13, 2025 at 04:42:00PM -0400, Jason Gunthorpe wrote:
> > On Mon, Jan 06, 2025 at 03:52:31PM -0600, Nishanth Aravamudan wrote:
> > > vfio_pci_ioctl_get_pci_hot_reset_info checks if either the vdev's slot
> > > or bus is not resettable by calling pci_probe_reset_{slot,bus}. Those
> > > functions in turn call pci_{slot,bus}_resettable() to see if the PCI
> > > device supports reset.
> >
> > This change makes sense to me, but..
> >
> > > However, commit d88f521da3ef ("PCI: Allow userspace to query and set
> > > device reset mechanism") added support for userspace to disable reset of
> > > specific PCI devices (by echo'ing "" into reset_method) and
> > > pci_{slot,bus}_resettable methods do not check pci_reset_supported() to
> > > see if userspace has disabled reset. Therefore, if an administrator
> > > disables PCI reset of a specific device, but then uses vfio-pci with
> > > that device (e.g. with qemu), vfio-pci will happily end up issuing a
> > > reset to that device.
> >
> > How does vfio-pci endup issuing a reset? It looked like all the paths
> > are blocked in the pci core with pci_reset_supported()? Is there also
> > a path that vfio is calling that is missing a pci_reset_supported()
> > check? If yes that should probably be fixed in another patch.
>
> This is the path I observed:
You didn't answer the question, I didn't ask about pci_probe_*() I
asked why doesn't pci_reset_supported() directly block the actual
reset?
Should we be adding:
@@ -5919,6 +5919,9 @@ int __pci_reset_bus(struct pci_bus *bus)
*/
int pci_reset_bus(struct pci_dev *pdev)
{
+ if (!pci_reset_supported(pdev))
+ return -EOPNOTSUPP;
+
return (!pci_probe_reset_slot(pdev->slot)) ?
__pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus);
And maybe more?
Jason
