| Message ID | 20250207205600.1846178-1-naravamudan@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] PCI: account for sysfs-disabled reset in pci_{slot,bus}_resettable() | expand |
On Fri, Feb 07, 2025 at 02:56:00PM -0600, Nishanth Aravamudan wrote: > 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. > > __pci_reset_bus() > -> pci_bus_reset(..., PCI_RESET_PROBE) > -> pci_bus_resettable() > > __pci_reset_slot() > -> pci_slot_reset(..., PCI_RESET_PROBE) > -> pci_slot_resettable() > > pci_reset_bus() > -> pci_probe_reset_slot() > -> pci_slot_reset(..., PCI_RESET_PROBE) > -> pci_bus_resettable() > if true: > __pci_reset_slot() > else: > __pci_reset_bus() > > I was able to reproduce this issue with a vfio device passed to a qemu > guest, where I had disabled PCI reset via sysfs. Both > vfio_pci_ioctl_get_pci_hot_reset_info() and > vfio_pci_ioctl_pci_hot_reset() check if either the vdev's slot or bus is > not resettable by calling pci_probe_reset_slot(). Before my change, this > ends up ignoring the sysfs file contents and vfio-pci happily ends up > issuing a reset to that device. > > Add an explicit check of pci_reset_supported() in both > pci_slot_resettable() and pci_bus_resettable() to ensure both the reset > status and reset execution are both bypassed if an administrator > disables it for a device. > > Fixes: d88f521da3ef ("PCI: Allow userspace to query and set device reset mechanism") > Signed-off-by: Nishanth Aravamudan <naravamudan@nvidia.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Raphael Norwitz <raphael.norwitz@nutanix.com> > Cc: Amey Narkhede <ameynarkhede03@gmail.com> > Cc: linux-pci@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: Jason Gunthorpe <jgg@nvidia.com> > Cc: Yishai Hadas <yishaih@nvidia.com> > Cc: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > Cc: Kevin Tian <kevin.tian@intel.com> > Cc: kvm@vger.kernel.org Applied to pci/reset for v6.15, thanks! > --- > > Changes since v2: > - update commit message to include more details > > Changes since v1: > - fix capitalization and ()s > - clarify same checks are done in reset path > > drivers/pci/pci.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 869d204a70a3..738d29375ad3 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5405,6 +5405,8 @@ static bool pci_bus_resettable(struct pci_bus *bus) > return false; > > list_for_each_entry(dev, &bus->devices, bus_list) { > + if (!pci_reset_supported(dev)) > + return false; > if (dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET || > (dev->subordinate && !pci_bus_resettable(dev->subordinate))) > return false; > @@ -5481,6 +5483,8 @@ static bool pci_slot_resettable(struct pci_slot *slot) > list_for_each_entry(dev, &slot->bus->devices, bus_list) { > if (!dev->slot || dev->slot != slot) > continue; > + if (!pci_reset_supported(dev)) > + return false; > if (dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET || > (dev->subordinate && !pci_bus_resettable(dev->subordinate))) > return false; > -- > 2.34.1 >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 869d204a70a3..738d29375ad3 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5405,6 +5405,8 @@ static bool pci_bus_resettable(struct pci_bus *bus) return false; list_for_each_entry(dev, &bus->devices, bus_list) { + if (!pci_reset_supported(dev)) + return false; if (dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET || (dev->subordinate && !pci_bus_resettable(dev->subordinate))) return false; @@ -5481,6 +5483,8 @@ static bool pci_slot_resettable(struct pci_slot *slot) list_for_each_entry(dev, &slot->bus->devices, bus_list) { if (!dev->slot || dev->slot != slot) continue; + if (!pci_reset_supported(dev)) + return false; if (dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET || (dev->subordinate && !pci_bus_resettable(dev->subordinate))) return false;
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. __pci_reset_bus() -> pci_bus_reset(..., PCI_RESET_PROBE) -> pci_bus_resettable() __pci_reset_slot() -> pci_slot_reset(..., PCI_RESET_PROBE) -> pci_slot_resettable() pci_reset_bus() -> pci_probe_reset_slot() -> pci_slot_reset(..., PCI_RESET_PROBE) -> pci_bus_resettable() if true: __pci_reset_slot() else: __pci_reset_bus() I was able to reproduce this issue with a vfio device passed to a qemu guest, where I had disabled PCI reset via sysfs. Both vfio_pci_ioctl_get_pci_hot_reset_info() and vfio_pci_ioctl_pci_hot_reset() check if either the vdev's slot or bus is not resettable by calling pci_probe_reset_slot(). Before my change, this ends up ignoring the sysfs file contents and vfio-pci happily ends up issuing a reset to that device. Add an explicit check of pci_reset_supported() in both pci_slot_resettable() and pci_bus_resettable() to ensure both the reset status and reset execution are both bypassed if an administrator disables it for a device. Fixes: d88f521da3ef ("PCI: Allow userspace to query and set device reset mechanism") Signed-off-by: Nishanth Aravamudan <naravamudan@nvidia.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Raphael Norwitz <raphael.norwitz@nutanix.com> Cc: Amey Narkhede <ameynarkhede03@gmail.com> Cc: linux-pci@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: Jason Gunthorpe <jgg@nvidia.com> Cc: Yishai Hadas <yishaih@nvidia.com> Cc: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Cc: Kevin Tian <kevin.tian@intel.com> Cc: kvm@vger.kernel.org --- Changes since v2: - update commit message to include more details Changes since v1: - fix capitalization and ()s - clarify same checks are done in reset path drivers/pci/pci.c | 4 ++++ 1 file changed, 4 insertions(+)