Re: [PATCHv2 2/2] pci: warn if a running device is unaware of reset

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Fri, 2024-11-01 at 13:21 -0600, Keith Busch wrote:
> On Tue, Oct 29, 2024 at 12:27:41PM +0100, Niklas Schnelle wrote:
> > For what it's worth on s390 I think the previous proposal of having the
> > attribute on the pci_bus would have been better in principle. On s390
> > the PCI bus probing is done by firmware and Linux doesn't see a pci_dev
> > for bridges but we do create struct pci_bus for example for a PF and
> > its child VFs.
> > 
> > Then again we can't really do a reset on this level, other than
> > manually going through the PCI functions and resetting them one by one.
> > In fact we may see PCI functions on their own bus while another Linux
> > instance (LPAR) sees other functions from that bus. So yeah, I guess
> > it's fair not to have this attribute but still wanted to offer these
> > thoughts.
> 
> This attribute uses the pci_dev bridge control register. If you don't
> have the bridge device, I don't think this would be useful, so I guess
> you'd have to fallback to resetting individual functions.
> 
> It seems people can navigate /sys/bus/pci/devices/ easier than finding a
> pci_bus under /sys/devices/, though, so that's a plus for pci_dev.
> 

Yes you're right, since the reset via __pci_reset_bus() and then
pci_bridge_secondary_bus_reset() requires the bridge device (unlike
pci_reset_bus()) it makes more sense to have it on the bridge thus also
requiring the bridge device to exist.

I still kind of wish pci_bridge_secondary_bus_reset() and
pcibios_reset_secondary_bus() would take a struct pci_bus and then we
could do the fallback in the latter platform specific code and having
the attribute on the bus would make sense, but I'm not sure it's all
that useful.

One more question though, what would happen with this reset for a bus
with an SR-IOV device with more than 256 VFs i.e. where
pci_iov_virtfn_bus() returns anything other than 0. I'm guessing since
VFs are physically still controlled by the bridge all VFs would be
reset but at the same time virtfn_add_bus() sets the bridge device for
the added bus as NULL so I think it might look odd in sysfs, sadly I
don't have such a device to test with. Still, this might actually be an
argument for having the attribute on the bridge.

Thanks,
Niklas