On Tue, Nov 12, 2024 at 05:16:23PM -0600, Bjorn Helgaas wrote: > On Fri, Oct 25, 2024 at 03:27:54PM -0700, Keith Busch wrote: > > From: Keith Busch <kbusch@xxxxxxxxxx> > > > > Resetting a bus from an end device only works if it's the only function > > on or below that bus. > > Can we clarify this wording? "Resetting a bus from an end device"? What I mean is, if you find a device you want to reset in the sysfs hierarchy, and find its reset method: /sys/bus/pci/devices/<D:B:D.f>/reset_method If you request "bus", it only works if it is the only function on that bus. I agree my commit message there is a bit unclear, though. I was a bit to deep in that code when I wrote it. > I guess this has something to do with pci_parent_bus_reset(dev), which > declines to call pci_bridge_secondary_bus_reset() if there are any > other devices on the same bus as "dev"? Yep, exactly. > pci_parent_bus_reset() is only called from pci_reset_bus_function(), > which is used by the "bus" and "cxl_bus" reset methods. > > So I guess the point is something like: > > The "bus" and "cxl_bus" reset methods reset a device by asserting > Secondary Bus Reset on the bridge leading to the device. > pci_parent_bus_reset() only allows that if the device is the only > device below the bridge. > > Add a sysfs attribute on bridges that can assert Secondary Bus Reset > regardless of how many devices are below the bridge. This makes it > possible for users to reset multiple devices in a single command. > > I omitted "safely" because this doesn't do any checking to ensure > safety, so I don't know in what sense it is safe. By "safe", what I mean is the device is locked by the kernel, config space is saved and restored on either side of the reset, and the attached driver (if any) is notified this action is about to happen and when it completes so it do whatever quiescent and restoring actions it needs for a bus reset. I can't say this is universally "safe" since it's a bit optomistic to assume everything affected by this action is going to work as the pci driver expects, but the alternatives (setpci) don't coordinate anything with the kernel, so it's "safe" relative to that :) > It seems like this is partly just a convenience, to reset several > devices at once. > > But I think it is *also* a new way to reset devices that we might not > be able to reset otherwise, e.g., if there's more than one device on > the bus, and they don't support ACPI, FLR, or PM reset, there > previously was no reset method that worked at all. Right? Exactly! I have multi-function devices in a switch hierarchy where this unfortunately is really the only way to do it. They don't support resetting individual functions, so SBR is the only way we have to reliably reset the device.
