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

[Bjorn Helgaas <helgaas@kernel.org>]

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