Re: [bug report] lockdep WARN at PCI device rescan

Bjorn Helgaas
2023-12-01T00:37:17Z

On Thu, Nov 30, 2023 at 10:36:01AM +0100, Lukas Wunner wrote:
> On Thu, Nov 30, 2023 at 07:30:56AM +0000, Shinichiro Kawasaki wrote:
> ...

> > Lukas, thank you for the idea. If I understand the comment by Andy
> > correctly, P2SB should not be unhidden between arch_initcall and
> > fs_initcall. Hmm.
> >
> > This made me think: how about to unhide and hide P2SB just during
> > fs_initcall to cache the P2SB resources? To try it, I added a function
> > below on top of the previous trial patch. The added function calls
> > p2sb_bar() for devfn=0 at fs_initcall so that the resource is cached
> > before probe of i2c-i801. This worked
> > good on my system. It avoided the deadlock as well as the lockdep WARN :)
>
> This may work if i2c-i801 is compiled as a module, but may not work
> if it's builtin: It would try to access the cached P2SB BAR when
> it's not yet been cached. So you'd have to return -EPROBE_DEFER
> from p2sb_bar() if it hasn't been cached yet. And you'd have to
> make sure that all the callers can cope with that return value.
>
> Another approach would be to cache the BARs very early at boot in
> arch/x86/kernel/early-quirks.c. That would obviate the need to
> defer probing if the BAR hasn't been cached yet.

I hinted at something like this too:

https://lore.kernel.org/all/20220107010305.GA334966@bhelgaas/

I think these hidden devices are an architectural nightmare. I doubt
firmware folks have signed up to support the OS unhiding and using
them. I think we're going out on a limb by assuming they're fair game
for the OS to use. So I'm ... a bit hesitant to complicate the PCI
core locking, enumeration, and BAR reading code to accommodate them.

> Looking through past discussions archived in lore, I've found an
> important issue raised by Bjorn:
>
> "Apparently this normally hidden device is consuming
> PCI address space. The PCI core needs to know about this. If it
> doesn't, the PCI core may assign this space to another device."
>
> https://lore.kernel.org/all/20210308185212.GA1790506@bjorn-Precision-5520/

I think that was covered because the address space is not included in
the PNP0A03 _CRS, as mentioned in the v2 commit log, right?

https://lore.kernel.org/all/20211221173945.53674-3-andriy.shevchenko@linux.intel.com/

Bjorn