On Thu, Apr 16, 2026 at 03:16:08PM +0300, Ilpo Järvinen wrote: > On Thu, 16 Apr 2026, Lukas Wunner wrote: > > IOV registers are not saved, they're reconstructed from > > pci_dev->resource[]: [...] > > Bridge windows are saved when portdrv probes (see call to pci_save_state() > > in pcie_portdrv_probe()) and that happens after the fs_initcall() because > > portdrv is registered from a device_initcall(). So those should be fine > > as well. > > It feels a bit odd they're all handled differently (not a problem when it > comes to this patch). Maybe it would make sense to eventually restore them > all from the pci_dev's resource array. Add something like > pci_restore_resources() that would restore all the resources that are > relevant for the type. > > I don't know if that's doable when it comes to the order of things, > especially with sriov, but if doable, it would prevent them ever getting > out of sync again and would avoid the trouble of having to save them > elsewhere.
I did consider restoring standard BARs from pci_dev->resource[], but the calculations and iterations involved looked expensive to me, compared to the cheap copying from pci_dev->saved_config_space[] that we currently do. It didn't look like something that lends itself well for a fix that needs to be backported. A different approach that I'm dwelling on is to amend accessors such as pci_write_config_dword() to update pci_dev->saved_config_space[] implicitly if the offset in config space is < 64. And also implicitly update pci_dev->saved_cap_space[]. The former is easy, the latter isn't. The end goal would be to remove pci_save_state() everywhere, except to populate the saved state once on enumeration. I think originally the idea was, we go to system sleep, so we save state. We resume, we restore state. But then error handling came along and that means we cannot always save the state before reset recovery, so ideally we want to save whenever we write to config space so that cache is never stale. I'm thinking of having a helper in <linux/pci.h> to determine whether a register write needs to be saved. Basically, return true for everything with offset < 64 and for Control registers in capabilities, return false for everything else such as Status registers. By having that in a header, the compiler could deduce at compile time whether saving is necessary or can be skipped. The problem is that to save registers in capabilities, the hlist needs to be searched, which is expensive. Maybe we can speed it up by using an xarray. Or cache the offset of all capabilities whose registers are saved. We already cache e.g. aer_cap, but other offsets are not cached. Hmmmm... Thanks, Lukas
