On Tue, 2006-07-11 at 13:02 +0530, Amit S. Kale wrote:
> Hi Andrei,
>
> Thanks for these patches. Your point about the need for a general cleanup of
> current thread info pointer fetch is exellent. We definitely need to add some
> ifdefs around stack size.
>
> I would also like to add more code to kernel entry points to save thread info
> pointer. It would be too intrusive though, since with such a code in place,
> acceptance into mainline kernel becomes difficult. We could add it to a
> non-lite patch.
Sounds like the "critical exception stack" is used for other things than
single stepping; like traps or interrupts.
-piet
>
> Suggestions?
> -Amit
>
>
> On Wednesday 05 July 2006 21:58, Andrei Konovalov wrote:
> > Hello,
> >
> > Here:
> >
> > + /*
> > + * On Book E and perhaps other processsors, singlestep is handled on
> > + * the critical exception stack. This causes current_thread_info()
> > + * to fail, since it it locates the thread_info by masking off
> > + * the low bits of the current stack pointer. We work around
> > + * this issue by copying the thread_info from the kernel stack
> > + * before calling kgdb_handle_exception, and copying it back
> > + * afterwards. On most processors the copy is avoided since
> > + * exception_thread_info == thread_info.
> > + */
> > + thread_info = (struct thread_info *)(regs->gpr[1] & ~(THREAD_SIZE-1));
> > + exception_thread_info = current_thread_info();
> >
> > - calling current_thread_info() is incorrect, as it assumes 8K stack, but
> > for Book E and ppc4xx the critical exception stack is 4K and is 4K-aligned
> > (see head_4xx.S, head_44x.S and head_fsl_booke.S in arch/ppc/kernel/).
> > So depending on the actual kernel layout/configuration
> > exception_thread_info could point 4K below the critical exception stack.
> > Even if
> > (exception_stack_bottom & ~(THREAD_SIZE-1)) != 0 (i.e. the stack is NOT 8K
> > aligned) everything seems to work OK in most cases (the overwritten data
> > are not so critical?). But we did encountered one case when trying to
> > single step from the default breakpoint made the kernel to hang.
> >
> > There are several possible ways to fix that:
> >
> > - make the critical exception stack to be 8K aligned:
> >
> > ===========================================================================
> >========== diff -upr a/arch/ppc/kernel/head_4xx.S
> > b/arch/ppc/kernel/head_4xx.S --- a/arch/ppc/kernel/head_4xx.S
> > 2006-07-05 20:23:51.000000000 +0400 +++ b/arch/ppc/kernel/head_4xx.S
> > 2006-07-05 19:54:51.000000000 +0400 @@ -1001,7 +1001,7 @@ swapper_pg_dir:
> >
> > /* Stack for handling critical exceptions from kernel mode */
> > .section .bss
> > - .align 12
> > + .align 13
> > exception_stack_bottom:
> > .space 4096
> > critical_stack_top:
> > ===========================================================================
> >==========
> >
> > - replace call to current_thread_info() with:
> >
> > ===========================================================================
> >========== diff -upr a/arch/ppc/kernel/kgdb.c b/arch/ppc/kernel/kgdb.c
> > --- a/arch/ppc/kernel/kgdb.c 2006-07-05 19:53:29.000000000 +0400
> > +++ b/arch/ppc/kernel/kgdb.c 2006-07-05 20:18:06.000000000 +0400
> > @@ -106,29 +106,32 @@ static int kgdb_breakpoint(struct pt_reg
> > static int kgdb_singlestep(struct pt_regs *regs)
> > {
> > struct thread_info *thread_info, *exception_thread_info;
> > + register unsigned long sp asm("r1");
> >
> > if (user_mode(regs))
> > return 0;
> > +
> > +#if defined(CONFIG_40x) || defined(CONFIG_44x) || defined(CONFIG_E500)
> > /*
> > - * On Book E and perhaps other processsors, singlestep is handled on
> > + * On Book E, ppc40x, and ppc44x, singlestep is handled on
> > * the critical exception stack. This causes current_thread_info()
> > - * to fail, since it it locates the thread_info by masking off
> > + * to fail, since it locates the thread_info by masking off
> > * the low bits of the current stack pointer. We work around
> > * this issue by copying the thread_info from the kernel stack
> > * before calling kgdb_handle_exception, and copying it back
> > - * afterwards. On most processors the copy is avoided since
> > - * exception_thread_info == thread_info.
> > + * afterwards.
> > */
> > +#define CRIT_STACK_SIZE 0x1000 /* 4kB */
> > thread_info = (struct thread_info *)(regs->gpr[1] &
> > ~(THREAD_SIZE-1)); - exception_thread_info = current_thread_info();
> > -
> > - if (thread_info != exception_thread_info)
> > - memcpy(exception_thread_info, thread_info, sizeof
> > *thread_info); + exception_thread_info = (struct thread_info *)(sp &
> > ~(CRIT_STACK_SIZE-1)); + memcpy(exception_thread_info, thread_info,
> > sizeof *thread_info);
> >
> > kgdb_handle_exception(0, SIGTRAP, 0, regs);
> >
> > - if (thread_info != exception_thread_info)
> > - memcpy(thread_info, exception_thread_info, sizeof
> > *thread_info); + memcpy(thread_info, exception_thread_info, sizeof
> > *thread_info); +#else
> > + kgdb_handle_exception(0, SIGTRAP, 0, regs);
> > +#endif
> >
> > return 1;
> > }
> > ===========================================================================
> >==========
> >
> > or
> >
> > ===========================================================================
> >========== diff -upr a/arch/ppc/kernel/kgdb.c c/arch/ppc/kernel/kgdb.c
> > --- a/arch/ppc/kernel/kgdb.c 2006-07-05 19:53:29.000000000 +0400
> > +++ c/arch/ppc/kernel/kgdb.c 2006-07-05 20:17:51.000000000 +0400
> > @@ -110,9 +110,9 @@ static int kgdb_singlestep(struct pt_reg
> > if (user_mode(regs))
> > return 0;
> > /*
> > - * On Book E and perhaps other processsors, singlestep is handled on
> > + * On Book E and perhaps other processors, singlestep is handled on
> > * the critical exception stack. This causes current_thread_info()
> > - * to fail, since it it locates the thread_info by masking off
> > + * to fail, since it locates the thread_info by masking off
> > * the low bits of the current stack pointer. We work around
> > * this issue by copying the thread_info from the kernel stack
> > * before calling kgdb_handle_exception, and copying it back
> > @@ -122,8 +122,14 @@ static int kgdb_singlestep(struct pt_reg
> > thread_info = (struct thread_info *)(regs->gpr[1] &
> > ~(THREAD_SIZE-1)); exception_thread_info = current_thread_info();
> >
> > - if (thread_info != exception_thread_info)
> > + if (thread_info != exception_thread_info) {
> > + #define CRIT_STACK_SIZE 0x1000
> > + register unsigned long sp asm("r1");
> > +
> > + exception_thread_info =
> > + (struct thread_info *)(sp & ~(CRIT_STACK_SIZE-1));
> > memcpy(exception_thread_info, thread_info, sizeof
> > *thread_info); + }
> >
> > kgdb_handle_exception(0, SIGTRAP, 0, regs);
> >
> > ===========================================================================
> >==========
> >
> > To say the true, I don't like all the three (these are just to illustrate
> > the idea). Seems some general cleanup in handling the stack size would make
> > sense.
> >
> > Suggestions?
> >
> >
> > Thanks,
> > Andrei
> >
> >
> > Using Tomcat but need to do more? Need to support web services, security?
> > Get stuff done quickly with pre-integrated technology to make your job
> > easier Download IBM WebSphere Application Server v.1.0.1 based on Apache
> > Geronimo
> > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> > _______________________________________________
> > Kgdb-bugreport mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
>
>
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> Kgdb-bugreport mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
--
Piet Delaney
BlueLane Teck
W: (408) 200-5256; [EMAIL PROTECTED]
H: (408) 243-8872; [EMAIL PROTECTED]
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Kgdb-bugreport mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
