[PATCH 2/4] PCI/ERR: Add support for resetting the slot in a platforms specific way

[Manivannan Sadhasivam]

On Wed, Apr 16, 2025 at 04:38:01PM +0200, Lukas Wunner wrote:
> On Tue, Apr 15, 2025 at 07:03:17PM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Apr 04, 2025 at 10:46:27AM +0200, Lukas Wunner wrote:
> > > On Fri, Apr 04, 2025 at 01:52:22PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > > When the PCI error handling requires resetting the slot, reset it
> > > > using the host bridge specific 'reset_slot' callback if available
> > > > before calling the 'slot_reset' callback of the PCI drivers.
> > > > 
> > > > The 'reset_slot' callback is responsible for resetting the given slot
> > > > referenced by the 'pci_dev' pointer in a platform specific way and
> > > > bring it back to the working state if possible. If any error occurs
> > > > during the slot reset operation, relevant errno should be returned.
> > > 
> > > This feels like something that should be plumbed into
> > > pcibios_reset_secondary_bus(), rather than pcie_do_recovery().
> > 
> > I did consider that, but didn't go for it since there was no platform
> > reset code present in that function. But I will try to use it as I
> > don't have a strong preference to do reset slot in pcie_do_recovery().
> 
> The only platform overriding pcibios_reset_secondary_bus() is powerpc,
> and it only does that on PowerNV.
> 
> I think you could continue to stick with the approach of adding a
> ->reset_slot() callback to struct pci_host_bridge, but it would
> be good if at the same time you could convert PowerNV to use the
> newly introduced callback as well.  And then remove the way to
> override the reset procedure via pcibios_reset_secondary_bus().
> 
> All pci_host_bridge's which do not define a ->reset_slot() could be
> assigned a default callback which just calls pci_reset_secondary_bus().
> 
> Alternatively, pcibios_reset_secondary_bus() could be made to invoke the
> pci_host_bridge's ->reset_slot() callback if it's not NULL, else
> pci_reset_secondary_bus(). 

Yes. This is what I now tried locally as well.

> And in that case, the __weak attribute
> could be removed as well as the powerpc-specific version of
> pcibios_reset_secondary_bus().
> 

I don't think it is possible to get rid of the powerpc version. It has its own
pci_dev::sysdata pointing to 'struct pci_controller' pointer which is internal
to powerpc arch code. And the generic code would need 'struct pci_host_bridge'
to access the callback.

> I guess what I'm trying to say is, you don't *have* to plumb this
> into pcibios_reset_secondary_bus().  In fact, having a host bridge
> specific callback could be useful if the SoC contains several
> host bridges which require different callbacks to perform a reset.
> 
> I just want to make sure that the code remains maintainable,
> i.e. we don't have two separate ways to override how a bus reset
> is performed.
> 

I think we need to have the override in powerpc anyway. But I will move the
'reset_slot' callback to it and get it called from pci_bus_error_reset() (for
both AER and Link Down).

- Mani

-- 
மணிவண்ணன் சதாசிவம்