Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754111Ab1C2TCY (ORCPT ); Tue, 29 Mar 2011 15:02:24 -0400 Received: from smtp-out.google.com ([74.125.121.67]:57108 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750942Ab1C2TCX convert rfc822-to-8bit (ORCPT ); Tue, 29 Mar 2011 15:02:23 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=google.com; s=beta; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=HFb+rEAM/xdqL0kANlRXexOwXLrUpIXCi1gSniIDEVecq6cBkIqyCptWntWNqDVSPa vm6w1datJ2eaVJ02RDPA== MIME-Version: 1.0 In-Reply-To: <1300307259-834-1-git-send-email-sjg@chromium.org> References: <1300307259-834-1-git-send-email-sjg@chromium.org> Date: Tue, 29 Mar 2011 12:02:19 -0700 X-Google-Sender-Auth: dnW7D2UzP04F6r2Gw37C5kMdJ_k Message-ID: Subject: Re: [PATCH] ARM: Use generic BUG() handler From: Simon Glass To: linux-arm-kernel@lists.infradead.org Cc: Russell King , Tony Lindgren , Nicolas Pitre , Catalin Marinas , Joe Perches , Laurent Pinchart , Alexander Shishkin , Phil Carmody , Rabin Vincent , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7368 Lines: 226 Hi, does anyone have any more comments on this? Has anyone tried it apart from anish kumar? Thanks, Simon On Wed, Mar 16, 2011 at 1:27 PM, Simon Glass wrote: > From: Simon Glass > > ARM uses its own BUG() handler which makes its output slightly different > from other archtectures. > > One of the problems is that the ARM implementation doesn't report the function > with the BUG() in it, but always reports the PC being in __bug(). The generic > implementation doesn't have this problem. > > Currently we get something like: > > kernel BUG at fs/proc/breakme.c:35! > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > ... > PC is at __bug+0x20/0x2c > > With this patch it displays: > > kernel BUG at fs/proc/breakme.c:35! > Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP > ... > PC is at write_breakme+0xd0/0x1b4 > > This implementation uses an undefined instruction to implement BUG, and sets up > a bug table containing the relevant information. Many version of gcc do not > support %c properly for ARM (inserting a # when they shouldn't) so we work > around this using distasteful macro magic. > > Also the backtrace the message now appears even when CONFIG_ARM_UNWIND is > defined. > > You can fall back to the previous BUG implementation by making GENERIC_BUG > default to n in arch/arm/Kconfig. > > Change-Id: I07d77c832e816f5ad2390e25f466ddf750adecf4 > > Signed-off-by: Simon Glass > --- > ?arch/arm/Kconfig ? ? ? ? ? ? ?| ? ?4 +++ > ?arch/arm/include/asm/bug.h ? ?| ? 57 +++++++++++++++++++++++++++++++++++++++++ > ?arch/arm/kernel/traps.c ? ? ? | ? 24 +++++++++++++++++ > ?arch/arm/kernel/vmlinux.lds.S | ? 12 ++++++++ > ?4 files changed, 97 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 166efa2..7c8f11c 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -191,6 +191,10 @@ config VECTORS_BASE > ? ? ? ?help > ? ? ? ? ?The base address of exception vectors. > > +config GENERIC_BUG > + ? ? ? def_bool y > + ? ? ? depends on BUG > + > ?source "init/Kconfig" > > ?source "kernel/Kconfig.freezer" > diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h > index 4d88425..3f8a798 100644 > --- a/arch/arm/include/asm/bug.h > +++ b/arch/arm/include/asm/bug.h > @@ -3,6 +3,61 @@ > > > ?#ifdef CONFIG_BUG > + > +#ifdef CONFIG_GENERIC_BUG > + > +/* > + * Use a suitable undefined instruction to use for ARM/Thumb2 bug handling. > + * We need to be careful not to conflict with those used by other modules and > + * the register_undef_hook() system. > + */ > +#ifdef CONFIG_THUMB2_KERNEL > +#define BUG_INSTR_VALUE 0xde02 > +#define BUG_INSTR_TYPE ".hword " > +#else > +#define BUG_INSTR_VALUE 0xe7f001f2 > +#define BUG_INSTR_TYPE ".word " > +#endif > + > + > +#define BUG() _BUG(__FILE__, __LINE__, BUG_INSTR_VALUE) > +#define _BUG(file, line, value) __BUG(file, line, value) > + > +#ifdef CONFIG_DEBUG_BUGVERBOSE > + > +/* > + * The extra indirection is to ensure that the __FILE__ string comes through > + * OK. Many version of gcc do not support the asm %c parameter which would be > + * preferable to this unpleasantness. We use mergeable string sections to > + * avoid multiple copies of the string appearing in the kernel image. > + */ > + > +#define __BUG(__file, __line, __value) ? ? ? ? ? ? ? ? ? ? ? ? \ > +do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \ > + ? ? ? BUILD_BUG_ON(sizeof(struct bug_entry) != 12); ? ? ? ? ? \ > + ? ? ? asm volatile("1:\t" BUG_INSTR_TYPE #__value "\n" ? ? ? ?\ > + ? ? ? ? ? ? ? ".pushsection .rodata.str, \"aMS\", 1\n" ? ? ? ?\ > + ? ? ? ? ? ? ? "2:\t.asciz " #__file "\n" ? ? ? ? ? ? ? ? ? ? ?\ > + ? ? ? ? ? ? ? ".popsection\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \ > + ? ? ? ? ? ? ? ".pushsection __bug_table,\"a\"\n" ? ? ? ? ? ? ?\ > + ? ? ? ? ? ? ? "3:\t.word 1b, 2b\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ?\ > + ? ? ? ? ? ? ? "\t.hword " #__line ", 0\n" ? ? ? ? ? ? ? ? ? ? \ > + ? ? ? ? ? ? ? ".popsection"); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \ > + ? ? ? unreachable(); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\ > +} while (0) > + > +#else ?/* not CONFIG_DEBUG_BUGVERBOSE */ > + > +#define __BUG(__file, __line, __value) ? ? ? ? ? ? ? ? ? ? ? ? \ > +do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \ > + ? ? ? asm volatile(BUG_INSTR_TYPE #__value); ? ? ? ? ? ? ? ? ?\ > + ? ? ? unreachable(); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\ > +} while (0) > +#endif ?/* CONFIG_DEBUG_BUGVERBOSE */ > + > + > +#else ?/* not CONFIG_GENERIC_BUG */ > + > ?#ifdef CONFIG_DEBUG_BUGVERBOSE > ?extern void __bug(const char *file, int line) __attribute__((noreturn)); > > @@ -16,6 +71,8 @@ extern void __bug(const char *file, int line) __attribute__((noreturn)); > > ?#endif > > +#endif ?/* CONFIG_GENERIC_BUG */ > + > ?#define HAVE_ARCH_BUG > ?#endif > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index ee57640..1199cfe 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -21,6 +21,7 @@ > ?#include > ?#include > ?#include > +#include > ?#include > ?#include > > @@ -163,6 +164,7 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > ?#ifdef CONFIG_ARM_UNWIND > ?static inline void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) > ?{ > + ? ? ? printk("Backtrace: "); > ? ? ? ?unwind_backtrace(regs, tsk); > ?} > ?#else > @@ -271,6 +273,8 @@ void die(const char *str, struct pt_regs *regs, int err) > ? ? ? ?spin_lock_irq(&die_lock); > ? ? ? ?console_verbose(); > ? ? ? ?bust_spinlocks(1); > + ? ? ? if (!user_mode(regs)) > + ? ? ? ? ? ? ? report_bug(regs->ARM_pc, regs); > ? ? ? ?ret = __die(str, err, thread, regs); > > ? ? ? ?if (regs && kexec_should_crash(thread->task)) > @@ -302,6 +306,26 @@ void arm_notify_die(const char *str, struct pt_regs *regs, > ? ? ? ?} > ?} > > +#ifdef CONFIG_GENERIC_BUG > + > +int is_valid_bugaddr(unsigned long pc) > +{ > +#ifdef CONFIG_THUMB2_KERNEL > + ? ? ? unsigned short bkpt; > +#else > + ? ? ? unsigned long bkpt; > +#endif > + > + ? ? ? if (pc < PAGE_OFFSET) > + ? ? ? ? ? ? ? return 0; > + ? ? ? if (probe_kernel_address((unsigned *)pc, bkpt)) > + ? ? ? ? ? ? ? return 0; > + > + ? ? ? return bkpt == BUG_INSTR_VALUE; > +} > + > +#endif > + > ?static LIST_HEAD(undef_hook); > ?static DEFINE_SPINLOCK(undef_lock); > > diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S > index 6146279..4f22346 100644 > --- a/arch/arm/kernel/vmlinux.lds.S > +++ b/arch/arm/kernel/vmlinux.lds.S > @@ -80,6 +80,18 @@ SECTIONS > > ? ? ? ?PERCPU(PAGE_SIZE) > > + ? ? ? /* > + ? ? ? * .exit.text is discarded at runtime, not link time, to deal with > + ? ? ? * ?references from bug_table > + ? ? ? */ > + ? ? ? .exit.text : AT(ADDR(.exit.text)) { > + ? ? ? ? ? ? ? EXIT_TEXT > + ? ? ? } > + > + ? ? ? .exit.data : AT(ADDR(.exit.data)) { > + ? ? ? ? ? ? ? EXIT_DATA > + ? ? ? } > + > ?#ifndef CONFIG_XIP_KERNEL > ? ? ? ?. = ALIGN(PAGE_SIZE); > ? ? ? ?__init_end = .; > -- > 1.7.3.1 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/