From: ben.dooks@codethink•co.uk (Ben Dooks)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 2/3] ARM: fix BUG() detection
Date: Fri, 16 Aug 2013 09:31:48 +0100 [thread overview]
Message-ID: <520DE374.5080903@codethink.co.uk> (raw)
In-Reply-To: <20130815150939.GC4103@localhost.localdomain>
On 15/08/13 16:09, Dave Martin wrote:
> On Wed, Aug 14, 2013 at 05:42:42PM +0100, Ben Dooks wrote:
>> The detection of the instruction used by BUG() did not take into account
>> the differences in endian-ness between instruction and data. Change the
>> code to use the relevant helpers in<asm/opcodes.h> to translate the
>> endian-ness of the instructions.
>>
>> Fixes issue reported by Dave Martin<Dave.Martin@arm•com>
>
> It probably makes sense to fold this with the preceding patch.
>
>>
>> Signed-off-by: Ben Dooks<ben.dooks@codethink•co.uk>
>> ---
>> arch/arm/include/asm/bug.h | 10 ++++++----
>> arch/arm/kernel/traps.c | 8 +++++---
>> 2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
>> index b95da52..b274bde 100644
>> --- a/arch/arm/include/asm/bug.h
>> +++ b/arch/arm/include/asm/bug.h
>> @@ -2,6 +2,8 @@
>> #define _ASMARM_BUG_H
>>
>> #include<linux/linkage.h>
>> +#include<linux/types.h>
>> +#include<asm/opcodes.h>
>>
>> #ifdef CONFIG_BUG
>>
>> @@ -12,10 +14,10 @@
>> */
>> #ifdef CONFIG_THUMB2_KERNEL
>> #define BUG_INSTR_VALUE 0xde02
>> -#define BUG_INSTR_TYPE ".inst.w "
>> +#define BUG_INSTR(__value) __inst_thumb16(__value)
>> #else
>> #define BUG_INSTR_VALUE 0xe7f001f2
>> -#define BUG_INSTR_TYPE ".inst "
>> +#define BUG_INSTR(__value) __inst_arm(__value)
>> #endif
>
> Looks OK. You could make things a bit less verbose by
>
> #define BUG_INSTR __inst_thumb16(BUG_INSTR_VALUE)
> #else
> ...
> #define BUG_INSTR __inst_arm(BUG_INSTR_VALUE)
I was trying to keep the macro as similar as possible, although
there is only one bug instruction value at the moment the __BUG()
code seems to assume that in the future there may be more of them
and that it should be able to handle them.
>>
>> @@ -33,7 +35,7 @@
>>
>> #define __BUG(__file, __line, __value) \
>> do { \
>> - asm volatile("1:\t" BUG_INSTR_TYPE #__value "\n" \
>> + asm volatile("1:\t" BUG_INSTR(__value) "\n" \
>> ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>> "2:\t.asciz " #__file "\n" \
>> ".popsection\n" \
>> @@ -48,7 +50,7 @@ do { \
>>
>> #define __BUG(__file, __line, __value) \
>> do { \
>> - asm volatile(BUG_INSTR_TYPE #__value); \
>> + asm volatile(BUG_INSTR(__value) "\n"); \
>> unreachable(); \
>> } while (0)
>> #endif /* CONFIG_DEBUG_BUGVERBOSE */
>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>> index cb67b04..ae2d828 100644
>> --- a/arch/arm/kernel/traps.c
>> +++ b/arch/arm/kernel/traps.c
>> @@ -344,15 +344,17 @@ void arm_notify_die(const char *str, struct pt_regs *regs,
>> int is_valid_bugaddr(unsigned long pc)
>> {
>> #ifdef CONFIG_THUMB2_KERNEL
>> - unsigned short bkpt;
>> + u16 bkpt;
>> + u16 insn = __opcode_to_mem_thumb16(BUG_INSTR_VALUE);
>> #else
>> - unsigned long bkpt;
>> + u32 bkpt;
>> + u32 insn = __opcode_to_mem_arm(BUG_INSTR_VALUE);
>> #endif
>>
>> if (probe_kernel_address((unsigned *)pc, bkpt))
>
> Hmm, the (unsigned *) actually looks weird here now I look at it.
>
> probe_kernel_address does (__force typeof(bkpt) __user *) on it anyway,
> so I guess that's harmless. void * might make more sense, though this
> patch may not be the place to fix it.
>
> Cheers
> ---Dave
>
>> return 0;
>>
>> - return bkpt == BUG_INSTR_VALUE;
>> + return bkpt == insn;
>> }
>>
>> #endif
>> --
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
next prev parent reply other threads:[~2013-08-16 8:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-14 16:42 BE8 patch updates Ben Dooks
2013-08-14 16:42 ` [PATCH 1/3] ARM: Correct BUG() assembly to ensure it is endian-agnostic Ben Dooks
2013-08-14 16:42 ` [PATCH 2/3] ARM: fix BUG() detection Ben Dooks
2013-08-15 15:09 ` Dave Martin
2013-08-16 8:31 ` Ben Dooks [this message]
2013-08-16 13:24 ` Dave Martin
2013-08-14 16:42 ` [PATCH 3/3] ARM: kdgb: use <asm/opcodes.h> for data to be assembled as intruction Ben Dooks
2013-08-15 15:53 ` Dave Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=520DE374.5080903@codethink.co.uk \
--to=ben.dooks@codethink$(echo .)co.uk \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox