From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751346AbdEBK1z (ORCPT ); Tue, 2 May 2017 06:27:55 -0400 Received: from foss.arm.com ([217.140.101.70]:42798 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751015AbdEBK1y (ORCPT ); Tue, 2 May 2017 06:27:54 -0400 Date: Tue, 2 May 2017 11:27:18 +0100 From: Mark Rutland To: Matthias Kaehlcke Cc: Catalin Marinas , Will Deacon , Ard Biesheuvel , Christoffer Dall , Marc Zyngier , Vladimir Murzin , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Grant Grundler , Greg Hackmann , Michael Davidson Subject: Re: [PATCH] arm64: Fix multiple 'asm-operand-widths' warnings Message-ID: <20170502102718.GA28132@leverpostej> References: <20170501212622.153720-1-mka@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170501212622.153720-1-mka@chromium.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 >