Bug 11334

Summary: myri10ge: use ioremap_wc: compilation failure on ARM
Product: Platform Specific/Hardware Reporter: Rafael J. Wysocki (rjw)
Component: ARMAssignee: linux-arm-kernel (linux-arm-kernel)
Status: CLOSED CODE_FIX    
Severity: normal CC: bunk
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: commit c7f80993a38f0354a8ad821bcd9335b47a464357 Subsystem:
Regression: Yes Bisected commit-id:
Bug Depends on:    
Bug Blocks: 11167    
Attachments: untested patch moving ioremap_wc's #define to linux/io.h

Description Rafael J. Wysocki 2008-08-14 08:14:56 UTC
Subject    : myri10ge: use ioremap_wc: compilation failure on ARM
Submitter  : Martin Michlmayr <tbm@cyrius.com>
Date       : 2008-08-10 11:25
References : http://marc.info/?l=linux-netdev&m=121836771727632&w=2
Handled-By : Brice Goglin <brice@myri.com>
Handled-By : Russell King <rmk+lkml@arm.linux.org.uk>

This entry is being used for tracking a regression from 2.6.26.  Please don't
close it until the problem is fixed in the mainline.
Comment 1 Rafael J. Wysocki 2008-08-14 08:16:25 UTC
Caused by:

commit c7f80993a38f0354a8ad821bcd9335b47a464357
Author: Brice Goglin <brice@myri.com>
Date:   Mon Jul 21 10:26:25 2008 +0200

    myri10ge: use ioremap_wc
Comment 2 Rafael J. Wysocki 2008-08-14 09:23:31 UTC
Not-Handled-By : Russell King <rmk+lkml@arm.linux.org.uk>
Comment 3 Brice Goglin 2008-08-16 14:47:44 UTC
Not-Handled-By: Brice Goglin <brice@myri.com>

There's nothing to fix in myri10ge, we're not going to add some #ifdef'ery in a driver because some arch fails to provide ioremap_wc. As discussed earlier, the dumb fix is to include <asm-generic/iomap.h> in asm-arm/io.h, but this is not going to happen. So, unless ioremap_wc() gets implemented on ARM, asm-arm/io.h needs some kind of #define ioremap_wc ioremap_nocache.
Comment 4 Rafael J. Wysocki 2008-08-17 05:31:52 UTC
On Sunday, 17 of August 2008, Martin Michlmayr wrote:
> * Rafael J. Wysocki <rjw@sisk.pl> [2008-08-16 21:02]:
> > The following bug entry is on the current list of known regressions
> > from 2.6.26.  Please verify if it still should be listed and let me know
> > (either way).
> 
> This is still there.
Comment 5 Adrian Bunk 2008-08-19 05:52:55 UTC
Martin explained to me that this is actually an ARM problem, any my reassigning was therefore wrong.
Comment 6 Rafael J. Wysocki 2008-08-24 14:02:03 UTC
On Sunday, 24 of August 2008, Martin Michlmayr wrote:
> * Rafael J. Wysocki <rjw@sisk.pl> [2008-08-23 20:10]:
> > This message has been generated automatically as a part of a report
> > of recent regressions.
> > 
> > The following bug entry is on the current list of known regressions
> > from 2.6.26.  Please verify if it still should be listed and let me know
> > (either way).
> 
> Yes, this is still there.
Comment 7 Brice Goglin 2008-08-26 22:40:18 UTC
This problem is not ARM specific, FRV (iirc) has the same problem. It's more about asm-generic/iomap.h not being included by all architectures.

As a (temporary?) workaround, could we move
  #ifndef ARCH_HAS_IOREMAP_WC
  #define ioremap_wc ioremap_nocache
  #endif
from include/asm-generic/iomap.h to include/linux/io.h?

People using ioremap_wc() are supposed to get it from linux/io.h, right? So moving this #define there should keep it for working architectures and add it for broken ones.
Comment 8 Brice Goglin 2008-08-26 22:41:12 UTC
Created attachment 17474 [details]
untested patch moving ioremap_wc's #define to linux/io.h
Comment 9 Anonymous Emailer 2008-08-27 02:35:06 UTC
Reply-To: linux@arm.linux.org.uk

On Tue, Aug 26, 2008 at 10:40:19PM -0700, bugme-daemon@bugzilla.kernel.org wrote:
> ------- Comment #7 from Brice.Goglin@ens-lyon.org  2008-08-26 22:40 -------
> This problem is not ARM specific, FRV (iirc) has the same problem. It's more
> about asm-generic/iomap.h not being included by all architectures.
> 
> As a (temporary?) workaround, could we move
>   #ifndef ARCH_HAS_IOREMAP_WC
>   #define ioremap_wc ioremap_nocache
>   #endif
> from include/asm-generic/iomap.h to include/linux/io.h?

Before the problem became visible (on mailing lists to me), Lennert
and myself discussed the problem and I gave Lennert some pointers
about solving it for ARM - which involved introducing a new MT_*
value so that we can setup the page tables correctly.

When it became visible, I responded via LKML saying that Lennert was
working on it.  However, lately there's been very little contact
with Lennert...

What I'll say is that any solution which involves including
asm-generic/iomap.h into linux/io.h is not acceptable.  Moving
that conditional definition as suggested above would be an ideal
solution - that's where it _should_ have been in the first place.
Comment 10 Adrian Bunk 2008-08-28 08:42:34 UTC
(In reply to comment #7)
> This problem is not ARM specific, FRV (iirc) has the same problem. It's more
> about asm-generic/iomap.h not being included by all architectures.
>...

FRV got fixed by commit 0c7281c0faa1d0bdbdc647430cbdf7e0aed7f385

I've checked all architectures, and the only architecture that suffers from this myri10ge compile error in Linus' tree now is ARM (ignoring architectures that offer a CONFIG_PCI that does not compile at all).
Comment 11 Anonymous Emailer 2008-08-28 08:53:07 UTC
Reply-To: linux@arm.linux.org.uk

On Thu, Aug 28, 2008 at 08:42:34AM -0700, bugme-daemon@bugzilla.kernel.org wrote:
> (In reply to comment #7)
> > This problem is not ARM specific, FRV (iirc) has the same problem. It's
> more
> > about asm-generic/iomap.h not being included by all architectures.
> >...
> 
> FRV got fixed by commit 0c7281c0faa1d0bdbdc647430cbdf7e0aed7f385
> 
> I've checked all architectures, and the only architecture that suffers from
> this myri10ge compile error in Linus' tree now is ARM (ignoring architectures
> that offer a CONFIG_PCI that does not compile at all).

And we're still wondering what's happened to Lennert who was working
on it.

If people stopped behaving like little kiddies and using /kill on IRC
when they see something they don't like, then maybe I'd know what
Lennert was up to.  Unfortunately, since that incident, there's been
no sign of Lennert on IRC, nor has he been particularly responsive
by email.

So... there's no current timescale on this getting fixed at present,
and all these bugzilla mails are driving me nuts.
Comment 12 Rafael J. Wysocki 2008-08-31 04:34:05 UTC
On Sunday, 31 of August 2008, Martin Michlmayr wrote:
> * Rafael J. Wysocki <rjw@sisk.pl> [2008-08-30 21:50]:
> > The following bug entry is on the current list of known regressions
> > from 2.6.26.  Please verify if it still should be listed and let me know
> > (either way).
> 
> It's still there.
Comment 13 Lennert Buytenhek 2008-09-04 19:15:42 UTC
On Thu, Aug 28, 2008 at 04:52:55PM +0100, Russell King - ARM Linux wrote:

> > > This problem is not ARM specific, FRV (iirc) has the same problem. It's
> more
> > > about asm-generic/iomap.h not being included by all architectures.
> > >...
> > 
> > FRV got fixed by commit 0c7281c0faa1d0bdbdc647430cbdf7e0aed7f385
> > 
> > I've checked all architectures, and the only architecture that suffers from
> > this myri10ge compile error in Linus' tree now is ARM (ignoring
> architectures
> > that offer a CONFIG_PCI that does not compile at all).
> 
> And we're still wondering what's happened to Lennert who was working
> on it.

I fell off the face of the planet for a while -- sorry about that.

How does this look?  Boot tested on an ARMv5 box, but I currently
don't have xsc3 or ARMv6 hardware available so I'd appreciate if
someone could test this on one of those platforms.



From: Lennert Buytenhek <buytenh@wantstofly.org>
Subject: [ARM] provide ioremap_wc()

This patch provides an ARM implementation of ioremap_wc().

We use different page table attributes depending on which CPU we
are running on:

- Non-XScale ARMv5 and earlier systems: The ARMv5 ARM documents four
  possible mapping types (CB=00/01/10/11).  We can't use any of the
  cached memory types (CB=10/11), since that breaks coherency with
  peripheral devices.  Both CB=00 and CB=01 are suitable for _wc, and
  CB=01 (Uncached/Buffered) allows the hardware more freedom than
  CB=00, so we'll use that.

  (The ARMv5 ARM seems to suggest that CB=01 is allowed to delay stores
  but isn't allowed to merge them, but there is no other mapping type
  we can use that allows the hardware to delay and merge stores, so
  we'll go with CB=01.)

- XScale v1/v2 (ARMv5): same as the ARMv5 case above, with the slight
  difference that on these platforms, CB=01 actually _does_ allow
  merging stores.  (If you want noncoalescing bufferable behavior
  on Xscale v1/v2, you need to use XCB=101.)

- Xscale v3 (ARMv5) and ARMv6+: on these systems, we use TEXCB=00100
  mappings (Inner/Outer Uncacheable in xsc3 parlance, Uncached Normal
  in ARMv6 parlance).

  The ARMv6 ARM explicitly says that any accesses to Normal memory can
  be merged, which makes Normal memory more suitable for _wc mappings
  than Device or Strongly Ordered memory, as the latter two mapping
  types are guaranteed to maintain transaction number, size and order.
  We use the Uncached variety of Normal mappings for the same reason
  that we can't use C=1 mappings on ARMv5.

  The xsc3 Architecture Specification documents TEXCB=00100 as being
  Uncacheable and allowing coalescing of writes, which is also just
  what we need.

Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>

Index: linux-2.6/arch/arm/include/asm/io.h
===================================================================
--- linux-2.6.orig/arch/arm/include/asm/io.h
+++ linux-2.6/arch/arm/include/asm/io.h
@@ -61,8 +61,9 @@ extern void __raw_readsl(const void __io
 #define MT_DEVICE_NONSHARED	1
 #define MT_DEVICE_CACHED	2
 #define MT_DEVICE_IXP2000	3
+#define MT_DEVICE_WC		4
 /*
- * types 4 onwards can be found in asm/mach/map.h and are undefined
+ * types 5 onwards can be found in asm/mach/map.h and are undefined
  * for ioremap
  */
 
@@ -215,11 +216,13 @@ extern void _memset_io(volatile void __i
 #define ioremap(cookie,size)		__arm_ioremap(cookie, size, MT_DEVICE)
 #define ioremap_nocache(cookie,size)	__arm_ioremap(cookie, size, MT_DEVICE)
 #define ioremap_cached(cookie,size)	__arm_ioremap(cookie, size, MT_DEVICE_CACHED)
+#define ioremap_wc(cookie,size)		__arm_ioremap(cookie, size, MT_DEVICE_WC)
 #define iounmap(cookie)			__iounmap(cookie)
 #else
 #define ioremap(cookie,size)		__arch_ioremap((cookie), (size), MT_DEVICE)
 #define ioremap_nocache(cookie,size)	__arch_ioremap((cookie), (size), MT_DEVICE)
 #define ioremap_cached(cookie,size)	__arch_ioremap((cookie), (size), MT_DEVICE_CACHED)
+#define ioremap_wc(cookie,size)		__arch_ioremap((cookie), (size), MT_DEVICE_WC)
 #define iounmap(cookie)			__arch_iounmap(cookie)
 #endif
 
Index: linux-2.6/arch/arm/include/asm/mach/map.h
===================================================================
--- linux-2.6.orig/arch/arm/include/asm/mach/map.h
+++ linux-2.6/arch/arm/include/asm/mach/map.h
@@ -18,13 +18,13 @@ struct map_desc {
 	unsigned int type;
 };
 
-/* types 0-3 are defined in asm/io.h */
-#define MT_CACHECLEAN		4
-#define MT_MINICLEAN		5
-#define MT_LOW_VECTORS		6
-#define MT_HIGH_VECTORS		7
-#define MT_MEMORY		8
-#define MT_ROM			9
+/* types 0-4 are defined in asm/io.h */
+#define MT_CACHECLEAN		5
+#define MT_MINICLEAN		6
+#define MT_LOW_VECTORS		7
+#define MT_HIGH_VECTORS		8
+#define MT_MEMORY		9
+#define MT_ROM			10
 
 #define MT_NONSHARED_DEVICE	MT_DEVICE_NONSHARED
 #define MT_IXP2000_DEVICE	MT_DEVICE_IXP2000
Index: linux-2.6/arch/arm/mm/mmu.c
===================================================================
--- linux-2.6.orig/arch/arm/mm/mmu.c
+++ linux-2.6/arch/arm/mm/mmu.c
@@ -211,6 +211,12 @@ static struct mem_type mem_types[] = {
 				  PMD_SECT_TEX(1),
 		.domain		= DOMAIN_IO,
 	},
+	[MT_DEVICE_WC] = {	/* ioremap_wc */
+		.prot_pte	= PROT_PTE_DEVICE,
+		.prot_l1	= PMD_TYPE_TABLE,
+		.prot_sect	= PROT_SECT_DEVICE,
+		.domain		= DOMAIN_IO,
+	},
 	[MT_CACHECLEAN] = {
 		.prot_sect = PMD_TYPE_SECT | PMD_SECT_XN,
 		.domain    = DOMAIN_KERNEL,
@@ -273,6 +279,20 @@ static void __init build_mem_type_table(
 	}
 
 	/*
+	 * On non-Xscale3 ARMv5-and-older systems, use CB=01
+	 * (Uncached/Buffered) for ioremap_wc() mappings.  On XScale3
+	 * and ARMv6+, use TEXCB=00100 mappings (Inner/Outer Uncacheable
+	 * in xsc3 parlance, Uncached Normal in ARMv6 parlance).
+	 */
+	if (cpu_is_xsc3() || cpu_arch >= CPU_ARCH_ARMv6) {
+		mem_types[MT_DEVICE_WC].prot_pte_ext |= PTE_EXT_TEX(1);
+		mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_TEX(1);
+	} else {
+		mem_types[MT_DEVICE_WC].prot_pte |= L_PTE_BUFFERABLE;
+		mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_BUFFERABLE;
+	}
+
+	/*
 	 * ARMv5 and lower, bit 4 must be set for page tables.
 	 * (was: cache "update-able on write" bit on ARM610)
 	 * However, Xscale cores require this bit to be cleared.
Comment 14 Anonymous Emailer 2008-09-05 02:44:26 UTC
Reply-To: linux@arm.linux.org.uk

On Fri, Sep 05, 2008 at 04:15:03AM +0200, Lennert Buytenhek wrote:
> I fell off the face of the planet for a while -- sorry about that.

That sounds a little extreme for when someone uses /kill on irc!

> How does this look?  Boot tested on an ARMv5 box, but I currently
> don't have xsc3 or ARMv6 hardware available so I'd appreciate if
> someone could test this on one of those platforms.

Looks fine.  Given that almost nothing at present uses ioremap_wc()
I'd say boot testing on ARMv5 is sufficient to confirm that the
other mapping types are still working, so let's go with it.

Please submit.  Thanks.
Comment 15 Rafael J. Wysocki 2008-09-05 03:37:38 UTC
Handled-By : Lennert Buytenhek <buytenh@wantstofly.org>
Patch : http://bugzilla.kernel.org/show_bug.cgi?id=11334#c13
Comment 16 Brice Goglin 2008-09-06 16:21:21 UTC
Thanks but this patch is too big for 2.6.27, right? We probably need something like just adding
  #define ioremap_wc ioremap
in arm's specific io headers.
Comment 17 Anonymous Emailer 2008-09-06 16:37:46 UTC
Reply-To: linux@arm.linux.org.uk

On Sat, Sep 06, 2008 at 04:21:21PM -0700, bugme-daemon@bugzilla.kernel.org wrote:
> Thanks but this patch is too big for 2.6.27, right? We probably need
> something like just adding
>   #define ioremap_wc ioremap
> in arm's specific io headers.

Patch is fine, and already queued.
Comment 18 Rafael J. Wysocki 2008-09-07 14:55:20 UTC
On Sunday, 7 of September 2008, Lennert Buytenhek wrote:
> On Sat, Sep 06, 2008 at 11:30:38PM +0200, Rafael J. Wysocki wrote:
> 
> > This message has been generated automatically as a part of a report
> > of recent regressions.
> > 
> > The following bug entry is on the current list of known regressions
> > from 2.6.26.  Please verify if it still should be listed and let me know
> > (either way).
> > 
> > 
> > Bug-Entry   : http://bugzilla.kernel.org/show_bug.cgi?id=11334
> > Subject             : myri10ge: use ioremap_wc: compilation failure on ARM
> > Submitter   : Martin Michlmayr <tbm@cyrius.com>
> > Date                : 2008-08-10 11:25 (28 days old)
> > References  : http://marc.info/?l=linux-netdev&m=121836771727632&w=2
> > Handled-By  : Lennert Buytenhek <buytenh@wantstofly.org>
> > Patch               : http://bugzilla.kernel.org/show_bug.cgi?id=11334#c13
> 
> Still relevant.  A patch to fix this has been submitted and should be in
> the ARM tree, but it has not made its way upstream yet, and the original
> submitter has not confirmed that it fixes the problem yet.