[PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang

[PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang

[Arnd Bergmann]

clang ignores the -mfpu=neon flag when building with -march=armv6:

In file included from lib/raid6/neon1.c:27:
clang/3.8.0/include/arm_neon.h:28:2: error: "NEON support not enabled"

There is no real need to build the file with -march=armv6 in a
multi-CPU enabled kernel, as nothing in here will ever get called
on an ARMv6 CPU. Adding -march=armv7 doesn't hurt and can only
improve the code quality.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/lib/Makefile | 2 +-
 lib/raid6/Makefile    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index d8a780799506..a86c6c8e0648 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -35,7 +35,7 @@ $(obj)/csumpartialcopy.o:	$(obj)/csumpartialcopygeneric.S
 $(obj)/csumpartialcopyuser.o:	$(obj)/csumpartialcopygeneric.S
 
 ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
-  NEON_FLAGS			:= -mfloat-abi=softfp -mfpu=neon
+  NEON_FLAGS			:= -mfloat-abi=softfp -mfpu=neon -march=armv7-a
   CFLAGS_xor-neon.o		+= $(NEON_FLAGS)
   obj-$(CONFIG_XOR_BLOCKS)	+= xor-neon.o
 endif
diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
index 3b10a48fa040..4ef3e800fb39 100644
--- a/lib/raid6/Makefile
+++ b/lib/raid6/Makefile
@@ -23,7 +23,7 @@ endif
 ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
 NEON_FLAGS := -ffreestanding
 ifeq ($(ARCH),arm)
-NEON_FLAGS += -mfloat-abi=softfp -mfpu=neon
+NEON_FLAGS += -mfloat-abi=softfp -mfpu=neon -march=armv7-a
 endif
 ifeq ($(ARCH),arm64)
 CFLAGS_REMOVE_neon1.o += -mgeneral-regs-only
-- 
2.7.0

Re: [PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang

[Nicolas Pitre]

On Thu, 18 Feb 2016, Arnd Bergmann wrote:

> clang ignores the -mfpu=neon flag when building with -march=armv6:
> 
> In file included from lib/raid6/neon1.c:27:
> clang/3.8.0/include/arm_neon.h:28:2: error: "NEON support not enabled"
> 
> There is no real need to build the file with -march=armv6 in a
> multi-CPU enabled kernel, as nothing in here will ever get called
> on an ARMv6 CPU. Adding -march=armv7 doesn't hurt and can only
> improve the code quality.

Is this enough to override a previous -mfpu for gcc?


> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/lib/Makefile | 2 +-
>  lib/raid6/Makefile    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index d8a780799506..a86c6c8e0648 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -35,7 +35,7 @@ $(obj)/csumpartialcopy.o:	$(obj)/csumpartialcopygeneric.S
>  $(obj)/csumpartialcopyuser.o:	$(obj)/csumpartialcopygeneric.S
>  
>  ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
> -  NEON_FLAGS			:= -mfloat-abi=softfp -mfpu=neon
> +  NEON_FLAGS			:= -mfloat-abi=softfp -mfpu=neon -march=armv7-a
>    CFLAGS_xor-neon.o		+= $(NEON_FLAGS)
>    obj-$(CONFIG_XOR_BLOCKS)	+= xor-neon.o
>  endif
> diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
> index 3b10a48fa040..4ef3e800fb39 100644
> --- a/lib/raid6/Makefile
> +++ b/lib/raid6/Makefile
> @@ -23,7 +23,7 @@ endif
>  ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
>  NEON_FLAGS := -ffreestanding
>  ifeq ($(ARCH),arm)
> -NEON_FLAGS += -mfloat-abi=softfp -mfpu=neon
> +NEON_FLAGS += -mfloat-abi=softfp -mfpu=neon -march=armv7-a
>  endif
>  ifeq ($(ARCH),arm64)
>  CFLAGS_REMOVE_neon1.o += -mgeneral-regs-only
> -- 
> 2.7.0
> 
> 

Re: [PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang

[Arnd Bergmann]

On Thursday 18 February 2016 12:31:35 Nicolas Pitre wrote:
> On Thu, 18 Feb 2016, Arnd Bergmann wrote:
> 
> > clang ignores the -mfpu=neon flag when building with -march=armv6:
> > 
> > In file included from lib/raid6/neon1.c:27:
> > clang/3.8.0/include/arm_neon.h:28:2: error: "NEON support not enabled"
> > 
> > There is no real need to build the file with -march=armv6 in a
> > multi-CPU enabled kernel, as nothing in here will ever get called
> > on an ARMv6 CPU. Adding -march=armv7 doesn't hurt and can only
> > improve the code quality.
> 
> Is this enough to override a previous -mfpu for gcc?

I did not see any build failures on gcc with this, so I assume it
has no effect. I could move the -march=armv7-a in front of
-mfpu=neon if you think that would be safer though.

	Arnd

Re: [PATCH 1/3] [RESEND] ARM: pass -march=armv7-a when building NEON files with clang

[Nicolas Pitre]

On Fri, 19 Feb 2016, Arnd Bergmann wrote:

> On Thursday 18 February 2016 12:31:35 Nicolas Pitre wrote:
> > On Thu, 18 Feb 2016, Arnd Bergmann wrote:
> > 
> > > clang ignores the -mfpu=neon flag when building with -march=armv6:
> > > 
> > > In file included from lib/raid6/neon1.c:27:
> > > clang/3.8.0/include/arm_neon.h:28:2: error: "NEON support not enabled"
> > > 
> > > There is no real need to build the file with -march=armv6 in a
> > > multi-CPU enabled kernel, as nothing in here will ever get called
> > > on an ARMv6 CPU. Adding -march=armv7 doesn't hurt and can only
> > > improve the code quality.
> > 
> > Is this enough to override a previous -mfpu for gcc?
> 
> I did not see any build failures on gcc with this, so I assume it
> has no effect. I could move the -march=armv7-a in front of
> -mfpu=neon if you think that would be safer though.

I don't know what's safer.  That's why I'm asking.  :-)


Nicolas

[PATCH 2/3] [RESEND] ARM: fix copypage-*.c building with clang

[Arnd Bergmann]

clang does not allow inline assembly in __naked functions to
have any register parameters and throws an error:

arch/arm/mm/copypage-v4wb.c:47:9: error: parameter references not allowed in naked functions
        : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));

Fortunately, all of these functions are trivial to convert to
using the registers directly.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mm/copypage-feroceon.c |  4 ++--
 arch/arm/mm/copypage-v4mc.c     | 26 +++++++++++++-------------
 arch/arm/mm/copypage-v4wb.c     |  4 ++--
 arch/arm/mm/copypage-v4wt.c     |  4 ++--
 arch/arm/mm/copypage-xsc3.c     |  4 ++--
 arch/arm/mm/copypage-xscale.c   |  4 ++--
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mm/copypage-feroceon.c b/arch/arm/mm/copypage-feroceon.c
index 49ee0c1a7209..e69bf2f15f32 100644
--- a/arch/arm/mm/copypage-feroceon.c
+++ b/arch/arm/mm/copypage-feroceon.c
@@ -18,7 +18,7 @@ feroceon_copy_user_page(void *kto, const void *kfrom)
 {
 	asm("\
 	stmfd	sp!, {r4-r9, lr}		\n\
-	mov	ip, %2				\n\
+	mov	ip, %0				\n\
 1:	mov	lr, r1				\n\
 	ldmia	r1!, {r2 - r9}			\n\
 	pld	[lr, #32]			\n\
@@ -64,7 +64,7 @@ feroceon_copy_user_page(void *kto, const void *kfrom)
 	mcr	p15, 0, ip, c7, c10, 4		@ drain WB\n\
 	ldmfd	sp!, {r4-r9, pc}"
 	:
-	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE));
+	: "I" (PAGE_SIZE));
 }
 
 void feroceon_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4mc.c b/arch/arm/mm/copypage-v4mc.c
index 1267e64133b9..ea89722c00db 100644
--- a/arch/arm/mm/copypage-v4mc.c
+++ b/arch/arm/mm/copypage-v4mc.c
@@ -45,23 +45,23 @@ mc_copy_user_page(void *from, void *to)
 {
 	asm volatile(
 	"stmfd	sp!, {r4, lr}			@ 2\n\
-	mov	r4, %2				@ 1\n\
-	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
-1:	mcr	p15, 0, %1, c7, c6, 1		@ 1   invalidate D line\n\
-	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
-	ldmia	%0!, {r2, r3, ip, lr}		@ 4+1\n\
-	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
-	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
-	mcr	p15, 0, %1, c7, c6, 1		@ 1   invalidate D line\n\
-	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
-	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
+	mov	r4, %0				@ 1\n\
+	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
+1:	mcr	p15, 0, r1, c7, c6, 1		@ 1   invalidate D line\n\
+	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
+	ldmia	r0!, {r2, r3, ip, lr}		@ 4+1\n\
+	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
+	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
+	mcr	p15, 0, r1, c7, c6, 1		@ 1   invalidate D line\n\
+	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
+	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
 	subs	r4, r4, #1			@ 1\n\
-	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
-	ldmneia	%0!, {r2, r3, ip, lr}		@ 4\n\
+	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
+	ldmneia	r0!, {r2, r3, ip, lr}		@ 4\n\
 	bne	1b				@ 1\n\
 	ldmfd	sp!, {r4, pc}			@ 3"
 	:
-	: "r" (from), "r" (to), "I" (PAGE_SIZE / 64));
+	: "I" (PAGE_SIZE / 64));
 }
 
 void v4_mc_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4wb.c b/arch/arm/mm/copypage-v4wb.c
index 067d0fdd630c..7ea9cf07bd5c 100644
--- a/arch/arm/mm/copypage-v4wb.c
+++ b/arch/arm/mm/copypage-v4wb.c
@@ -27,7 +27,7 @@ v4wb_copy_user_page(void *kto, const void *kfrom)
 {
 	asm("\
 	stmfd	sp!, {r4, lr}			@ 2\n\
-	mov	r2, %2				@ 1\n\
+	mov	r2, %0				@ 1\n\
 	ldmia	r1!, {r3, r4, ip, lr}		@ 4\n\
 1:	mcr	p15, 0, r0, c7, c6, 1		@ 1   invalidate D line\n\
 	stmia	r0!, {r3, r4, ip, lr}		@ 4\n\
@@ -44,7 +44,7 @@ v4wb_copy_user_page(void *kto, const void *kfrom)
 	mcr	p15, 0, r1, c7, c10, 4		@ 1   drain WB\n\
 	ldmfd	 sp!, {r4, pc}			@ 3"
 	:
-	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
+	: "I" (PAGE_SIZE / 64));
 }
 
 void v4wb_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4wt.c b/arch/arm/mm/copypage-v4wt.c
index b85c5da2e510..c742ab24efd6 100644
--- a/arch/arm/mm/copypage-v4wt.c
+++ b/arch/arm/mm/copypage-v4wt.c
@@ -25,7 +25,7 @@ v4wt_copy_user_page(void *kto, const void *kfrom)
 {
 	asm("\
 	stmfd	sp!, {r4, lr}			@ 2\n\
-	mov	r2, %2				@ 1\n\
+	mov	r2, %0				@ 1\n\
 	ldmia	r1!, {r3, r4, ip, lr}		@ 4\n\
 1:	stmia	r0!, {r3, r4, ip, lr}		@ 4\n\
 	ldmia	r1!, {r3, r4, ip, lr}		@ 4+1\n\
@@ -40,7 +40,7 @@ v4wt_copy_user_page(void *kto, const void *kfrom)
 	mcr	p15, 0, r2, c7, c7, 0		@ flush ID cache\n\
 	ldmfd	sp!, {r4, pc}			@ 3"
 	:
-	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
+	: "I" (PAGE_SIZE / 64));
 }
 
 void v4wt_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-xsc3.c b/arch/arm/mm/copypage-xsc3.c
index 03a2042aced5..17e4e11c4612 100644
--- a/arch/arm/mm/copypage-xsc3.c
+++ b/arch/arm/mm/copypage-xsc3.c
@@ -34,7 +34,7 @@ xsc3_mc_copy_user_page(void *kto, const void *kfrom)
 {
 	asm("\
 	stmfd	sp!, {r4, r5, lr}		\n\
-	mov	lr, %2				\n\
+	mov	lr, %0				\n\
 						\n\
 	pld	[r1, #0]			\n\
 	pld	[r1, #32]			\n\
@@ -67,7 +67,7 @@ xsc3_mc_copy_user_page(void *kto, const void *kfrom)
 						\n\
 	ldmfd	sp!, {r4, r5, pc}"
 	:
-	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64 - 1));
+	: "I" (PAGE_SIZE / 64 - 1));
 }
 
 void xsc3_mc_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-xscale.c b/arch/arm/mm/copypage-xscale.c
index 0fb85025344d..1034b4ce80cc 100644
--- a/arch/arm/mm/copypage-xscale.c
+++ b/arch/arm/mm/copypage-xscale.c
@@ -45,7 +45,7 @@ mc_copy_user_page(void *from, void *to)
 	 */
 	asm volatile(
 	"stmfd	sp!, {r4, r5, lr}		\n\
-	mov	lr, %2				\n\
+	mov	lr, %0				\n\
 	pld	[r0, #0]			\n\
 	pld	[r0, #32]			\n\
 	pld	[r1, #0]			\n\
@@ -81,7 +81,7 @@ mc_copy_user_page(void *from, void *to)
 	beq	2b				\n\
 	ldmfd	sp!, {r4, r5, pc}		"
 	:
-	: "r" (from), "r" (to), "I" (PAGE_SIZE / 64 - 1));
+	: "I" (PAGE_SIZE / 64 - 1));
 }
 
 void xscale_mc_copy_user_highpage(struct page *to, struct page *from,
-- 
2.7.0

Re: [PATCH 2/3] [RESEND] ARM: fix copypage-*.c building with clang

[Nicolas Pitre]

On Thu, 18 Feb 2016, Arnd Bergmann wrote:

> clang does not allow inline assembly in __naked functions to
> have any register parameters and throws an error:
> 
> arch/arm/mm/copypage-v4wb.c:47:9: error: parameter references not allowed in naked functions
>         : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
> 
> Fortunately, all of these functions are trivial to convert to
> using the registers directly.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Nicolas Pitre <nico@linaro.org>


> ---
>  arch/arm/mm/copypage-feroceon.c |  4 ++--
>  arch/arm/mm/copypage-v4mc.c     | 26 +++++++++++++-------------
>  arch/arm/mm/copypage-v4wb.c     |  4 ++--
>  arch/arm/mm/copypage-v4wt.c     |  4 ++--
>  arch/arm/mm/copypage-xsc3.c     |  4 ++--
>  arch/arm/mm/copypage-xscale.c   |  4 ++--
>  6 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/mm/copypage-feroceon.c b/arch/arm/mm/copypage-feroceon.c
> index 49ee0c1a7209..e69bf2f15f32 100644
> --- a/arch/arm/mm/copypage-feroceon.c
> +++ b/arch/arm/mm/copypage-feroceon.c
> @@ -18,7 +18,7 @@ feroceon_copy_user_page(void *kto, const void *kfrom)
>  {
>  	asm("\
>  	stmfd	sp!, {r4-r9, lr}		\n\
> -	mov	ip, %2				\n\
> +	mov	ip, %0				\n\
>  1:	mov	lr, r1				\n\
>  	ldmia	r1!, {r2 - r9}			\n\
>  	pld	[lr, #32]			\n\
> @@ -64,7 +64,7 @@ feroceon_copy_user_page(void *kto, const void *kfrom)
>  	mcr	p15, 0, ip, c7, c10, 4		@ drain WB\n\
>  	ldmfd	sp!, {r4-r9, pc}"
>  	:
> -	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE));
> +	: "I" (PAGE_SIZE));
>  }
>  
>  void feroceon_copy_user_highpage(struct page *to, struct page *from,
> diff --git a/arch/arm/mm/copypage-v4mc.c b/arch/arm/mm/copypage-v4mc.c
> index 1267e64133b9..ea89722c00db 100644
> --- a/arch/arm/mm/copypage-v4mc.c
> +++ b/arch/arm/mm/copypage-v4mc.c
> @@ -45,23 +45,23 @@ mc_copy_user_page(void *from, void *to)
>  {
>  	asm volatile(
>  	"stmfd	sp!, {r4, lr}			@ 2\n\
> -	mov	r4, %2				@ 1\n\
> -	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
> -1:	mcr	p15, 0, %1, c7, c6, 1		@ 1   invalidate D line\n\
> -	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
> -	ldmia	%0!, {r2, r3, ip, lr}		@ 4+1\n\
> -	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
> -	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
> -	mcr	p15, 0, %1, c7, c6, 1		@ 1   invalidate D line\n\
> -	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
> -	ldmia	%0!, {r2, r3, ip, lr}		@ 4\n\
> +	mov	r4, %0				@ 1\n\
> +	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
> +1:	mcr	p15, 0, r1, c7, c6, 1		@ 1   invalidate D line\n\
> +	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
> +	ldmia	r0!, {r2, r3, ip, lr}		@ 4+1\n\
> +	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
> +	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
> +	mcr	p15, 0, r1, c7, c6, 1		@ 1   invalidate D line\n\
> +	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
> +	ldmia	r0!, {r2, r3, ip, lr}		@ 4\n\
>  	subs	r4, r4, #1			@ 1\n\
> -	stmia	%1!, {r2, r3, ip, lr}		@ 4\n\
> -	ldmneia	%0!, {r2, r3, ip, lr}		@ 4\n\
> +	stmia	r1!, {r2, r3, ip, lr}		@ 4\n\
> +	ldmneia	r0!, {r2, r3, ip, lr}		@ 4\n\
>  	bne	1b				@ 1\n\
>  	ldmfd	sp!, {r4, pc}			@ 3"
>  	:
> -	: "r" (from), "r" (to), "I" (PAGE_SIZE / 64));
> +	: "I" (PAGE_SIZE / 64));
>  }
>  
>  void v4_mc_copy_user_highpage(struct page *to, struct page *from,
> diff --git a/arch/arm/mm/copypage-v4wb.c b/arch/arm/mm/copypage-v4wb.c
> index 067d0fdd630c..7ea9cf07bd5c 100644
> --- a/arch/arm/mm/copypage-v4wb.c
> +++ b/arch/arm/mm/copypage-v4wb.c
> @@ -27,7 +27,7 @@ v4wb_copy_user_page(void *kto, const void *kfrom)
>  {
>  	asm("\
>  	stmfd	sp!, {r4, lr}			@ 2\n\
> -	mov	r2, %2				@ 1\n\
> +	mov	r2, %0				@ 1\n\
>  	ldmia	r1!, {r3, r4, ip, lr}		@ 4\n\
>  1:	mcr	p15, 0, r0, c7, c6, 1		@ 1   invalidate D line\n\
>  	stmia	r0!, {r3, r4, ip, lr}		@ 4\n\
> @@ -44,7 +44,7 @@ v4wb_copy_user_page(void *kto, const void *kfrom)
>  	mcr	p15, 0, r1, c7, c10, 4		@ 1   drain WB\n\
>  	ldmfd	 sp!, {r4, pc}			@ 3"
>  	:
> -	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
> +	: "I" (PAGE_SIZE / 64));
>  }
>  
>  void v4wb_copy_user_highpage(struct page *to, struct page *from,
> diff --git a/arch/arm/mm/copypage-v4wt.c b/arch/arm/mm/copypage-v4wt.c
> index b85c5da2e510..c742ab24efd6 100644
> --- a/arch/arm/mm/copypage-v4wt.c
> +++ b/arch/arm/mm/copypage-v4wt.c
> @@ -25,7 +25,7 @@ v4wt_copy_user_page(void *kto, const void *kfrom)
>  {
>  	asm("\
>  	stmfd	sp!, {r4, lr}			@ 2\n\
> -	mov	r2, %2				@ 1\n\
> +	mov	r2, %0				@ 1\n\
>  	ldmia	r1!, {r3, r4, ip, lr}		@ 4\n\
>  1:	stmia	r0!, {r3, r4, ip, lr}		@ 4\n\
>  	ldmia	r1!, {r3, r4, ip, lr}		@ 4+1\n\
> @@ -40,7 +40,7 @@ v4wt_copy_user_page(void *kto, const void *kfrom)
>  	mcr	p15, 0, r2, c7, c7, 0		@ flush ID cache\n\
>  	ldmfd	sp!, {r4, pc}			@ 3"
>  	:
> -	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
> +	: "I" (PAGE_SIZE / 64));
>  }
>  
>  void v4wt_copy_user_highpage(struct page *to, struct page *from,
> diff --git a/arch/arm/mm/copypage-xsc3.c b/arch/arm/mm/copypage-xsc3.c
> index 03a2042aced5..17e4e11c4612 100644
> --- a/arch/arm/mm/copypage-xsc3.c
> +++ b/arch/arm/mm/copypage-xsc3.c
> @@ -34,7 +34,7 @@ xsc3_mc_copy_user_page(void *kto, const void *kfrom)
>  {
>  	asm("\
>  	stmfd	sp!, {r4, r5, lr}		\n\
> -	mov	lr, %2				\n\
> +	mov	lr, %0				\n\
>  						\n\
>  	pld	[r1, #0]			\n\
>  	pld	[r1, #32]			\n\
> @@ -67,7 +67,7 @@ xsc3_mc_copy_user_page(void *kto, const void *kfrom)
>  						\n\
>  	ldmfd	sp!, {r4, r5, pc}"
>  	:
> -	: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64 - 1));
> +	: "I" (PAGE_SIZE / 64 - 1));
>  }
>  
>  void xsc3_mc_copy_user_highpage(struct page *to, struct page *from,
> diff --git a/arch/arm/mm/copypage-xscale.c b/arch/arm/mm/copypage-xscale.c
> index 0fb85025344d..1034b4ce80cc 100644
> --- a/arch/arm/mm/copypage-xscale.c
> +++ b/arch/arm/mm/copypage-xscale.c
> @@ -45,7 +45,7 @@ mc_copy_user_page(void *from, void *to)
>  	 */
>  	asm volatile(
>  	"stmfd	sp!, {r4, r5, lr}		\n\
> -	mov	lr, %2				\n\
> +	mov	lr, %0				\n\
>  	pld	[r0, #0]			\n\
>  	pld	[r0, #32]			\n\
>  	pld	[r1, #0]			\n\
> @@ -81,7 +81,7 @@ mc_copy_user_page(void *from, void *to)
>  	beq	2b				\n\
>  	ldmfd	sp!, {r4, r5, pc}		"
>  	:
> -	: "r" (from), "r" (to), "I" (PAGE_SIZE / 64 - 1));
> +	: "I" (PAGE_SIZE / 64 - 1));
>  }
>  
>  void xscale_mc_copy_user_highpage(struct page *to, struct page *from,
> -- 
> 2.7.0
> 
> 

[PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets

[Arnd Bergmann]

build-testing with clang showed that the "J" constraint does not take
positive arguments on clang when building in for Thumb-2:

core.c:540:3: error: invalid operand for inline asm constraint 'J'

This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061

However, looking at the source code in depth, I found that the
kernel is also wrong, and it should not use "J" at all, but should
use "I" to pass an immediate argument to the inline assembly when that
is used as an offset to an 'ldr' instruction rather than the 'sub'
argument.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/probes/kprobes/core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index a4ec240ee7ba..4b34b40ca917 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
 		:
 		: "r" (kcb->jprobe_saved_regs.ARM_sp),
 		  "I" (sizeof(struct pt_regs) * 2),
-		  "J" (offsetof(struct pt_regs, ARM_sp)),
-		  "J" (offsetof(struct pt_regs, ARM_pc)),
-		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
-		  "J" (offsetof(struct pt_regs, ARM_lr))
+		  "I" (offsetof(struct pt_regs, ARM_sp)),
+		  "I" (offsetof(struct pt_regs, ARM_pc)),
+		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
+		  "I" (offsetof(struct pt_regs, ARM_lr))
 		: "memory", "cc");
 }
 
-- 
2.7.0

Re: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets

[Jon Medhurst (Tixy)]

On Thu, 2016-02-18 at 18:05 +0100, Arnd Bergmann wrote:
> build-testing with clang showed that the "J" constraint does not take
> positive arguments on clang when building in for Thumb-2:
> 
> core.c:540:3: error: invalid operand for inline asm constraint 'J'
> 
> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
> 
> However, looking at the source code in depth, I found that the
> kernel is also wrong, and it should not use "J" at all, but should
> use "I" to pass an immediate argument to the inline assembly when that
> is used as an offset to an 'ldr' instruction rather than the 'sub'
> argument.

This patch doesn't seem correct to me.

The ARM ARM says the immediate offset to an ARM ldr instructions is "any
value in the range 0-4095" and offsets may be added or subtracted,
leading to values from −4095 to 4095".

And GCC machine constraints [1] says

I
    Integer that is valid as an immediate operand in a data processing
    instruction. That is, an integer in the range 0 to 255 rotated by a
    multiple of 2
J
    Integer in the range −4095 to 4095 

So the current use of 'J' seems correct to me.

[1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints


> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/probes/kprobes/core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240ee7ba..4b34b40ca917 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
>  		:
>  		: "r" (kcb->jprobe_saved_regs.ARM_sp),
>  		  "I" (sizeof(struct pt_regs) * 2),
> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
> -		  "J" (offsetof(struct pt_regs, ARM_lr))
> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
> +		  "I" (offsetof(struct pt_regs, ARM_lr))
>  		: "memory", "cc");
>  }
>  

Re: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets

[Robin Murphy]

On 18/02/16 18:12, Jon Medhurst (Tixy) wrote:
> On Thu, 2016-02-18 at 18:05 +0100, Arnd Bergmann wrote:
>> build-testing with clang showed that the "J" constraint does not take
>> positive arguments on clang when building in for Thumb-2:
>>
>> core.c:540:3: error: invalid operand for inline asm constraint 'J'
>>
>> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
>>
>> However, looking at the source code in depth, I found that the
>> kernel is also wrong, and it should not use "J" at all, but should
>> use "I" to pass an immediate argument to the inline assembly when that
>> is used as an offset to an 'ldr' instruction rather than the 'sub'
>> argument.
>
> This patch doesn't seem correct to me.
>
> The ARM ARM says the immediate offset to an ARM ldr instructions is "any
> value in the range 0-4095" and offsets may be added or subtracted,
> leading to values from −4095 to 4095".
>
> And GCC machine constraints [1] says
>
> I
>      Integer that is valid as an immediate operand in a data processing
>      instruction. That is, an integer in the range 0 to 255 rotated by a
>      multiple of 2
> J
>      Integer in the range −4095 to 4095
>
> So the current use of 'J' seems correct to me.

Hmm, Arnd reports the failure when building for Thumb-2, and the code 
under #ifdef CONFIG_THUMB2_KERNEL contains an ldrd, which takes a 
different immediate of the form imm8 * 4. Maybe it's just operand %5 
which needs fixing, although I don't see that a suitable constraint for 
that actually exists...

Robin.

> [1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints
>
>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   arch/arm/probes/kprobes/core.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
>> index a4ec240ee7ba..4b34b40ca917 100644
>> --- a/arch/arm/probes/kprobes/core.c
>> +++ b/arch/arm/probes/kprobes/core.c
>> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
>>   		:
>>   		: "r" (kcb->jprobe_saved_regs.ARM_sp),
>>   		  "I" (sizeof(struct pt_regs) * 2),
>> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
>> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
>> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
>> -		  "J" (offsetof(struct pt_regs, ARM_lr))
>> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
>> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
>> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
>> +		  "I" (offsetof(struct pt_regs, ARM_lr))
>>   		: "memory", "cc");
>>   }
>>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets

[Jon Medhurst (Tixy)]

On Thu, 2016-02-18 at 18:59 +0000, Robin Murphy wrote:
> On 18/02/16 18:12, Jon Medhurst (Tixy) wrote:
> > On Thu, 2016-02-18 at 18:05 +0100, Arnd Bergmann wrote:
> >> build-testing with clang showed that the "J" constraint does not take
> >> positive arguments on clang when building in for Thumb-2:
> >>
> >> core.c:540:3: error: invalid operand for inline asm constraint 'J'
> >>
> >> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
> >>
> >> However, looking at the source code in depth, I found that the
> >> kernel is also wrong, and it should not use "J" at all, but should
> >> use "I" to pass an immediate argument to the inline assembly when that
> >> is used as an offset to an 'ldr' instruction rather than the 'sub'
> >> argument.
> >
> > This patch doesn't seem correct to me.
> >
> > The ARM ARM says the immediate offset to an ARM ldr instructions is "any
> > value in the range 0-4095" and offsets may be added or subtracted,
> > leading to values from −4095 to 4095".
> >
> > And GCC machine constraints [1] says
> >
> > I
> >      Integer that is valid as an immediate operand in a data processing
> >      instruction. That is, an integer in the range 0 to 255 rotated by a
> >      multiple of 2
> > J
> >      Integer in the range −4095 to 4095
> >
> > So the current use of 'J' seems correct to me.
> 
> Hmm, Arnd reports the failure when building for Thumb-2, and the code 
> under #ifdef CONFIG_THUMB2_KERNEL contains an ldrd, which takes a 
> different immediate of the form imm8 * 4. Maybe it's just operand %5 
> which needs fixing, although I don't see that a suitable constraint for 
> that actually exists...

Well, under Thumb-2 plain LDR is also different, the offset is up to
+/-255, except for pre-indexed without writeback mode (what the code
uses) which goes up to +4095. I saw this yesterday but also that there
aren't asm constraints for that.

Actually, there is constraint 'Uq' which is "A memory reference suitable
for the ARMv4 ldrsb instruction" and that is a value +/- 255.

In practice, for the code in question, which is getting offsets into
struct pt_regs, either 'I', 'J' or 'Uq' would work. 'Uq' is the one that
expresses the strictest restrictions, that if met, will work with all
assembler instructions used, but as it's documented as being for the
"ARMv4 ldrsb instruction", it would seem a bit confusing to use that to
me.

-- 
Tixy

> 
> Robin.
> 
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints
> >
> >
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>   arch/arm/probes/kprobes/core.c | 8 ++++----
> >>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> >> index a4ec240ee7ba..4b34b40ca917 100644
> >> --- a/arch/arm/probes/kprobes/core.c
> >> +++ b/arch/arm/probes/kprobes/core.c
> >> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
> >>   		:
> >>   		: "r" (kcb->jprobe_saved_regs.ARM_sp),
> >>   		  "I" (sizeof(struct pt_regs) * 2),
> >> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
> >> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
> >> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
> >> -		  "J" (offsetof(struct pt_regs, ARM_lr))
> >> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
> >> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
> >> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
> >> +		  "I" (offsetof(struct pt_regs, ARM_lr))
> >>   		: "memory", "cc");
> >>   }
> >>
> >
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> 

Re: [PATCH 3/3] [RESEND] ARM: kprobes: use "I" constraint for inline assembly offsets

[Nicolas Pitre]

On Thu, 18 Feb 2016, Arnd Bergmann wrote:

> build-testing with clang showed that the "J" constraint does not take
> positive arguments on clang when building in for Thumb-2:
> 
> core.c:540:3: error: invalid operand for inline asm constraint 'J'
> 
> This has been reported as llvm bug https://llvm.org/bugs/show_bug.cgi?id=26061
> 
> However, looking at the source code in depth, I found that the
> kernel is also wrong, and it should not use "J" at all, but should
> use "I" to pass an immediate argument to the inline assembly when that
> is used as an offset to an 'ldr' instruction rather than the 'sub'
> argument.

I don't follow you.

>From the gcc manual:

     'I'
          Integer that is valid as an immediate operand in a data
          processing instruction.  That is, an integer in the range 0 to
          255 rotated by a multiple of 2

     'J'
          Integer in the range -4095 to 4095

>From the ARM ARM:

   LDR<c> <Rt>, [<Rn>{, #+/-<imm12>}]



where imm12 is a constant between 0 and 4095.

So J is really the appropriate constraint here.

Sure, in this case it is very likely that I would just works given that 
offset_of() is unlikely to exceed shifted 8 bits and that's what people 
use in most cases. But strictly speaking it's J that perfectly matches 
the LDR/STR instructions.








> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/probes/kprobes/core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240ee7ba..4b34b40ca917 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -570,10 +570,10 @@ void __kprobes jprobe_return(void)
>  		:
>  		: "r" (kcb->jprobe_saved_regs.ARM_sp),
>  		  "I" (sizeof(struct pt_regs) * 2),
> -		  "J" (offsetof(struct pt_regs, ARM_sp)),
> -		  "J" (offsetof(struct pt_regs, ARM_pc)),
> -		  "J" (offsetof(struct pt_regs, ARM_cpsr)),
> -		  "J" (offsetof(struct pt_regs, ARM_lr))
> +		  "I" (offsetof(struct pt_regs, ARM_sp)),
> +		  "I" (offsetof(struct pt_regs, ARM_pc)),
> +		  "I" (offsetof(struct pt_regs, ARM_cpsr)),
> +		  "I" (offsetof(struct pt_regs, ARM_lr))
>  		: "memory", "cc");
>  }
>  
> -- 
> 2.7.0
> 
>