From patchwork Mon Nov 11 16:18:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Martin X-Patchwork-Id: 3168301 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1A4769F39E for ; Mon, 11 Nov 2013 16:19:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 47C60201B7 for ; Mon, 11 Nov 2013 16:19:16 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A060320225 for ; Mon, 11 Nov 2013 16:19:14 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VfuCf-0005VV-Hs; Mon, 11 Nov 2013 16:19:05 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VfuCd-0004wt-4E; Mon, 11 Nov 2013 16:19:03 +0000 Received: from fw-tnat.cambridge.arm.com ([217.140.96.21] helo=cam-smtp0.cambridge.arm.com) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VfuCZ-0004vK-3Q for linux-arm-kernel@lists.infradead.org; Mon, 11 Nov 2013 16:19:00 +0000 Received: from localhost.localdomain (e103592.cambridge.arm.com [10.1.203.45]) by cam-smtp0.cambridge.arm.com (8.13.8/8.13.8) with ESMTP id rABGIUGN023011; Mon, 11 Nov 2013 16:18:30 GMT Date: Mon, 11 Nov 2013 16:18:30 +0000 From: Dave Martin To: Ben Dooks Subject: Re: [PATCH 9/9] ARM: asm/opcodes.h: use ARM_HAVE_INST to use .inst to build instructions Message-ID: <20131111161829.GC3166@localhost.localdomain> References: <1383935832-20865-1-git-send-email-ben.dooks@codethink.co.uk> <1383935832-20865-10-git-send-email-ben.dooks@codethink.co.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1383935832-20865-10-git-send-email-ben.dooks@codethink.co.uk> Thread-Topic: [PATCH 9/9] ARM: asm/opcodes.h: use ARM_HAVE_INST to use .inst to build instructions Accept-Language: en-GB, en-US Content-Language: en-US User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131111_111859_402854_101FD496 X-CRM114-Status: GOOD ( 21.29 ) X-Spam-Score: -2.6 (--) Cc: "taras.kondratiuk@linaro.org" , "linux-arm-kernel@lists.infradead.org" , "victor.kamensky@linaro.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Nov 08, 2013 at 06:37:12PM +0000, Ben Dooks wrote: > Currently the header uses .work and .short to build .work? :) > instructions. This means the output data does not get marked as an > instruction which can cause issues such as BE8 code failures. Are you aware of any actual failures caused by this? Use of .short/.long instead of .inst confuses tools like objdump, but there should be no runtime issue because vmlinux doesn't have the mapping symbols anyway. If there's an actual bug here, we need to fix it, though. > > Signed-off-by: Ben Dooks > --- > arch/arm/include/asm/opcodes.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h > index e796c59..4c7cac1 100644 > --- a/arch/arm/include/asm/opcodes.h > +++ b/arch/arm/include/asm/opcodes.h > @@ -103,9 +103,15 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr); > #define __opcode_to_mem_arm(x) ___opcode_swab32(x) > #define __opcode_to_mem_thumb16(x) ___opcode_swab16(x) > #define __opcode_to_mem_thumb32(x) ___opcode_swahb32(x) > +#ifdef ARM_HAVE_INST > #define ___asm_opcode_to_mem_arm(x) ___asm_opcode_swab32(x) > #define ___asm_opcode_to_mem_thumb16(x) ___asm_opcode_swab16(x) > #define ___asm_opcode_to_mem_thumb32(x) ___asm_opcode_swahb32(x) > +#else > +#define ___asm_opcode_to_mem_arm(x) ___asm_opcode_identity32(x) > +#define ___asm_opcode_to_mem_thumb16(x) ___asm_opcode_identity16(x) > +#define ___asm_opcode_to_mem_thumb32(x) ___asm_opcode_identity32(x) > +#endif /* !ARM_HAVE_INST */ Although these macros are not currently used for anything else, I intended the ___asm_* macros to mirror the __* macros here. I'd prefer that we just modify the ___inst* helper macros to get the required effect. > > #else /* ! CONFIG_CPU_ENDIAN_BE8 */ > > @@ -218,14 +224,27 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr); > > /* Helpers for the helpers. Don't use these directly. */ > #ifdef __ASSEMBLY__ > +#ifdef ARM_HAVE_INST > +#define ___inst_arm(x) .inst x > +#define ___inst_thumb16(x) .inst.w x > +#define ___inst_thumb32(first, second) .inst.w first, second > +#else > #define ___inst_arm(x) .long x > #define ___inst_thumb16(x) .short x > #define ___inst_thumb32(first, second) .short first, second > +#endif /* !ARM_HAVE_INST */ > +#else > +#ifdef ARM_HAVE_INST > +#define ___inst_arm(x) ".inst " __stringify(x) "\n\t" > +#define ___inst_thumb16(x) ".inst.w " __stringify(x) "\n\t" > +#define ___inst_thumb32(first, second) \ > + ".inst.w " __stringify(first) ", " __stringify(second) "\n\t" This doesn't look right, because we'll end up with two words instead of one. > #else > #define ___inst_arm(x) ".long " __stringify(x) "\n\t" > #define ___inst_thumb16(x) ".short " __stringify(x) "\n\t" > #define ___inst_thumb32(first, second) \ > ".short " __stringify(first) ", " __stringify(second) "\n\t" > +#endif /* !ARM_HAVE_INST */ > #endif > > #endif /* __ASM_ARM_OPCODES_H */ Can you have a go with the following patch? Beware -- I've not tested it yet... Cheers ---Dave From 1978c354e3871bfeecce12d2db66fa4032b1f183 Mon Sep 17 00:00:00 2001 From: Dave Martin Date: Mon, 11 Nov 2013 16:13:33 +0000 Subject: [PATCH] ARM: opcodes: Use .inst to emit instructions if available Use of assembler data directives such as .long and .short to emit instruction opcodes can confuse tools that try to interpret the linker output, such as disassemblers or debuggers. Newer versions of the GNU assembler support an .inst directive, which can be used instead to mark the emitted bytes correctly as instructions. This patch makes use of the .inst directive instead of data directives, if available. Signed-off-by: Dave Martin --- arch/arm/include/asm/opcodes.h | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h index e796c59..f64c646 100644 --- a/arch/arm/include/asm/opcodes.h +++ b/arch/arm/include/asm/opcodes.h @@ -199,12 +199,18 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr); */ #include +#ifdef HAVE_ARM_INST +#define __inst_arm(x) ___inst_arm(x) +#define __inst_thumb32(x) ___inst_thumb32(x) +#define __inst_thumb16(x) ___inst_thumb16(x) +#else /* ! HAVE_ARM_INST */ #define __inst_arm(x) ___inst_arm(___asm_opcode_to_mem_arm(x)) #define __inst_thumb32(x) ___inst_thumb32( \ ___asm_opcode_to_mem_thumb16(___asm_opcode_thumb32_first(x)), \ ___asm_opcode_to_mem_thumb16(___asm_opcode_thumb32_second(x)) \ ) #define __inst_thumb16(x) ___inst_thumb16(___asm_opcode_to_mem_thumb16(x)) +#endif /* ! HAVE_ARM_INST */ #ifdef CONFIG_THUMB2_KERNEL #define __inst_arm_thumb16(arm_opcode, thumb_opcode) \ @@ -216,16 +222,36 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr); #define __inst_arm_thumb32(arm_opcode, thumb_opcode) __inst_arm(arm_opcode) #endif -/* Helpers for the helpers. Don't use these directly. */ +/* + * Helpers for the helpers. Don't use these directly. + * Note that the interface for these depends on whether or not HAVE_ARM_INST + * is defined. + */ #ifdef __ASSEMBLY__ + +#ifdef HAVE_ARM_INST +#define ___inst_arm(x) .inst x +#define ___inst_thumb16(x) .inst.n x +#define ___inst_thumb32(x) .inst.w x +#else /* ! HAVE_ARM_INST */ #define ___inst_arm(x) .long x #define ___inst_thumb16(x) .short x #define ___inst_thumb32(first, second) .short first, second -#else +#endif /* ! HAVE_ARM_INST */ + +#else /* ! __ASSEMBLY__ */ + +#ifdef HAVE_ARM_INST +#define ___inst_arm(x) ".inst " __stringify(x) "\n\t" +#define ___inst_thumb16(x) ".inst.n " __stringify(x) "\n\t" +#define ___inst_thumb32(x) ".inst.w " __stringify(x) "\n\t" +#else /* ! HAVE_ARM_INST */ #define ___inst_arm(x) ".long " __stringify(x) "\n\t" #define ___inst_thumb16(x) ".short " __stringify(x) "\n\t" #define ___inst_thumb32(first, second) \ ".short " __stringify(first) ", " __stringify(second) "\n\t" -#endif +#endif /* ! HAVE_ARM_INST */ + +#endif /* ! __ASSEMBLY__ */ #endif /* __ASM_ARM_OPCODES_H */