From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2] xen: arm64: more useful logging on bad trap. Date: Thu, 19 Feb 2015 10:17:48 +0000 Message-ID: <54E5B84C.5000302@citrix.com> References: <1424278915-6468-1-git-send-email-ian.campbell@citrix.com> <54E4CB49.1020209@citrix.com> <1424335550.25370.27.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424335550.25370.27.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: julien.grall@linaro.org, tim@xen.org, jintack@cs.columbia.edu, stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 19/02/15 08:45, Ian Campbell wrote: > On Wed, 2015-02-18 at 17:26 +0000, Andrew Cooper wrote: >> On 18/02/15 17:01, Ian Campbell wrote: >>> Dump the register state before panicing so we have some clue where the >>> issue occurred. Also decode the ESR register a bit to save having to >>> grab a pen and paper. >>> >>> ESR_EL2 is a 32-bit register, so use SYSREG_READ32 not ..._READ64, as >>> we already do correctly in the main trap handler. >>> >>> While here notice that do_trap_serror is never called and remove it. >>> >>> Signed-off-by: Ian Campbell >>> Reviewed-by: Julien Grall >>> Tested-by: Jintack Lim >>> Cc: jintack@cs.columbia.edu >>> --- >>> Jintack, since you have a system which is exhibiting SError issues I >>> wonder if I could prevail on you to give this patch a try on your >>> system and report on the output. I've only compile tested this myself. >>> >>> v2: Added blank line after variable declaration >>> Split log message into two lines. >>> s/code/ESR/ and reformat a little. >>> --- >>> xen/arch/arm/arm64/traps.c | 14 ++++++-------- >>> 1 file changed, 6 insertions(+), 8 deletions(-) >>> >>> diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c >>> index 1693b5d..31a3ca5 100644 >>> --- a/xen/arch/arm/arm64/traps.c >>> +++ b/xen/arch/arm/arm64/traps.c >>> @@ -24,11 +24,6 @@ >>> >>> #include >>> >>> -asmlinkage void do_trap_serror(struct cpu_user_regs *regs) >>> -{ >>> - panic("Unhandled serror trap"); >>> -} >>> - >>> static const char *handler[]= { >>> "Synchronous Abort", >>> "IRQ", >>> @@ -38,11 +33,14 @@ static const char *handler[]= { >>> >>> asmlinkage void do_bad_mode(struct cpu_user_regs *regs, int reason) >>> { >>> - uint64_t esr = READ_SYSREG64(ESR_EL2); >>> - printk("Bad mode in %s handler detected, code 0x%08"PRIx64"\n", >>> - handler[reason], esr); >>> + union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) }; >>> + >>> + printk("Bad mode in %s handler detected", handler[reason]); >>> + printk("ESR=0x%08"PRIx32": EC=%"PRIx32", IL=%"PRIx32", ISS=%"PRIx32"\n", >>> + hsr.bits, hsr.ec, hsr.len, hsr.iss); >> This would be better as a single printk() call, otherwise a different >> cpu issuing a printk() could interleave in the middle of the line. >> >> Also, you appear to have dropped the space between "detected" and "ESR" > That's because I forgot to add the \n to the end of the first printk > (the intention was to make the log line <80 columns by splitting it into > two lines). Having fixed that I think your first comment then becomes > irrelevant? Or is there some benefit to printk("foo\nbar\n")? Not completely irrelevant, but certainly far less problematic, and something I wouldn't worry about. ~Andrew