All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-01 21:26 ` Matthias Kaehlcke
  0 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-01 21:26 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
	Christoffer Dall, Marc Zyngier, Vladimir Murzin
  Cc: linux-arm-kernel, linux-kernel, Grant Grundler, Greg Hackmann,
	Michael Davidson, Matthias Kaehlcke

clang raises 'asm-operand-widths' warnings in inline assembly code when
the size of an operand is < 64 bits and the operand width is unspecified.
Most warnings are raised in macros, i.e. the datatype of the operand may
vary. Forcing the use of an x register through the 'x' operand modifier
would silence the warning however it involves the risk that for operands
< 64 bits 'unused' bits may be assigned to 64-bit values (more details at
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
Instead we cast the operand to 64 bits, which also forces the use of a
x register, but without the unexpected behavior.

In gic_write_bpr1() use write_sysreg_s() to write the register. This
aligns the functions with others in this header and fixes an
'asm-operand-widths' warning.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/arm64/include/asm/arch_gicv3.h  | 2 +-
 arch/arm64/include/asm/barrier.h     | 2 +-
 arch/arm64/include/asm/uaccess.h     | 2 +-
 arch/arm64/kernel/armv8_deprecated.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index f37e3a21f6e7..9092d612d8c2 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
 
 static inline void gic_write_bpr1(u32 val)
 {
-	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
+	write_sysreg_s(val, ICC_BPR1_EL1);
 }
 
 #define gic_read_typer(c)		readq_relaxed(c)
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 4e0497f581a0..1248401b07ab 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -60,7 +60,7 @@ do {									\
 		break;							\
 	case 8:								\
 		asm volatile ("stlr %1, %0"				\
-				: "=Q" (*p) : "r" (v) : "memory");	\
+			      : "=Q" (*p) : "r" ((__u64)v) : "memory");	\
 		break;							\
 	}								\
 } while (0)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 5308d696311b..7db143689694 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -302,7 +302,7 @@ do {									\
 	"	.previous\n"						\
 	_ASM_EXTABLE(1b, 3b)						\
 	: "+r" (err)							\
-	: "r" (x), "r" (addr), "i" (-EFAULT))
+	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
 
 #define __put_user_err(x, ptr, err)					\
 do {									\
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 657977e77ec8..79b9fef48b14 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -306,7 +306,7 @@ do {								\
 	_ASM_EXTABLE(0b, 4b)					\
 	_ASM_EXTABLE(1b, 4b)					\
 	: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2)	\
-	: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT),		\
+	: "r" ((__u64)addr), "i" (-EAGAIN), "i" (-EFAULT),	\
 	  "i" (__SWP_LL_SC_LOOPS)				\
 	: "memory");						\
 	uaccess_disable();					\
-- 
2.13.0.rc0.306.g87b477812d-goog

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-01 21:26 ` Matthias Kaehlcke
  0 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-01 21:26 UTC (permalink / raw)
  To: linux-arm-kernel

clang raises 'asm-operand-widths' warnings in inline assembly code when
the size of an operand is < 64 bits and the operand width is unspecified.
Most warnings are raised in macros, i.e. the datatype of the operand may
vary. Forcing the use of an x register through the 'x' operand modifier
would silence the warning however it involves the risk that for operands
< 64 bits 'unused' bits may be assigned to 64-bit values (more details at
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
Instead we cast the operand to 64 bits, which also forces the use of a
x register, but without the unexpected behavior.

In gic_write_bpr1() use write_sysreg_s() to write the register. This
aligns the functions with others in this header and fixes an
'asm-operand-widths' warning.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/arm64/include/asm/arch_gicv3.h  | 2 +-
 arch/arm64/include/asm/barrier.h     | 2 +-
 arch/arm64/include/asm/uaccess.h     | 2 +-
 arch/arm64/kernel/armv8_deprecated.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index f37e3a21f6e7..9092d612d8c2 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
 
 static inline void gic_write_bpr1(u32 val)
 {
-	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
+	write_sysreg_s(val, ICC_BPR1_EL1);
 }
 
 #define gic_read_typer(c)		readq_relaxed(c)
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 4e0497f581a0..1248401b07ab 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -60,7 +60,7 @@ do {									\
 		break;							\
 	case 8:								\
 		asm volatile ("stlr %1, %0"				\
-				: "=Q" (*p) : "r" (v) : "memory");	\
+			      : "=Q" (*p) : "r" ((__u64)v) : "memory");	\
 		break;							\
 	}								\
 } while (0)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 5308d696311b..7db143689694 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -302,7 +302,7 @@ do {									\
 	"	.previous\n"						\
 	_ASM_EXTABLE(1b, 3b)						\
 	: "+r" (err)							\
-	: "r" (x), "r" (addr), "i" (-EFAULT))
+	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
 
 #define __put_user_err(x, ptr, err)					\
 do {									\
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 657977e77ec8..79b9fef48b14 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -306,7 +306,7 @@ do {								\
 	_ASM_EXTABLE(0b, 4b)					\
 	_ASM_EXTABLE(1b, 4b)					\
 	: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2)	\
-	: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT),		\
+	: "r" ((__u64)addr), "i" (-EAGAIN), "i" (-EFAULT),	\
 	  "i" (__SWP_LL_SC_LOOPS)				\
 	: "memory");						\
 	uaccess_disable();					\
-- 
2.13.0.rc0.306.g87b477812d-goog

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-01 21:26 ` Matthias Kaehlcke
@ 2017-05-02  8:25   ` Marc Zyngier
  -1 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2017-05-02  8:25 UTC (permalink / raw)
  To: Matthias Kaehlcke, Catalin Marinas, Will Deacon, Mark Rutland,
	Ard Biesheuvel, Christoffer Dall, Vladimir Murzin
  Cc: linux-arm-kernel, linux-kernel, Grant Grundler, Greg Hackmann,
	Michael Davidson

On 01/05/17 22:26, Matthias Kaehlcke wrote:
> clang raises 'asm-operand-widths' warnings in inline assembly code when
> the size of an operand is < 64 bits and the operand width is unspecified.
> Most warnings are raised in macros, i.e. the datatype of the operand may
> vary. Forcing the use of an x register through the 'x' operand modifier
> would silence the warning however it involves the risk that for operands
> < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> Instead we cast the operand to 64 bits, which also forces the use of a
> x register, but without the unexpected behavior.
> 
> In gic_write_bpr1() use write_sysreg_s() to write the register. This
> aligns the functions with others in this header and fixes an
> 'asm-operand-widths' warning.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
>  arch/arm64/include/asm/barrier.h     | 2 +-
>  arch/arm64/include/asm/uaccess.h     | 2 +-
>  arch/arm64/kernel/armv8_deprecated.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index f37e3a21f6e7..9092d612d8c2 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
>  
>  static inline void gic_write_bpr1(u32 val)
>  {
> -	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
> +	write_sysreg_s(val, ICC_BPR1_EL1);
>  }

For the GICv3 part:

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-02  8:25   ` Marc Zyngier
  0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2017-05-02  8:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/05/17 22:26, Matthias Kaehlcke wrote:
> clang raises 'asm-operand-widths' warnings in inline assembly code when
> the size of an operand is < 64 bits and the operand width is unspecified.
> Most warnings are raised in macros, i.e. the datatype of the operand may
> vary. Forcing the use of an x register through the 'x' operand modifier
> would silence the warning however it involves the risk that for operands
> < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> Instead we cast the operand to 64 bits, which also forces the use of a
> x register, but without the unexpected behavior.
> 
> In gic_write_bpr1() use write_sysreg_s() to write the register. This
> aligns the functions with others in this header and fixes an
> 'asm-operand-widths' warning.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
>  arch/arm64/include/asm/barrier.h     | 2 +-
>  arch/arm64/include/asm/uaccess.h     | 2 +-
>  arch/arm64/kernel/armv8_deprecated.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index f37e3a21f6e7..9092d612d8c2 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
>  
>  static inline void gic_write_bpr1(u32 val)
>  {
> -	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
> +	write_sysreg_s(val, ICC_BPR1_EL1);
>  }

For the GICv3 part:

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-01 21:26 ` Matthias Kaehlcke
@ 2017-05-02 10:27   ` Mark Rutland
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-02 10:27 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Christoffer Dall,
	Marc Zyngier, Vladimir Murzin, linux-arm-kernel, linux-kernel,
	Grant Grundler, Greg Hackmann, Michael Davidson

Hi Matthias,

On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> clang raises 'asm-operand-widths' warnings in inline assembly code when
> the size of an operand is < 64 bits and the operand width is unspecified.
> Most warnings are raised in macros, i.e. the datatype of the operand may
> vary. Forcing the use of an x register through the 'x' operand modifier
> would silence the warning however it involves the risk that for operands
> < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> Instead we cast the operand to 64 bits, which also forces the use of a
> x register, but without the unexpected behavior.
> 
> In gic_write_bpr1() use write_sysreg_s() to write the register. This
> aligns the functions with others in this header and fixes an
> 'asm-operand-widths' warning.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
>  arch/arm64/include/asm/barrier.h     | 2 +-
>  arch/arm64/include/asm/uaccess.h     | 2 +-
>  arch/arm64/kernel/armv8_deprecated.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

Thanks for putting this together.

Just to check, are these the only instances that you see clang warning
about?

There are a number of other cases where we can see similar problems
(e.g. passing a u8 value to an smp_store_release() on a u32 variable),
so we need to fix more than the clang warnings.

I'm currently attempting a systematic audit of our inline asm to correct
all instances, looking at:

	git grep -e asm \
	--and --not -e 'include' \
	--and --not -e 'asmlinkage' \
	-- arch/arm64

I hope to have patches shortly, and will keep you informed.

Thanks,
Mark.

> 
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index f37e3a21f6e7..9092d612d8c2 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
>  
>  static inline void gic_write_bpr1(u32 val)
>  {
> -	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
> +	write_sysreg_s(val, ICC_BPR1_EL1);
>  }
>  
>  #define gic_read_typer(c)		readq_relaxed(c)
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 4e0497f581a0..1248401b07ab 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -60,7 +60,7 @@ do {									\
>  		break;							\
>  	case 8:								\
>  		asm volatile ("stlr %1, %0"				\
> -				: "=Q" (*p) : "r" (v) : "memory");	\
> +			      : "=Q" (*p) : "r" ((__u64)v) : "memory");	\
>  		break;							\
>  	}								\
>  } while (0)
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 5308d696311b..7db143689694 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -302,7 +302,7 @@ do {									\
>  	"	.previous\n"						\
>  	_ASM_EXTABLE(1b, 3b)						\
>  	: "+r" (err)							\
> -	: "r" (x), "r" (addr), "i" (-EFAULT))
> +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
>  
>  #define __put_user_err(x, ptr, err)					\
>  do {									\
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 657977e77ec8..79b9fef48b14 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -306,7 +306,7 @@ do {								\
>  	_ASM_EXTABLE(0b, 4b)					\
>  	_ASM_EXTABLE(1b, 4b)					\
>  	: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2)	\
> -	: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT),		\
> +	: "r" ((__u64)addr), "i" (-EAGAIN), "i" (-EFAULT),	\
>  	  "i" (__SWP_LL_SC_LOOPS)				\
>  	: "memory");						\
>  	uaccess_disable();					\
> -- 
> 2.13.0.rc0.306.g87b477812d-goog
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-02 10:27   ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-02 10:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Matthias,

On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> clang raises 'asm-operand-widths' warnings in inline assembly code when
> the size of an operand is < 64 bits and the operand width is unspecified.
> Most warnings are raised in macros, i.e. the datatype of the operand may
> vary. Forcing the use of an x register through the 'x' operand modifier
> would silence the warning however it involves the risk that for operands
> < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> Instead we cast the operand to 64 bits, which also forces the use of a
> x register, but without the unexpected behavior.
> 
> In gic_write_bpr1() use write_sysreg_s() to write the register. This
> aligns the functions with others in this header and fixes an
> 'asm-operand-widths' warning.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
>  arch/arm64/include/asm/barrier.h     | 2 +-
>  arch/arm64/include/asm/uaccess.h     | 2 +-
>  arch/arm64/kernel/armv8_deprecated.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

Thanks for putting this together.

Just to check, are these the only instances that you see clang warning
about?

There are a number of other cases where we can see similar problems
(e.g. passing a u8 value to an smp_store_release() on a u32 variable),
so we need to fix more than the clang warnings.

I'm currently attempting a systematic audit of our inline asm to correct
all instances, looking at:

	git grep -e asm \
	--and --not -e 'include' \
	--and --not -e 'asmlinkage' \
	-- arch/arm64

I hope to have patches shortly, and will keep you informed.

Thanks,
Mark.

> 
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index f37e3a21f6e7..9092d612d8c2 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
>  
>  static inline void gic_write_bpr1(u32 val)
>  {
> -	asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
> +	write_sysreg_s(val, ICC_BPR1_EL1);
>  }
>  
>  #define gic_read_typer(c)		readq_relaxed(c)
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 4e0497f581a0..1248401b07ab 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -60,7 +60,7 @@ do {									\
>  		break;							\
>  	case 8:								\
>  		asm volatile ("stlr %1, %0"				\
> -				: "=Q" (*p) : "r" (v) : "memory");	\
> +			      : "=Q" (*p) : "r" ((__u64)v) : "memory");	\
>  		break;							\
>  	}								\
>  } while (0)
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 5308d696311b..7db143689694 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -302,7 +302,7 @@ do {									\
>  	"	.previous\n"						\
>  	_ASM_EXTABLE(1b, 3b)						\
>  	: "+r" (err)							\
> -	: "r" (x), "r" (addr), "i" (-EFAULT))
> +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
>  
>  #define __put_user_err(x, ptr, err)					\
>  do {									\
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 657977e77ec8..79b9fef48b14 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -306,7 +306,7 @@ do {								\
>  	_ASM_EXTABLE(0b, 4b)					\
>  	_ASM_EXTABLE(1b, 4b)					\
>  	: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2)	\
> -	: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT),		\
> +	: "r" ((__u64)addr), "i" (-EAGAIN), "i" (-EFAULT),	\
>  	  "i" (__SWP_LL_SC_LOOPS)				\
>  	: "memory");						\
>  	uaccess_disable();					\
> -- 
> 2.13.0.rc0.306.g87b477812d-goog
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-02 10:27   ` Mark Rutland
@ 2017-05-02 17:26     ` Matthias Kaehlcke
  -1 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-02 17:26 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Christoffer Dall,
	Marc Zyngier, Vladimir Murzin, linux-arm-kernel, linux-kernel,
	Grant Grundler, Greg Hackmann, Michael Davidson

Hi Mark,

El Tue, May 02, 2017 at 11:27:18AM +0100 Mark Rutland ha dit:

> On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > clang raises 'asm-operand-widths' warnings in inline assembly code when
> > the size of an operand is < 64 bits and the operand width is unspecified.
> > Most warnings are raised in macros, i.e. the datatype of the operand may
> > vary. Forcing the use of an x register through the 'x' operand modifier
> > would silence the warning however it involves the risk that for operands
> > < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> > Instead we cast the operand to 64 bits, which also forces the use of a
> > x register, but without the unexpected behavior.
> > 
> > In gic_write_bpr1() use write_sysreg_s() to write the register. This
> > aligns the functions with others in this header and fixes an
> > 'asm-operand-widths' warning.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
> >  arch/arm64/include/asm/barrier.h     | 2 +-
> >  arch/arm64/include/asm/uaccess.h     | 2 +-
> >  arch/arm64/kernel/armv8_deprecated.c | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> Thanks for putting this together.
> 
> Just to check, are these the only instances that you see clang warning
> about?

Yes, these are the only warnings clang raises in my builds (custom and
defconfig).

> There are a number of other cases where we can see similar problems
> (e.g. passing a u8 value to an smp_store_release() on a u32 variable),
> so we need to fix more than the clang warnings.
> 
> I'm currently attempting a systematic audit of our inline asm to correct
> all instances, looking at:
> 
> 	git grep -e asm \
> 	--and --not -e 'include' \
> 	--and --not -e 'asmlinkage' \
> 	-- arch/arm64
> 
> I hope to have patches shortly, and will keep you informed.

Sounds good!

Thanks

Matthias

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-02 17:26     ` Matthias Kaehlcke
  0 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-02 17:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mark,

El Tue, May 02, 2017 at 11:27:18AM +0100 Mark Rutland ha dit:

> On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > clang raises 'asm-operand-widths' warnings in inline assembly code when
> > the size of an operand is < 64 bits and the operand width is unspecified.
> > Most warnings are raised in macros, i.e. the datatype of the operand may
> > vary. Forcing the use of an x register through the 'x' operand modifier
> > would silence the warning however it involves the risk that for operands
> > < 64 bits 'unused' bits may be assigned to 64-bit values (more details at
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503816.html).
> > Instead we cast the operand to 64 bits, which also forces the use of a
> > x register, but without the unexpected behavior.
> > 
> > In gic_write_bpr1() use write_sysreg_s() to write the register. This
> > aligns the functions with others in this header and fixes an
> > 'asm-operand-widths' warning.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  arch/arm64/include/asm/arch_gicv3.h  | 2 +-
> >  arch/arm64/include/asm/barrier.h     | 2 +-
> >  arch/arm64/include/asm/uaccess.h     | 2 +-
> >  arch/arm64/kernel/armv8_deprecated.c | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> Thanks for putting this together.
> 
> Just to check, are these the only instances that you see clang warning
> about?

Yes, these are the only warnings clang raises in my builds (custom and
defconfig).

> There are a number of other cases where we can see similar problems
> (e.g. passing a u8 value to an smp_store_release() on a u32 variable),
> so we need to fix more than the clang warnings.
> 
> I'm currently attempting a systematic audit of our inline asm to correct
> all instances, looking at:
> 
> 	git grep -e asm \
> 	--and --not -e 'include' \
> 	--and --not -e 'asmlinkage' \
> 	-- arch/arm64
> 
> I hope to have patches shortly, and will keep you informed.

Sounds good!

Thanks

Matthias

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-01 21:26 ` Matthias Kaehlcke
@ 2017-05-02 17:29   ` Mark Rutland
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-02 17:29 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Christoffer Dall,
	Marc Zyngier, Vladimir Murzin, linux-arm-kernel, linux-kernel,
	Grant Grundler, Greg Hackmann, Michael Davidson

Hi,

On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 5308d696311b..7db143689694 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -302,7 +302,7 @@ do {									\
>  	"	.previous\n"						\
>  	_ASM_EXTABLE(1b, 3b)						\
>  	: "+r" (err)							\
> -	: "r" (x), "r" (addr), "i" (-EFAULT))
> +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
>  

For reference, do you have the warning for this case to hand?

In __put_user_err() we make __pu_val the same type as *ptr, then we
switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
For cases 1, 2, and 4, we use "%w" as the register template.

So I can't see why we'd  need this cast in __put_user_err().

I must be missing something.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-02 17:29   ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-02 17:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 5308d696311b..7db143689694 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -302,7 +302,7 @@ do {									\
>  	"	.previous\n"						\
>  	_ASM_EXTABLE(1b, 3b)						\
>  	: "+r" (err)							\
> -	: "r" (x), "r" (addr), "i" (-EFAULT))
> +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
>  

For reference, do you have the warning for this case to hand?

In __put_user_err() we make __pu_val the same type as *ptr, then we
switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
For cases 1, 2, and 4, we use "%w" as the register template.

So I can't see why we'd  need this cast in __put_user_err().

I must be missing something.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-02 17:29   ` Mark Rutland
@ 2017-05-02 18:52     ` Matthias Kaehlcke
  -1 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-02 18:52 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Christoffer Dall,
	Marc Zyngier, Vladimir Murzin, linux-arm-kernel, linux-kernel,
	Grant Grundler, Greg Hackmann, Michael Davidson

Hi,

El Tue, May 02, 2017 at 06:29:48PM +0100 Mark Rutland ha dit:

> On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > index 5308d696311b..7db143689694 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -302,7 +302,7 @@ do {									\
> >  	"	.previous\n"						\
> >  	_ASM_EXTABLE(1b, 3b)						\
> >  	: "+r" (err)							\
> > -	: "r" (x), "r" (addr), "i" (-EFAULT))
> > +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
> >  
> 
> For reference, do you have the warning for this case to hand?
> 
> In __put_user_err() we make __pu_val the same type as *ptr, then we
> switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
> For cases 1, 2, and 4, we use "%w" as the register template.
> 
> So I can't see why we'd  need this cast in __put_user_err().
> 
> I must be missing something.

This is one of many instances:

./include/linux/pagemap.h:554:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                return __put_user(0, end);
                       ^
./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
        __put_user_err((x), (ptr), __pu_err);                           \
        ^
./arch/arm64/include/asm/uaccess.h:326:38: note: expanded from macro '__put_user_err'
                __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
                                                   ^
./include/linux/pagemap.h:554:10: note: use constraint modifier "w"
./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
        __put_user_err((x), (ptr), __pu_err);                           \
        ^
./arch/arm64/include/asm/uaccess.h:326:34: note: expanded from macro '__put_user_err'
                __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
                                               ^

'end' is a char pointer, it is not clear to me why we would end up in
the width == 8 branch.

Matthias

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-02 18:52     ` Matthias Kaehlcke
  0 siblings, 0 replies; 14+ messages in thread
From: Matthias Kaehlcke @ 2017-05-02 18:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

El Tue, May 02, 2017 at 06:29:48PM +0100 Mark Rutland ha dit:

> On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > index 5308d696311b..7db143689694 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -302,7 +302,7 @@ do {									\
> >  	"	.previous\n"						\
> >  	_ASM_EXTABLE(1b, 3b)						\
> >  	: "+r" (err)							\
> > -	: "r" (x), "r" (addr), "i" (-EFAULT))
> > +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
> >  
> 
> For reference, do you have the warning for this case to hand?
> 
> In __put_user_err() we make __pu_val the same type as *ptr, then we
> switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
> For cases 1, 2, and 4, we use "%w" as the register template.
> 
> So I can't see why we'd  need this cast in __put_user_err().
> 
> I must be missing something.

This is one of many instances:

./include/linux/pagemap.h:554:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                return __put_user(0, end);
                       ^
./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
        __put_user_err((x), (ptr), __pu_err);                           \
        ^
./arch/arm64/include/asm/uaccess.h:326:38: note: expanded from macro '__put_user_err'
                __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
                                                   ^
./include/linux/pagemap.h:554:10: note: use constraint modifier "w"
./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
        __put_user_err((x), (ptr), __pu_err);                           \
        ^
./arch/arm64/include/asm/uaccess.h:326:34: note: expanded from macro '__put_user_err'
                __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
                                               ^

'end' is a char pointer, it is not clear to me why we would end up in
the width == 8 branch.

Matthias

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
  2017-05-02 18:52     ` Matthias Kaehlcke
@ 2017-05-03 10:51       ` Mark Rutland
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-03 10:51 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Christoffer Dall,
	Marc Zyngier, Vladimir Murzin, linux-arm-kernel, linux-kernel,
	Grant Grundler, Greg Hackmann, Michael Davidson

On Tue, May 02, 2017 at 11:52:12AM -0700, Matthias Kaehlcke wrote:
> El Tue, May 02, 2017 at 06:29:48PM +0100 Mark Rutland ha dit:
> > On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > > index 5308d696311b..7db143689694 100644
> > > --- a/arch/arm64/include/asm/uaccess.h
> > > +++ b/arch/arm64/include/asm/uaccess.h
> > > @@ -302,7 +302,7 @@ do {									\
> > >  	"	.previous\n"						\
> > >  	_ASM_EXTABLE(1b, 3b)						\
> > >  	: "+r" (err)							\
> > > -	: "r" (x), "r" (addr), "i" (-EFAULT))
> > > +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
> > >  
> > 
> > For reference, do you have the warning for this case to hand?
> > 
> > In __put_user_err() we make __pu_val the same type as *ptr, then we
> > switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
> > For cases 1, 2, and 4, we use "%w" as the register template.
> > 
> > So I can't see why we'd  need this cast in __put_user_err().
> > 
> > I must be missing something.
> 
> This is one of many instances:
> 
> ./include/linux/pagemap.h:554:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                 return __put_user(0, end);
>                        ^
> ./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
>         __put_user_err((x), (ptr), __pu_err);                           \
>         ^
> ./arch/arm64/include/asm/uaccess.h:326:38: note: expanded from macro '__put_user_err'
>                 __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
>                                                    ^
> ./include/linux/pagemap.h:554:10: note: use constraint modifier "w"
> ./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
>         __put_user_err((x), (ptr), __pu_err);                           \
>         ^
> ./arch/arm64/include/asm/uaccess.h:326:34: note: expanded from macro '__put_user_err'
>                 __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
>                                                ^

Thanks for the log above!

> 'end' is a char pointer, it is not clear to me why we would end up in
> the width == 8 branch.

Indeed.

I took a look, and I think the issue is that clang instantiates the
assembly in all cases, producing the warning, *then* optimizes away the
unreachable cases.

If you ask clang to build the following:

----
#define __put_user(val, ptr)						\
do {									\
	__typeof__(*(ptr)) __pu_val = (val);				\
	switch (sizeof(*(ptr))) { 					\
	case 1:								\
		asm volatile ("strb %w1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 2:								\
		asm volatile ("strh %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 4:								\
		asm volatile ("str %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 8:								\
		asm volatile ("str %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	}								\
} while (0)

void put_char(char in, char *ptr)
{
	__put_user(in, ptr);
}
----

It complains for all of the unmatched cases:

----
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:11:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:10:23: note: expanded from macro '__put_user'
                asm volatile ("strh %1, %0"                             \
                                    ^
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:15:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:14:22: note: expanded from macro '__put_user'
                asm volatile ("str %1, %0"                              \
                                   ^
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:19:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:18:22: note: expanded from macro '__put_user'
                asm volatile ("str %1, %0"                              \
                                   ^
3 warnings generated.
----

AFAICT, in all other cases where we switch(sizeof(...)), we (will) use
an explicit cast on the parameter, which placates clang.

I think the best option is to get rid of __pu_val, and have an explicit
cast of x in each case of the switch statement. I'll add that to my asm
fixups series, with your Reported-by.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings
@ 2017-05-03 10:51       ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2017-05-03 10:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 02, 2017 at 11:52:12AM -0700, Matthias Kaehlcke wrote:
> El Tue, May 02, 2017 at 06:29:48PM +0100 Mark Rutland ha dit:
> > On Mon, May 01, 2017 at 02:26:22PM -0700, Matthias Kaehlcke wrote:
> > > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > > index 5308d696311b..7db143689694 100644
> > > --- a/arch/arm64/include/asm/uaccess.h
> > > +++ b/arch/arm64/include/asm/uaccess.h
> > > @@ -302,7 +302,7 @@ do {									\
> > >  	"	.previous\n"						\
> > >  	_ASM_EXTABLE(1b, 3b)						\
> > >  	: "+r" (err)							\
> > > -	: "r" (x), "r" (addr), "i" (-EFAULT))
> > > +	: "r" ((__u64)x), "r" (addr), "i" (-EFAULT))
> > >  
> > 
> > For reference, do you have the warning for this case to hand?
> > 
> > In __put_user_err() we make __pu_val the same type as *ptr, then we
> > switch on sizeof(*ptr), and pass __pu_val to __put_user_asm(), as x.
> > For cases 1, 2, and 4, we use "%w" as the register template.
> > 
> > So I can't see why we'd  need this cast in __put_user_err().
> > 
> > I must be missing something.
> 
> This is one of many instances:
> 
> ./include/linux/pagemap.h:554:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                 return __put_user(0, end);
>                        ^
> ./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
>         __put_user_err((x), (ptr), __pu_err);                           \
>         ^
> ./arch/arm64/include/asm/uaccess.h:326:38: note: expanded from macro '__put_user_err'
>                 __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
>                                                    ^
> ./include/linux/pagemap.h:554:10: note: use constraint modifier "w"
> ./arch/arm64/include/asm/uaccess.h:338:2: note: expanded from macro '__put_user'
>         __put_user_err((x), (ptr), __pu_err);                           \
>         ^
> ./arch/arm64/include/asm/uaccess.h:326:34: note: expanded from macro '__put_user_err'
>                 __put_user_asm("str", "sttr", "%", __pu_val, (ptr),     \
>                                                ^

Thanks for the log above!

> 'end' is a char pointer, it is not clear to me why we would end up in
> the width == 8 branch.

Indeed.

I took a look, and I think the issue is that clang instantiates the
assembly in all cases, producing the warning, *then* optimizes away the
unreachable cases.

If you ask clang to build the following:

----
#define __put_user(val, ptr)						\
do {									\
	__typeof__(*(ptr)) __pu_val = (val);				\
	switch (sizeof(*(ptr))) { 					\
	case 1:								\
		asm volatile ("strb %w1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 2:								\
		asm volatile ("strh %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 4:								\
		asm volatile ("str %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	case 8:								\
		asm volatile ("str %1, %0"				\
				: "+Q" (*(ptr)) : "r" (__pu_val));	\
		break;							\
	}								\
} while (0)

void put_char(char in, char *ptr)
{
	__put_user(in, ptr);
}
----

It complains for all of the unmatched cases:

----
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:11:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:10:23: note: expanded from macro '__put_user'
                asm volatile ("strh %1, %0"                             \
                                    ^
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:15:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:14:22: note: expanded from macro '__put_user'
                asm volatile ("str %1, %0"                              \
                                   ^
size-switch.c:26:2: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        __put_user(in, ptr);
        ^
size-switch.c:19:28: note: expanded from macro '__put_user'
                                : "+Q" (*(ptr)) : "r" (__pu_val));      \
                                                       ^
size-switch.c:26:2: note: use constraint modifier "w"
size-switch.c:18:22: note: expanded from macro '__put_user'
                asm volatile ("str %1, %0"                              \
                                   ^
3 warnings generated.
----

AFAICT, in all other cases where we switch(sizeof(...)), we (will) use
an explicit cast on the parameter, which placates clang.

I think the best option is to get rid of __pu_val, and have an explicit
cast of x in each case of the switch statement. I'll add that to my asm
fixups series, with your Reported-by.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-05-03 10:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-01 21:26 [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings Matthias Kaehlcke
2017-05-01 21:26 ` Matthias Kaehlcke
2017-05-02  8:25 ` Marc Zyngier
2017-05-02  8:25   ` Marc Zyngier
2017-05-02 10:27 ` Mark Rutland
2017-05-02 10:27   ` Mark Rutland
2017-05-02 17:26   ` Matthias Kaehlcke
2017-05-02 17:26     ` Matthias Kaehlcke
2017-05-02 17:29 ` Mark Rutland
2017-05-02 17:29   ` Mark Rutland
2017-05-02 18:52   ` Matthias Kaehlcke
2017-05-02 18:52     ` Matthias Kaehlcke
2017-05-03 10:51     ` Mark Rutland
2017-05-03 10:51       ` Mark Rutland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.