diff mbox series

[RESEND] arm: Enlarge IO_SPACE_LIMIT needed for some SoC

Message ID 20210508175537.202-1-ansuelsmth@gmail.com (mailing list archive)
State New, archived
Headers show
Series [RESEND] arm: Enlarge IO_SPACE_LIMIT needed for some SoC | expand

Commit Message

Ansuel Smith May 8, 2021, 5:55 p.m. UTC
Ipq8064 SoC requires larger IO_SPACE_LIMIT or second and third pci port
fails to register the IO addresses and connected device doesn't work.

Cc: <stable@vger.kernel.org> # 4.9+
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 arch/arm/include/asm/io.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Russell King (Oracle) May 8, 2021, 6:50 p.m. UTC | #1
On Sat, May 08, 2021 at 07:55:35PM +0200, Ansuel Smith wrote:
> Ipq8064 SoC requires larger IO_SPACE_LIMIT or second and third pci port
> fails to register the IO addresses and connected device doesn't work.
> 
> Cc: <stable@vger.kernel.org> # 4.9+
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

I don't see any consideration of whether this increase results in any
clashes with any other related areas. Also, there is no update of the
memory layout documentation.

The memory layout documentation says:

=============== =============== ===============================================
Start           End             Use
=============== =============== ===============================================
fee00000        feffffff        Mapping of PCI I/O space. This is a static
                                mapping within the vmalloc space.

which means there's a maximum of 0x001fffff available. You are
increasing it's size from 0x000fffff to 0x00ffffff. This means it
expands from 0xfee00000 through to 0xffdfffff.

This conflicts with these entries:

ffc80000        ffefffff        Fixmap mapping region.  Addresses provided
                                by fix_to_virt() will be located here.

ffc00000        ffc7ffff        Guard region

ff800000        ffbfffff        Permanent, fixed read-only mapping of the
                                firmware provided DT blob

So, I have no option but to NAK this change. Sorry.

You can find this documentation in the "Documentation" subdirectory.
Ansuel Smith May 8, 2021, 10:51 p.m. UTC | #2
On Sat, May 08, 2021 at 07:50:44PM +0100, Russell King - ARM Linux admin wrote:
> On Sat, May 08, 2021 at 07:55:35PM +0200, Ansuel Smith wrote:
> > Ipq8064 SoC requires larger IO_SPACE_LIMIT or second and third pci port
> > fails to register the IO addresses and connected device doesn't work.
> > 
> > Cc: <stable@vger.kernel.org> # 4.9+
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> 
> I don't see any consideration of whether this increase results in any
> clashes with any other related areas. Also, there is no update of the
> memory layout documentation.
> 
> The memory layout documentation says:
> 
> =============== =============== ===============================================
> Start           End             Use
> =============== =============== ===============================================
> fee00000        feffffff        Mapping of PCI I/O space. This is a static
>                                 mapping within the vmalloc space.
> 
> which means there's a maximum of 0x001fffff available. You are
> increasing it's size from 0x000fffff to 0x00ffffff. This means it
> expands from 0xfee00000 through to 0xffdfffff.
> 
> This conflicts with these entries:
> 
> ffc80000        ffefffff        Fixmap mapping region.  Addresses provided
>                                 by fix_to_virt() will be located here.
> 
> ffc00000        ffc7ffff        Guard region
> 
> ff800000        ffbfffff        Permanent, fixed read-only mapping of the
>                                 firmware provided DT blob
> 
> So, I have no option but to NAK this change. Sorry.
> 
> You can find this documentation in the "Documentation" subdirectory.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Hi,
Thanks a lot for the review and sorry for the mess. Just to make sure I
don't push a very wrong patch another time. ipq8064 require 0x300000 of
IO space if all 3 lines are used. From what I can read in the
documentation, the PCI I/O mapping section does have space and can be
expanded to ff0fffff without causing collision. So I have to update that
part and the IO_LIMIT to 0x2fffff. Tell me if I'm completely wrong and
again, thanks for the review.
Russell King (Oracle) May 9, 2021, 1:09 p.m. UTC | #3
On Sun, May 09, 2021 at 12:51:20AM +0200, Ansuel Smith wrote:
> On Sat, May 08, 2021 at 07:50:44PM +0100, Russell King - ARM Linux admin wrote:
> > On Sat, May 08, 2021 at 07:55:35PM +0200, Ansuel Smith wrote:
> > > Ipq8064 SoC requires larger IO_SPACE_LIMIT or second and third pci port
> > > fails to register the IO addresses and connected device doesn't work.
> > > 
> > > Cc: <stable@vger.kernel.org> # 4.9+
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > 
> > I don't see any consideration of whether this increase results in any
> > clashes with any other related areas. Also, there is no update of the
> > memory layout documentation.
> > 
> > The memory layout documentation says:
> > 
> > =============== =============== ===============================================
> > Start           End             Use
> > =============== =============== ===============================================
> > fee00000        feffffff        Mapping of PCI I/O space. This is a static
> >                                 mapping within the vmalloc space.
> > 
> > which means there's a maximum of 0x001fffff available. You are
> > increasing it's size from 0x000fffff to 0x00ffffff. This means it
> > expands from 0xfee00000 through to 0xffdfffff.
> > 
> > This conflicts with these entries:
> > 
> > ffc80000        ffefffff        Fixmap mapping region.  Addresses provided
> >                                 by fix_to_virt() will be located here.
> > 
> > ffc00000        ffc7ffff        Guard region
> > 
> > ff800000        ffbfffff        Permanent, fixed read-only mapping of the
> >                                 firmware provided DT blob
> > 
> > So, I have no option but to NAK this change. Sorry.
> > 
> > You can find this documentation in the "Documentation" subdirectory.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
> Hi,
> Thanks a lot for the review and sorry for the mess. Just to make sure I
> don't push a very wrong patch another time. ipq8064 require 0x300000 of
> IO space if all 3 lines are used. From what I can read in the
> documentation, the PCI I/O mapping section does have space and can be
> expanded to ff0fffff without causing collision. So I have to update that
> part and the IO_LIMIT to 0x2fffff. Tell me if I'm completely wrong and
> again, thanks for the review.

Well, an obvious question would be: do you really need that much IO
space? PCs have got away with 64K of IO space without having a problem
for years, so 64K per bus should be fine. If you have 3 buses, that
should be 3 * 64K or 192K.

I would bet in the normal circumstance that the IO space on every PCI
bus is very sparsely used, so using 1MiB per bus seems over the top.

That said, using 1MB each for three buses takes the top of the IO space
to 0xff100000, which shouldn't be a problem, assuming memory.rst is up
to date.

In any case, I really don't think such a change has anything to do with
stable kernels, so please drop that for your next submission.
David Laight May 9, 2021, 4:54 p.m. UTC | #4
...
> Well, an obvious question would be: do you really need that much IO
> space? PCs have got away with 64K of IO space without having a problem
> for years, so 64K per bus should be fine. If you have 3 buses, that
> should be 3 * 64K or 192K.

Not only that, but most modern PCI (even) cards don't require IO space.
Or does this include memory space - which can be much bigger.

I have cut one of our FPGA PCIe slaves down from two 8MB BARs to
two 1MB ones, with the extra small BARs (eg for MSI-X) the bridge
would have allocated 32MB, now 4MB.
Fortunately that card only needs to work on x86-64.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index fc748122f1e0..6f3e89f08bd8 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -197,7 +197,7 @@  void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size);
 #ifdef CONFIG_NEED_MACH_IO_H
 #include <mach/io.h>
 #elif defined(CONFIG_PCI)
-#define IO_SPACE_LIMIT	((resource_size_t)0xfffff)
+#define IO_SPACE_LIMIT	((resource_size_t)0xffffff)
 #define __io(a)		__typesafe_io(PCI_IO_VIRT_BASE + ((a) & IO_SPACE_LIMIT))
 #else
 #define __io(a)		__typesafe_io((a) & IO_SPACE_LIMIT)