From: Vince Weaver <vince@xxxxxxxxxx>
Date: Wed, 24 Jan 2007 22:00:12 -0500 (EST)
> I've reproduced it, here are the results (the bug is still at line 413,
> my debug code pushed it down a bit):
>
> [ 263.773194] VMW: fault_code=4 address=ff0cc000 regs->tpc=70131660
>
I have a theory about what is happening here, but not why.
But I'd like some more confirmation. What I think is going
on is that (in do_sparc64_fault):
fault_code = get_thread_fault_code();
if (notify_page_fault(DIE_PAGE_FAULT, "page_fault", regs,
fault_code, 0, SIGSEGV) == NOTIFY_STOP)
return;
si_code = SEGV_MAPERR;
address = current_thread_info()->fault_address;
The addr/code state is changed between the read of
get_thread_fault_code() and ->fault_address.
Besides the main fault path, there is only one other path which
could trigger here which writes to these thread state variables.
That's the window spill/fill code, BUT that code guards the writes
with a check if we came from privileged mode which should always
be true, for example:
rdpr %tstate, %g1
andcc %g1, TSTATE_PRIV, %g0
saved
be,pn %xcc, 1f
and %g1, TSTATE_CWP, %g1
retry
1: mov FAULT_CODE_WRITE | FAULT_CODE_DTLB | FAULT_CODE_WINFIXUP, %g4
stb %g4, [%g6 + TI_FAULT_CODE]
stx %g5, [%g6 + TI_FAULT_ADDR]
If TSTATE_PRIV is set, it skips the write to TI_FAULT_{CODE,ADDR}.
I highly suspected this code path because the value of "address" is
in the user process stack address range, however the logic is clearly
correct here.
The other problematic case are other cpus accessing the
current_thread_info()->flags member in a non-atomic manner.
This is critical because get_thread_fault_code() is a byte
embedded in that area. So if another thread does a read-modify-write
of current_thread_info->flags we can get:
Cpu 1 Cpu 2
... read ->flags, see old fault_code
store new fault_code
store new fault_address
write ->flags, puts old fault_code there
and that would look exactly like this.
I think the fault being taken is a stack fault via a save instruction
in userspace or similar. The most recent fault before that was an
instruction fault into GLIBC which set the fault code to FAULT_CODE_ITLB.
That's why we see FAULT_CODE_ITLB combined with a stack address.
I did some auditing and I really can't see how it can happen :-)
But we can test this theory with a relatively simple patch,
can you give this a try? What it does is it moves fault_code
out of the thread flags word.
If you can't trigger it, then my theory is right. If you can't,
then the bug is somewhere else :-)
Note, this patch is pretty straight forward, but I've only compile
tested it. I'm about to test boot it myself right now.
diff --git a/include/asm-sparc64/thread_info.h b/include/asm-sparc64/thread_info.h
index 2ebf7f2..75921fd 100644
--- a/include/asm-sparc64/thread_info.h
+++ b/include/asm-sparc64/thread_info.h
@@ -11,8 +11,6 @@
#define NSWINS 7
-#define TI_FLAG_BYTE_FAULT_CODE 0
-#define TI_FLAG_FAULT_CODE_SHIFT 56
#define TI_FLAG_BYTE_WSTATE 1
#define TI_FLAG_WSTATE_SHIFT 48
#define TI_FLAG_BYTE_CWP 2
@@ -49,7 +47,8 @@ struct thread_info {
int preempt_count; /* 0 => preemptable, <0 => BUG */
__u8 new_child;
__u8 syscall_noerror;
- __u16 __pad;
+ __u8 fault_code;
+ __u8 __pad;
unsigned long *utraps;
@@ -77,7 +76,6 @@ struct thread_info {
/* offsets into the thread_info struct for assembly code access */
#define TI_TASK 0x00000000
#define TI_FLAGS 0x00000008
-#define TI_FAULT_CODE (TI_FLAGS + TI_FLAG_BYTE_FAULT_CODE)
#define TI_WSTATE (TI_FLAGS + TI_FLAG_BYTE_WSTATE)
#define TI_CWP (TI_FLAGS + TI_FLAG_BYTE_CWP)
#define TI_CURRENT_DS (TI_FLAGS + TI_FLAG_BYTE_CURRENT_DS)
@@ -92,6 +90,7 @@ struct thread_info {
#define TI_PRE_COUNT 0x00000038
#define TI_NEW_CHILD 0x0000003c
#define TI_SYS_NOERROR 0x0000003d
+#define TI_FAULT_CODE 0x0000003e
#define TI_UTRAPS 0x00000040
#define TI_REG_WINDOW 0x00000048
#define TI_RWIN_SPTRS 0x000003c8
@@ -179,8 +178,8 @@ register struct thread_info *current_thread_info_reg asm("g6");
((unsigned char *)(&((ti)->flags)))
#define __cur_thread_flag_byte_ptr __thread_flag_byte_ptr(current_thread_info())
-#define get_thread_fault_code() (__cur_thread_flag_byte_ptr[TI_FLAG_BYTE_FAULT_CODE])
-#define set_thread_fault_code(val) (__cur_thread_flag_byte_ptr[TI_FLAG_BYTE_FAULT_CODE] = (val))
+#define get_thread_fault_code() (current_thread_info()->fault_code)
+#define set_thread_fault_code(val) (current_thread_info()->fault_code = (val))
#define get_thread_wstate() (__cur_thread_flag_byte_ptr[TI_FLAG_BYTE_WSTATE])
#define set_thread_wstate(val) (__cur_thread_flag_byte_ptr[TI_FLAG_BYTE_WSTATE] = (val))
#define get_thread_cwp() (__cur_thread_flag_byte_ptr[TI_FLAG_BYTE_CWP])
-
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
