On Apr 30, 2008, at 4:54 PM, Benjamin Herrenschmidt wrote:
On Wed, 2008-04-30 at 04:27 -0500, Kumar Gala wrote:
* Cleanup the code a bit my allocating an INT_FRAME on our exception
stack there by make references go from GPR11-INT_FRAME_SIZE(r8) to
just GPR(r8)
^^ 11 ?
r8 is correct for my example. My point is s/GPR11-INT_FRAME_SIZE/GPR11/
* simplify {lvl}_transfer_to_handler code by moving the copying of
the
temp registers we use if we come from user space into the PROLOG
* If the exception came from kernel mode copy thread_info flags,
preempt, and task pointer from the process thread_info.
Signed-off-by: Kumar Gala <[EMAIL PROTECTED]>
---
I'm not sure if the copying of TI_FLAGS, TI_PREEMPT, and TI_TASK
are really needed. I'm a bit concerned what to do if we get a
CriticalInput while in kernel mode and the handler causes a
reschedule.
Well, reschedule or signal, either way, you can't handle them on the
way
out. Maybe an option there is to set the flag in the normal kernel
stack's thread info and trigger an interrupt asap via the PIT so
things
get handled ?
If we don't handle reschedule or signal will we actually not function
properly? I assume reschedule isn't an issue, but could we lose a
signal?
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/
head_booke.h
index d647e05..78baec5 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -78,12 +78,12 @@
slwi r8,r8,2; \
addis r8,r8,[EMAIL PROTECTED]; \
lwz r8,[EMAIL PROTECTED](r8); \
- addi r8,r8,THREAD_SIZE;
+ addi r8,r8,THREAD_SIZE-INT_FRAME_SIZE;
#else
#define BOOKE_LOAD_EXC_LEVEL_STACK(level) \
lis r8,[EMAIL PROTECTED]; \
lwz r8,[EMAIL PROTECTED](r8); \
- addi r8,r8,THREAD_SIZE;
+ addi r8,r8,THREAD_SIZE-INT_FRAME_SIZE;
#endif
Nothing specific to your patch, but those level##_STACK_TOP seem to
be pretty badly named if you end up -adding- THREAD_SIZE to actually
get to the stack's top. Do they really contain the stack top or do
they in fact contain the stack base/bottom ?
That might be stale from how the old code worked. I can never
remember what we consider the top and bottom of the stack. (please
remind me and I'll fixup the comments and variables).
/*
@@ -97,22 +97,35 @@
#define EXC_LEVEL_EXCEPTION_PROLOG(exc_level, exc_level_srr0,
exc_level_srr1) \
mtspr exc_level##_SPRG,r8; \
BOOKE_LOAD_EXC_LEVEL_STACK(exc_level);/* r8 points to the
exc_level stack*/ \
- stw r10,GPR10-INT_FRAME_SIZE(r8); \
- stw r11,GPR11-INT_FRAME_SIZE(r8); \
+ stw r9,GPR9(r8); /* save various registers */\
+ stw r10,GPR10(r8); \
+ stw r11,GPR11(r8); \
+ mfspr r11,SPRN_SPRG3; /* if from user, start at top of */\
+ lwz r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
+ addi r11,r11,THREAD_SIZE-INT_FRAME_SIZE; /* Alloc exception frm */\
So you do the above whether it will actually be needed or not right ?
good point I can move this down.
mfcr r10; /* save CR in r10 for now */\
- mfspr r11,exc_level_srr1; /* check whether user or kernel */\
- andi. r11,r11,MSR_PR; \
- mr r11,r8; \
- mfspr r8,exc_level##_SPRG; \
+ mfspr r9,exc_level_srr1; /* check whether user or kernel */\
+ andi. r9,r9,MSR_PR; \
beq 1f; \
/* COMING FROM USER MODE */ \
- mfspr r11,SPRN_SPRG3; /* if from user, start at top of */\
- lwz r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
- addi r11,r11,THREAD_SIZE; \
-1: subi r11,r11,INT_FRAME_SIZE; /* Allocate an exception frame
*/\
+ lwz r9,GPR9(r8); /* copy regs from exception stack */\
+ stw r9,GPR9(r11); \
+ lwz r9,GPR10(r8); \
+ stw r9,GPR10(r11); \
+ lwz r9,GPR11(r8); \
+ stw r9,GPR11(r11); \
+ b 2f; \
Are we concerned with performances here or not ? Because using the
same
reg all along isn't the best ...
I thought about that. The only case its a bit of an issue is for
CriticalInput. I don't see Watchdog, Debug, or MachineCheck as being
performance critical. I can use r10 if I save and restored the CR or
some other register.
+ /* COMING FROM PRIV MODE */ \
+1: lwz r9,TI_FLAGS-THREAD_SIZE(r11); \
+ stw r9,TI_FLAGS-THREAD_SIZE(r8); \
+ lwz r9,TI_PREEMPT-THREAD_SIZE(r11); \
+ stw r9,TI_PREEMPT-THREAD_SIZE(r8); \
+ lwz r9,TI_TASK-THREAD_SIZE(r11); \
+ stw r9,TI_TASK-THREAD_SIZE(r8); \
+ mr r11,r8; \
Don't you want to also stick in HARDIRQ_OFFSET in preempt_count or
leave that to C code ?
just leaving it to C code. I assume that preempt_count should be the
same value on entry and exit. Do think we should set HARDIRQ_OFFSET
for debug level exceptions?
Also, you might want at one point to tell
lockdep about IRQs being disabled in case they were enabled when
you took the exception. (Do that after you've saved enough
stuff of crouse)
Where should I look for an example of how to convey that information
to lockdep?
- k
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
