[PATCH] powerpc: Fix size of st_nlink on 64bit
commit e57f93cc53b7 (powerpc: get rid of nlink_t uses, switch to explicitly-sized type) changed the size of st_nlink on ppc64 from a long to a short, resulting in boot failures. Signed-off-by: Anton Blanchard --- Index: linux-build/arch/powerpc/include/asm/stat.h === --- linux-build.orig/arch/powerpc/include/asm/stat.h2012-06-02 21:25:50.322275743 +1000 +++ linux-build/arch/powerpc/include/asm/stat.h 2012-06-02 21:26:35.183130538 +1000 @@ -30,7 +30,7 @@ struct stat { unsigned long st_dev; ino_t st_ino; #ifdef __powerpc64__ - unsigned short st_nlink; + unsigned long st_nlink; mode_t st_mode; #else mode_t st_mode; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> > On Fri, 2012-06-01 at 17:42 -0500, Scott Wood wrote: > > On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote: > > > BTW. My point of view is that this whole business about MSR:DE is a HW > > > design bug. There should be -no- (absolutely 0) interaction between the > > > SW state and the HW debugger for normal operations unless the user of > > > the debugger explicitly wants to change some state. > > > > I agree entirely, and e500mc at least has less of this than e500v2 (not > > sure if it still needs MSR[DE], but supposedly it doesn't have the > > requirement for there to be a valid instruction at the debug vector, > > which is lots of fun when booting). But this isn't exactly something > > Freescale is going to replace existing chips over. > > > > Getting all the way to zero interaction would require a completely > > separate debug facility so software can debug at the same time. I'd be > > all for that (and let's throw in a third, for the hypervisor), but I'm > > not the one that needs to be convinced. > > You can find a good compromise. If you have some kind of SPR letting you > know now many DACs and IACs are available, you could essentially > "reserve" some for HW debug with the probe. Not as good as a fully > separate facility but still better than stepping on each other toes. > > Things like DBCR should probably still be separated. There's no excuse > for the MSR:DE bullshit tho :-) hmm, where does this go w.r.t the patch? Got the feeling that the best thing is to just turn MSR:DE on and be done with it? Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix size of st_nlink on 64bit
On Sat, 2012-06-02 at 21:34 +1000, Anton Blanchard wrote: > commit e57f93cc53b7 (powerpc: get rid of nlink_t uses, switch to > explicitly-sized type) changed the size of st_nlink on ppc64 from > a long to a short, resulting in boot failures. > > Signed-off-by: Anton Blanchard Acked-by: Benjamin Herrenschmidt Al, did you CC me on that ? I don't remember seeing it but it could be my fault... Cheers, Ben. > --- > > Index: linux-build/arch/powerpc/include/asm/stat.h > === > --- linux-build.orig/arch/powerpc/include/asm/stat.h 2012-06-02 > 21:25:50.322275743 +1000 > +++ linux-build/arch/powerpc/include/asm/stat.h 2012-06-02 > 21:26:35.183130538 +1000 > @@ -30,7 +30,7 @@ struct stat { > unsigned long st_dev; > ino_t st_ino; > #ifdef __powerpc64__ > - unsigned short st_nlink; > + unsigned long st_nlink; > mode_t st_mode; > #else > mode_t st_mode; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote: > > hmm, where does this go w.r.t the patch? Got the feeling that the > best thing is to just turn MSR:DE on and be done with it? Not unconditionally, we need to have a close look, that might be ok specifically for BookE 32-bit, it's certainly not ok for BookE 64-bit at this point. For now, I'm ok with a debug CONFIG_* option. Also do we know if MSR:DE has any performance impact on any CPU ? I know having DACs enabled has a major impact on some for example. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix size of st_nlink on 64bit
Hi Anton, On Sat, 2 Jun 2012 21:34:52 +1000 Anton Blanchard wrote: > > commit e57f93cc53b7 (powerpc: get rid of nlink_t uses, switch to > explicitly-sized type) changed the size of st_nlink on ppc64 from > a long to a short, resulting in boot failures. > > Signed-off-by: Anton Blanchard Would this affect my (early user mode) boot problems reported yesterday; /init: 71: mknod: Permission denied /init: 88: mknod: Permission denied /init: 88: mknod: Permission denied -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpS0MjgO32my.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix size of st_nlink on 64bit
Hi Stephen, > > commit e57f93cc53b7 (powerpc: get rid of nlink_t uses, switch to > > explicitly-sized type) changed the size of st_nlink on ppc64 from > > a long to a short, resulting in boot failures. > > > > Signed-off-by: Anton Blanchard > > Would this affect my (early user mode) boot problems reported > yesterday; > > /init: 71: mknod: Permission denied > /init: 88: mknod: Permission denied > /init: 88: mknod: Permission denied Very similar to the errors I was seeing so I think the patch will fix it. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/3] ppc32/kprobe: complete kprobe and migrate exception frame
On 05/10/2012 11:50 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-12-15 at 19:00 +0800, Tiejun Chen wrote: >> We can't emulate stwu since that may corrupt current exception stack. >> So we will have to do real store operation in the exception return code. >> >> Firstly we'll allocate a trampoline exception frame below the kprobed >> function stack and copy the current exception frame to the trampoline. >> Then we can do this real store operation to implement 'stwu', and reroute >> the trampoline frame to r1 to complete this exception migration. >> >> Signed-off-by: Tiejun Chen >> --- >> arch/powerpc/kernel/entry_32.S | 35 +++ >> 1 files changed, 35 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S >> index 56212bc..0cdd27d 100644 >> --- a/arch/powerpc/kernel/entry_32.S >> +++ b/arch/powerpc/kernel/entry_32.S >> @@ -850,6 +850,41 @@ resume_kernel: >> >> /* interrupts are hard-disabled at this point */ >> restore: >> +lwz r3,_MSR(r1) /* Returning to user mode? */ >> +andi. r0,r3,MSR_PR >> +bne 1f > > .../... > Sorry for this delay response since I can't take time to do this last week :( > Wouldn't it be better to use resume_kernel here ? Agreed :) > > IE. We already have restore_user vs. resume_kernel labels, including > the PR test. In the !PREEMPT case, resume_kernel is empty but it's > there, so you can just "populate" it, ie, something like: > > restore_user: > ... existing dbcr0 stuff ... > b restore > > resume_kernel: <-- removed the ifdef CONFIG_PREEMPT > > ... Do the stack store business here... > > #ifdef CONFIG_PREEMPT > ... move the preempt code here... > #endif > > restore: > ... existing stuff ... > > Also, the added advantage is that the code to calc > the thread info pointer and load the TI_FLAG can be > shared now between your stuff and preempt, ie: > > resume_kernel: > rlwinm r9,r1,0,0,(31-THREAD_SHIFT) > lwz r0,TI_FLAGS(r9) > andis. r0,r0,_TIF_EMULATE_STACK_STORE@h > bne-emulate_stack_store > #ifdef CONFIG_PREEMPT > lwz r8,TI_PREEMPT(r9) <-- note use of r8 instead of r0, > I -think- r8 can be clobbered > here but pls dbl check Its should be safe. > cmpwi 0,r8,0 > bne restore > andi. r0,r0,_TIF_NEED_RESCHED > etc... Please check if next, v3, is fine. > >> +/* check current_thread_info, _TIF_EMULATE_STACK_STORE */ >> +rlwinm r9,r1,0,0,(31-THREAD_SHIFT) >> +lwz r0,TI_FLAGS(r9) >> +andis. r0,r0,_TIF_EMULATE_STACK_STORE@h >> +beq+1f >> + >> +addir9,r1,INT_FRAME_SIZE/* Get the kprobed function entry */ >> + >> +lwz r3,GPR1(r1) >> +subir3,r3,INT_FRAME_SIZE/* dst: Allocate a trampoline exception >> frame */ >> +mr r4,r1 /* src: current exception frame */ >> +li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ >> +mr r1,r3 /* Reroute the trampoline frame to r1 */ >> +bl memcpy /* Copy from the original to the >> trampoline */ >> + >> +/* Do real store operation to complete stwu */ >> +lwz r5,GPR1(r1) >> +stw r9,0(r5) > > Ok, I think I -finally- understand your trick of using r1 + > INT_FRAME_SIZE as the value to store :-) It makes sense and it's > actually a nice hack :-) > > I would recommend that in the C code part of the emulation, you > add some sanity checking to make sure we don't overflow the > kernel stack etc... it should come in generally handy especially > if what's your trying to debug with kprobes is a kernel stack > overflow :-) Added. > >> +/* Clear _TIF_EMULATE_STACK_STORE flag */ >> +rlwinm r9,r1,0,0,(31-THREAD_SHIFT) >> +lis r11,_TIF_EMULATE_STACK_STORE@h >> +addir9,r9,TI_FLAGS >> +0: lwarx r8,0,r9 >> +andcr8,r8,r11 >> +#ifdef CONFIG_IBM405_ERR77 >> +dcbt0,r9 >> +#endif >> +stwcx. r8,0,r9 >> +bne-0b >> +1: >> + >> #ifdef CONFIG_44x >> BEGIN_MMU_FTR_SECTION >> b 1f > > BTW. Are you going to do a ppc64 variant of that patch ? I'd like to go ppc64 ASAP once we did on ppc32 is good enough :) Tiejun > > Cheers, > Ben. > > > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[v3 PATCH 1/3] powerpc/kprobe: introduce a new thread flag
We need to add a new thread flag, TIF_EMULATE_STACK_STORE, for emulating stack store operation while exiting exception. Signed-off-by: Tiejun Chen --- arch/powerpc/include/asm/thread_info.h |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h index bcebc75..45d098c 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -110,6 +110,8 @@ static inline struct thread_info *current_thread_info(void) #define TIF_NOERROR12 /* Force successful syscall return */ #define TIF_NOTIFY_RESUME 13 /* callback before returning to user */ #define TIF_SYSCALL_TRACEPOINT 15 /* syscall tracepoint instrumentation */ +#defineTIF_EMULATE_STACK_STORE 17 /* Is an instruction emulation + for stack store? */ /* as above, but as bit values */ #define _TIF_SYSCALL_TRACE (1
[v3 PATCH 2/3] ppc32/kprobe: complete kprobe and migrate exception frame
We can't emulate stwu since that may corrupt current exception stack. So we will have to do real store operation in the exception return code. Firstly we'll allocate a trampoline exception frame below the kprobed function stack and copy the current exception frame to the trampoline. Then we can do this real store operation to implement 'stwu', and reroute the trampoline frame to r1 to complete this exception migration. Signed-off-by: Tiejun Chen --- arch/powerpc/kernel/entry_32.S | 43 +-- 1 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index ba3aeb4..e7eefdf 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -829,19 +829,50 @@ restore_user: bnel- load_dbcr0 #endif -#ifdef CONFIG_PREEMPT b restore /* N.B. the only way to get here is from the beq following ret_from_except. */ resume_kernel: - /* check current_thread_info->preempt_count */ + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */ rlwinm r9,r1,0,0,(31-THREAD_SHIFT) - lwz r0,TI_PREEMPT(r9) - cmpwi 0,r0,0 /* if non-zero, just restore regs and return */ - bne restore lwz r0,TI_FLAGS(r9) + andis. r0,r0,_TIF_EMULATE_STACK_STORE@h + beq+1f + + addir8,r1,INT_FRAME_SIZE/* Get the kprobed function entry */ + + lwz r3,GPR1(r1) + subir3,r3,INT_FRAME_SIZE/* dst: Allocate a trampoline exception frame */ + mr r4,r1 /* src: current exception frame */ + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */ + mr r1,r3 /* Reroute the trampoline frame to r1 */ + bl memcpy /* Copy from the original to the trampoline */ + + /* Do real store operation to complete stwu */ + lwz r5,GPR1(r1) + stw r8,0(r5) + + /* Clear _TIF_EMULATE_STACK_STORE flag */ + rlwinm r9,r1,0,0,(31-THREAD_SHIFT) + lis r11,_TIF_EMULATE_STACK_STORE@h + addir5,r9,TI_FLAGS +0: lwarx r8,0,r5 + andcr8,r8,r11 +#ifdef CONFIG_IBM405_ERR77 + dcbt0,r5 +#endif + stwcx. r8,0,r5 + bne-0b +1: + +#ifdef CONFIG_PREEMPT + /* check current_thread_info->preempt_count */ + lwz r8,TI_PREEMPT(r9) + cmpwi 0,r8,0 /* if non-zero, just restore regs and return */ + bne restore andi. r0,r0,_TIF_NEED_RESCHED beq+restore + lwz r3,_MSR(r1) andi. r0,r3,MSR_EE/* interrupts off? */ beq restore /* don't schedule if so */ #ifdef CONFIG_TRACE_IRQFLAGS @@ -862,8 +893,6 @@ resume_kernel: */ bl trace_hardirqs_on #endif -#else -resume_kernel: #endif /* CONFIG_PREEMPT */ /* interrupts are hard-disabled at this point */ -- 1.5.6 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[v3 PATCH 3/3] ppc32/kprobe: don't emulate store when kprobe stwu r1
We don't do the real store operation for kprobing 'stwu Rx,(y)R1' since this may corrupt the exception frame, now we will do this operation safely in exception return code after migrate current exception frame below the kprobed function stack. So we only update gpr[1] here and trigger a thread flag to mask this. Note we should make sure if we trigger kernel stack over flow. Signed-off-by: Tiejun Chen --- arch/powerpc/lib/sstep.c | 37 +++-- 1 files changed, 35 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index 9a52349..a4ce463 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -566,7 +566,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr) unsigned long int ea; unsigned int cr, mb, me, sh; int err; - unsigned long old_ra; + unsigned long old_ra, val3, r1; long ival; opcode = instr >> 26; @@ -1486,11 +1486,44 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr) goto ldst_done; case 36:/* stw */ - case 37:/* stwu */ val = regs->gpr[rd]; err = write_mem(val, dform_ea(instr, regs), 4, regs); goto ldst_done; + case 37:/* stwu */ + __asm__ __volatile__("mr %0,1" : "=r" (r1) :); + + val = regs->gpr[rd]; + val3 = dform_ea(instr, regs); + /* +* For PPC32 we always use stwu to change stack point with r1. So +* this emulated store may corrupt the exception frame, now we +* have to provide the exception frame trampoline, which is pushed +* below the kprobed function stack. So we only update gpr[1] but +* don't emulate the real store operation. We will do real store +* operation safely in exception return code by checking this flag. +*/ + if ((ra == 1) && !(regs->msr & MSR_PR) && (val3 >= r1)) { + /* +* Check if we will touch kernel sack overflow +*/ + if (r1 - STACK_INT_FRAME_SIZE <= current->thread.ksp_limit) { + printk(KERN_CRIT "Can't kprobe this since Kernel stack overflow.\n"); + err = -EINVAL; + break; + } + + /* +* Check if we already set since that means we'll +* lose the previous value. +*/ + WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE)); + set_thread_flag(TIF_EMULATE_STACK_STORE); + err = 0; + } else + err = write_mem(val, val3, 4, regs); + goto ldst_done; + case 38:/* stb */ case 39:/* stbu */ val = regs->gpr[rd]; -- 1.5.6 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[v3 PATCH 0/3] ppc32/kprobe: Fix a bug for kprobe stwu r1
Changes from V2: * populate those existed codes to reorganize codes * add check if we'll trigger kernel stack over flow Changes from V1: * use memcpy simply to withdraw copy_exc_stack * add !(regs->msr & MSR_PR)) and WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE)); to make sure we're in goot path. * move this migration process inside 'restore' * clear TIF flag atomically Tiejun Chen (3): powerpc/kprobe: introduce a new thread flag ppc32/kprobe: complete kprobe and migrate exception frame ppc32/kprobe: don't emulate store when kprobe stwu r1 arch/powerpc/include/asm/thread_info.h |3 ++ arch/powerpc/kernel/entry_32.S | 43 ++- arch/powerpc/lib/sstep.c | 37 ++- 3 files changed, 74 insertions(+), 9 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v3 PATCH 0/3] ppc32/kprobe: Fix a bug for kprobe stwu r1
On 06/03/2012 01:07 PM, Tiejun Chen wrote: > Changes from V2: > > * populate those existed codes to reorganize codes > * add check if we'll trigger kernel stack over flow BTW, I always validate this on mpc8536ds(UP)/mpc8572ds(SMP) with/without CONFIG_PREEMPT. Tiejun > > Changes from V1: > > * use memcpy simply to withdraw copy_exc_stack > * add !(regs->msr & MSR_PR)) and > WARN_ON(test_thread_flag(TIF_EMULATE_STACK_STORE)); > to make sure we're in goot path. > * move this migration process inside 'restore' > * clear TIF flag atomically > > Tiejun Chen (3): > powerpc/kprobe: introduce a new thread flag > ppc32/kprobe: complete kprobe and migrate exception frame > ppc32/kprobe: don't emulate store when kprobe stwu r1 > > arch/powerpc/include/asm/thread_info.h |3 ++ > arch/powerpc/kernel/entry_32.S | 43 ++- > arch/powerpc/lib/sstep.c | 37 ++- > 3 files changed, 74 insertions(+), 9 deletions(-) > ___ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix size of st_nlink on 64bit
Hi Anton, On Sun, 3 Jun 2012 13:48:36 +1000 Anton Blanchard wrote: > > > > commit e57f93cc53b7 (powerpc: get rid of nlink_t uses, switch to > > > explicitly-sized type) changed the size of st_nlink on ppc64 from > > > a long to a short, resulting in boot failures. > > > > > > Signed-off-by: Anton Blanchard > > > > Would this affect my (early user mode) boot problems reported > > yesterday; > > > > /init: 71: mknod: Permission denied > > /init: 88: mknod: Permission denied > > /init: 88: mknod: Permission denied > > Very similar to the errors I was seeing so I think the patch will fix > it. Great. One less thing to bisect tomorrow :-) -- Cheers, Stephen Rothwells...@canb.auug.org.au pgp5Oo7aByK7K.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev