MINSTATE_START_SAVE_MIN_PHYS looking very confused

MINSTATE_START_SAVE_MIN_PHYS looking very confused

[David Mosberger]

While working on the libunwind-enabled Linux kernel, I noticed that
MINSTATE_START_SAVE_MIN_PHYS is loking rather confused.  Part of the
problem seems to have been introduced by rja's patch from last
December (it cause "r1" to be setup incorrectly when already in kerenl
mode).  However, the bigger issue to me is: given that we have a
separate INIT stack, why bother distinguishing between an INIT that
hits user-level vs. one that hits the kernel.  It's not possible to
handle nested INITs anyhow, so why not just simplify that code to
always switch to the INIT stack?

	--david

Re: MINSTATE_START_SAVE_MIN_PHYS looking very confused

[David Mosberger]

>>>>> On Fri, 6 May 2005 05:08:37 -0700, David Mosberger <davidm@linux.hpl.hp.com> said:

  David> It's not possible to handle nested INITs anyhow, so why not
  David> just simplify that code to always switch to the INIT stack?

I suspect Keith will tell me "that's what I've been working on". ;-)

However, the existing code clearly is bogus, putting the kernel stack
at a (almost) random place when INIT hits while in kernel-mode.  Patch
below fixes this.

	--david

[IA64] Fix stack placement when INIT hits in kernel mode.

Without this patch, the stack is placed _below_ the current task
structure, which is risky at best.

Tony, I think this patch needs to go into 2.6.12, since it fixes a
real bug.  Without it, INIT may case secondary errors, which would be
most unpleasant.

Signed-off-by: David Mosberger-Tang <davidm@hpl.hp.com>

Index: arch/ia64/kernel/minstate.h
=================================--- b5f77812a2ece5addfd983e38eda85017039a29d/arch/ia64/kernel/minstate.h  (mode:100644 sha1:1dbc7b2497c90d4b3c0a8cef4c7afb94e7abc315)
+++ uncommitted/arch/ia64/kernel/minstate.h  (mode:100644)
@@ -41,7 +41,7 @@
 (pKStk) addl r3=THIS_CPU(ia64_mca_data),r3;;							\
 (pKStk) ld8 r3 = [r3];;										\
 (pKStk) addl r3=IA64_MCA_CPU_INIT_STACK_OFFSET,r3;;						\
-(pKStk) addl sp=IA64_STK_OFFSET-IA64_PT_REGS_SIZE,r3;						\
+(pKStk) addl r1=IA64_STK_OFFSET-IA64_PT_REGS_SIZE,r3;						\
 (pUStk)	mov ar.rsc=0;		/* set enforced lazy mode, pl 0, little-endian, loadrs=0 */	\
 (pUStk)	addl r22=IA64_RBS_OFFSET,r1;		/* compute base of register backing store */	\
 	;;											\
@@ -50,7 +50,6 @@
 (pUStk)	mov r23=ar.bspstore;				/* save ar.bspstore */			\
 (pUStk)	dep r22=-1,r22,61,3;			/* compute kernel virtual addr of RBS */	\
 	;;											\
-(pKStk) addl r1=-IA64_PT_REGS_SIZE,r1;		/* if in kernel mode, use sp (r12) */		\
 (pUStk)	mov ar.bspstore=r22;			/* switch to kernel RBS */			\
 	;;											\
 (pUStk)	mov r18=ar.bsp;										\

Re: MINSTATE_START_SAVE_MIN_PHYS looking very confused

[Russ Anderson]

David Mosberger wrote:
> 
>          However, the bigger issue to me is: given that we have a
> separate INIT stack, why bother distinguishing between an INIT that
> hits user-level vs. one that hits the kernel.  It's not possible to
> handle nested INITs anyhow, so why not just simplify that code to
> always switch to the INIT stack?

I don't know the historical reason for the user/kernel differences
in the code.  Maybe concerns about saving the right state information
depending on the context?  You're right that it could be made to 
treat both the same.

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

Re: MINSTATE_START_SAVE_MIN_PHYS looking very confused

[David Mosberger]

>>>>> On Fri, 6 May 2005 11:15:48 -0500 (CDT), Russ Anderson <rja@sgi.com> said:

  Russ> David Mosberger wrote:

  >>  However, the bigger issue to me is: given that we have a
  >> separate INIT stack, why bother distinguishing between an INIT
  >> that hits user-level vs. one that hits the kernel.  It's not
  >> possible to handle nested INITs anyhow, so why not just simplify
  >> that code to always switch to the INIT stack?

  Russ> I don't know the historical reason for the user/kernel
  Russ> differences in the code.  Maybe concerns about saving the
  Russ> right state information depending on the context?

I don't know the history there either.  At least in today's
environment with per-CPU INIT stacks, the code makes little sense,
though.

Keith, are your MCA cleanups close to being ready for prime-time?

	--david

Re: MINSTATE_START_SAVE_MIN_PHYS looking very confused

[Keith Owens]

On Mon, 9 May 2005 06:34:38 -0700, 
David Mosberger <davidm@napali.hpl.hp.com> wrote:
>Keith, are your MCA cleanups close to being ready for prime-time?

Stalled for four weeks, due to SLES9 work and the changeover from bk to
git.  I hope to get back to the MCA changes this week.