Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933298AbbGGSrq (ORCPT ); Tue, 7 Jul 2015 14:47:46 -0400 Received: from mail-pd0-f174.google.com ([209.85.192.174]:35390 "EHLO mail-pd0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932630AbbGGSr2 (ORCPT ); Tue, 7 Jul 2015 14:47:28 -0400 MIME-Version: 1.0 X-Originating-IP: [50.206.43.50] In-Reply-To: <1436189569-28802-3-git-send-email-jcmvbkbc@gmail.com> References: <1436189569-28802-1-git-send-email-jcmvbkbc@gmail.com> <1436189569-28802-3-git-send-email-jcmvbkbc@gmail.com> Date: Tue, 7 Jul 2015 11:47:27 -0700 Message-ID: Subject: Re: [PATCH 2/8] xtensa: keep exception/interrupt stack continuous From: Chris Zankel To: Max Filippov Cc: "linux-xtensa@linux-xtensa.org" , LKML , Marc Gauthier Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6177 Lines: 168 Hi Max, Not such a big fan about these changes as they introduce additional load and stores and a branch. Copy spill area: - is this only for debugging? Could the debugger identify the 'kernel exception' frame and handle it appropriately? - if we need it for debugging/perf, maybe move these line under a kernel option (could be enabled by default) - since we know the kernel stack frame size, why can't we just load from that offset (instead of the l32i and add). Also, can't we use s32e (need to look up the spec again). Branch: - is there any logical change in the code or are these only to avoid using a0? - if for a0, couldn't we load a0 just before the 'xsr a3, ps' and keep the code as before? Thanks, -Chris On Mon, Jul 6, 2015 at 6:32 AM, Max Filippov wrote: > Restore original a0 in the kernel exception stack frame. This way it > looks like the frame that got interrupt/exception did alloca (copy a0 and > a1 potentially spilled under old stack to the new location as well) to > save registers and then did a call to handler. The point where > interrupt/exception was taken is not in the stack chain, only in pt_regs > (call4 from that address can be simulated to keep it in the stack trace). > > Signed-off-by: Max Filippov > --- > arch/xtensa/kernel/entry.S | 60 +++++++++++++++++++++++++++------------------- > 1 file changed, 35 insertions(+), 25 deletions(-) > > diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S > index 82bbfa5..ab6a857 100644 > --- a/arch/xtensa/kernel/entry.S > +++ b/arch/xtensa/kernel/entry.S > @@ -218,7 +218,7 @@ _user_exception: > /* We are back to the original stack pointer (a1) */ > > 2: /* Now, jump to the common exception handler. */ > - > + movi a0, 0 # terminate user stack trace with 0 > j common_exception > > ENDPROC(user_exception) > @@ -264,6 +264,19 @@ ENTRY(kernel_exception) > .globl _kernel_exception > _kernel_exception: > > + /* Copy spill slots of a0 and a1 to imitate movsp > + * in order to keep exception stack continuous > + */ > + l32i a0, a1, PT_AREG1 > + addi a2, a1, -16 > + addi a0, a0, -16 > + l32i a3, a0, 0 > + l32i a0, a0, 4 > + s32i a3, a2, 0 > + s32i a0, a2, 4 > + > + l32i a0, a1, PT_AREG0 # restore saved a0 > + > /* Save SAR and turn off single stepping */ > > movi a2, 0 > @@ -338,52 +351,50 @@ common_exception: > xsr a2, lcount > s32i a2, a1, PT_LCOUNT > > - /* It is now save to restore the EXC_TABLE_FIXUP variable. */ > + /* It is now safe to restore the EXC_TABLE_FIXUP variable. */ > > - rsr a0, exccause > movi a3, 0 > rsr a2, excsave1 > - s32i a0, a1, PT_EXCCAUSE > s32i a3, a2, EXC_TABLE_FIXUP > > - /* All unrecoverable states are saved on stack, now, and a1 is valid, > - * so we can allow exceptions and interrupts (*) again. > - * Set PS(EXCM = 0, UM = 0, RING = 0, OWB = 0, WOE = 1, INTLEVEL = X) > + /* Save remaining unrecoverable states (exccause, ps) on stack. > + * Now we can allow exceptions again. In case we've got an interrupt > + * PS.INTLEVEL is set to LOCKLEVEL disabling furhter interrupts, > + * otherwise it's left unchanged. > * > - * (*) We only allow interrupts if they were previously enabled and > - * we're not handling an IRQ > + * Set PS(EXCM = 0, UM = 0, RING = 0, OWB = 0, WOE = 1, INTLEVEL = X) > */ > > rsr a3, ps > - addi a0, a0, -EXCCAUSE_LEVEL1_INTERRUPT > - movi a2, LOCKLEVEL > + movi a2, (1 << PS_WOE_BIT) > extui a3, a3, PS_INTLEVEL_SHIFT, PS_INTLEVEL_WIDTH > - # a3 = PS.INTLEVEL > - moveqz a3, a2, a0 # a3 = LOCKLEVEL iff interrupt > - movi a2, 1 << PS_WOE_BIT > or a3, a3, a2 > - rsr a0, exccause > - xsr a3, ps > > + rsr a2, exccause > + bnei a2, EXCCAUSE_LEVEL1_INTERRUPT, 1f > + movi a3, (1 << PS_WOE_BIT) | LOCKLEVEL > +1: > + xsr a3, ps > + s32i a2, a1, PT_EXCCAUSE > s32i a3, a1, PT_PS # save ps > > /* Save lbeg, lend */ > > - rsr a2, lbeg > + rsr a4, lbeg > rsr a3, lend > - s32i a2, a1, PT_LBEG > + s32i a4, a1, PT_LBEG > s32i a3, a1, PT_LEND > > /* Save SCOMPARE1 */ > > #if XCHAL_HAVE_S32C1I > - rsr a2, scompare1 > - s32i a2, a1, PT_SCOMPARE1 > + rsr a3, scompare1 > + s32i a3, a1, PT_SCOMPARE1 > #endif > > /* Save optional registers. */ > > - save_xtregs_opt a1 a2 a4 a5 a6 a7 PT_XTREGS_OPT > + save_xtregs_opt a1 a3 a4 a5 a6 a7 PT_XTREGS_OPT > > #ifdef CONFIG_TRACE_IRQFLAGS > l32i a4, a1, PT_DEPC > @@ -391,8 +402,7 @@ common_exception: > * while PS.EXCM was set, i.e. interrupts disabled. > */ > bgeui a4, VALID_DOUBLE_EXCEPTION_ADDRESS, 1f > - l32i a4, a1, PT_EXCCAUSE > - bnei a4, EXCCAUSE_LEVEL1_INTERRUPT, 1f > + bnei a2, EXCCAUSE_LEVEL1_INTERRUPT, 1f > /* We came here with an interrupt means interrupts were enabled > * and we've just disabled them. > */ > @@ -407,8 +417,8 @@ common_exception: > > rsr a4, excsave1 > mov a6, a1 # pass stack frame > - mov a7, a0 # pass EXCCAUSE > - addx4 a4, a0, a4 > + mov a7, a2 # pass EXCCAUSE > + addx4 a4, a2, a4 > l32i a4, a4, EXC_TABLE_DEFAULT # load handler > > /* Call the second-level handler */ > -- > 1.8.1.4 > -- 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/