Re: [PATCH] arm: Enlarge IO_SPACE_LIMIT needed for some SoC

[Ansuel Smith]

On Tue, May 11, 2021 at 02:46:49PM +0200, Ard Biesheuvel wrote:
> On Tue, 11 May 2021 at 14:37, Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
> >
> > On Tue, May 11, 2021 at 02:30:36PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 11 May 2021 at 14:15, Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
> > > >
> > > > On Tue, May 11, 2021 at 06:26:28AM +0200, Ard Biesheuvel wrote:
> > > > > On Tue, 11 May 2021 at 04:32, Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
> > > > > >
> > > > > > On Tue, May 11, 2021 at 03:24:29AM +0100, Matthew Wilcox wrote:
> > > > > > > On Tue, May 11, 2021 at 04:16:54AM +0200, Ansuel Smith wrote:
> > > > > > > > Ipq8064 SoC requires larger IO_SPACE_LIMIT on second and third pci port.
> > > > > > >
> > > > > > > Do you really?  I mean, yes, theoretically, I understand it, the
> > > > > > > hardware supports 64kB of I/O port space per root port.  But I/O
> > > > > > > port space is rather deprecated these days.  My laptop has precisely
> > > > > > > two devices with I/O ports, one with 64 bytes and the other with 32
> > > > > > > bytes.  Would you really suffer by allocating 16kB of I/O port
> > > > > > > space to each root port?
> > > > > >
> > > > > > We were talking about this in the other wrong patch. I also think this
> > > > > > much space looks wrong. The current ipq806x dts have this space so it's
> > > > > > actually broken from a long time. The only reason pci worked before was
> > > > > > because the pci driver didn't actually check if the settings were right.
> > > > > > New kernel introduced more checks and this problem showed up. (to be
> > > > > > more precise, the pci port are commonly used by the ath10k wifi and the
> > > > > > second ath10k wifi fails to init because of this problem)
> > > > > > If you can give me any hint on how to check if the space can be reduced
> > > > > > I would be very happy to investigate it.
> > > > > > In the driver I notice that the max buffer is set to 2k, could be this a
> > > > > > hint?
> > > > > >
> > > > >
> > > > > Could you share the output of lspci -vv from such a system?
> > > > >
> > > > > I agree with Matthew that fiddling with the size of the I/O space
> > > > > range probably papers over another problem, and with the odd
> > > > > exception, no PCIe card used on ARM systems actually uses their I/O
> > > > > BARs, even when they have them. (I used to carry a PCIe serial port
> > > > > card to UEFI plugfests because that was the only thing that would stop
> > > > > working if a system configured its I/O resource window incorrectly)
> > > >
> > > > Here is the output of lspci -vv
> > > >
> > > > 0000:00:00.0 PCI bridge: Qualcomm Device 0101 (prog-if 00 [Normal decode])
> > > >         Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> > > >         I/O behind bridge: [disabled]
> > > >         Memory behind bridge: 08000000-081fffff [size=2M]
> > > >         Prefetchable memory behind bridge: [disabled]
> > >
> > > So this a MMIO window to the endpoint
> > >
> > > ...
> > >
> > > >
> > > > 0000:01:00.0 Network controller: Qualcomm Atheros QCA9984 802.11ac Wave 2 Wireless Network Adapter
> > > >         Region 0: Memory at 08000000 (64-bit, non-prefetchable) [size=2M]
> > >
> > > ... and the endpoint has a single *MMIO* BAR of size 2 MiB.
> > >
> > > This has *nothing* to do with port I/O, which is what you are
> > > modifying with your patch.
> > >
> > > Did you check that the problem exists without the patch, and that the
> > > patch makes it go away?
> > >
> > >
> >
> > Yes without the change to IO_SPACE_LIMIT, the ath10k driver fails to
> > init as it can't access the reg. Only the first pci wifi works but the
> > second one fails to init. By increasing the limit all comes back to
> > normal. What I really can't understand is if the big IO space set in the
> > ipq8064 dtsi was wrong from the start and the ath10k fails to init just
> > because is missconfigured. Any idea how to find the appropriate max IO space
> > for the pci?
> >
> 
> OK, so the entire second host bridge fails to probe, because there is
> no virtual address space left for the I/O window.
> 
> Just change the 'downstream I/O' window size in the DT to 64k
> (0x10000) - I assume the current size (0x100000) is a typo anyway.

Ok, so my fear were right... someone just typo the IO when the dtsi was
pushed and was wrong from all that times. Much easier and cleaner
solution.