Hi Max,

On 5/24/14, 3:19 PM, Max Filippov wrote:
There are two FIXMEs in the double exception handler 'for the extremely
unlikely case'. This case gets hit by gcc during kernel build once in
a few hours, resulting in an unrecoverable exception condition.
So, it wasn't really that 'extremely' unlikely. Glad you found a case and have a fix already! :-)

Just some minor questions/comments about the patch:

* Even with only a few instructions added to the double exception handler, are we still in the limits of the vector (esp. if there are no narrow instructions)? Does anyone know the current size limit?

* I guess you wanted to keep the vectors.S 'pure' to vector entries, and added the fixup handler to the entry.S. I'm wondering if it would make sense to keep the fixup closer to the cause, especially since ... (next point)

* There seems to be some duplication of code between the overflow handler and the fixup handler. Given that the fixup handler is really rare, would it be possible to just jump into the original code?

* Minor (personal preferences) inline...

Thanks,
-Chris

Cc: [email protected]
Signed-off-by: Max Filippov <[email protected]>
---
  arch/xtensa/kernel/entry.S   |  109 ++++++++++++++++++++++++++++++++++++++++++
  arch/xtensa/kernel/vectors.S |   46 ++++++++++--------
  2 files changed, 134 insertions(+), 21 deletions(-)

diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S
index ef7f499..c69b609 100644
--- a/arch/xtensa/kernel/entry.S
+++ b/arch/xtensa/kernel/entry.S
@@ -1400,6 +1400,115 @@ ENTRY(fast_syscall_spill_registers_fixup_return)
ENDPROC(fast_syscall_spill_registers_fixup_return) +/*
+ * Fixup handler for TLB miss in double exception handler for window owerflow.
+ * We get here with windowbase set to the window that was being spilled and
+ * a0 trashed. a0 bit 7 determines if this is a call8 (bit clear) or call12
+ * (bit set) window.
+ *
+ * We do the following here:
+ * - go to the original window retaining a0 value;
+ * - set up exception stack to return back to appropriate a0 restore code
+ *   (we'll need to rotate window back and there's no place to save this
+ *    information, use different return address for that);
+ * - handle the exception;
+ * - go to the window that was being spilled;
+ * - set up window_overflow_restore_a0_fixup as a fixup routine;
+ * - reload a0;
+ * - restore the original window;
+ * - reset the default fixup routine;
+ * - return to user. By the time we get to this fixup handler all information
+ *   about the conditions of the original double exception that happened in
+ *   the window overflow handler is lost, so we just return to userspace to
+ *   retry overflow from start.
+ *
+ * a0: value of depc, original value in depc
+ * a2: trashed, original value in EXC_TABLE_DOUBLE_SAVE
+ * a3: exctable, original value in excsave1
+ */
+
+ENTRY(window_overflow_restore_a0_fixup)
+
+       rsr     a0, ps
+       extui   a0, a0, PS_OWB_SHIFT, PS_OWB_WIDTH
+       rsr     a2, windowbase
+       sub     a0, a2, a0
+       extui   a0, a0, 0, 3
+       l32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       xsr     a3, excsave1
+
+       _beqi   a0, 1, .L1pane
+       _beqi   a0, 3, .L3pane
+
+       .macro  overflow_fixup_handle_exception_pane n
+.L\n\()pane:
+       xsr     a0, depc
Maybe use rsr instead of xsr as depc is not actually restored but overwritten a few instructions later.

+       rotw    -\n
+
+       xsr     a3, excsave1
+       wsr     a2, depc
+       l32i    a2, a3, EXC_TABLE_KSTK
+       s32i    a0, a2, PT_AREG0
+
+       movi    a0, .L\n\()restore
+       s32i    a0, a2, PT_DEPC
+
+       rsr     a0, exccause
+       addx4   a0, a0, a3
+       l32i    a0, a0, EXC_TABLE_FAST_USER
+       xsr     a3, excsave1
+       jx      a0
+
+       .endm
+
+       .macro  overflow_fixup_restore_a0_pane n
+
+.L\n\()restore:
+       rotw    \n
+       /* Need to preserve a0 value here to be able to handle exception
+        * that may occur on a0 reload from stack. It may occur because
+        * TLB miss handler may not be atomic and pointer to page table
+        * may be lost before we get here. There are no free registers,
+        * so we need to use EXC_TABLE_DOUBLE_SAVE area.
+        */
+       xsr     a3, excsave1
+       s32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       movi    a2, window_overflow_restore_a0_fixup
+       s32i    a2, a3, EXC_TABLE_FIXUP
+       l32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       xsr     a3, excsave1
+       bbsi.l  a0, 7, 1f
+       l32e    a0, a9, -16
+       j       2f
+1:
+       l32e    a0, a13, -16
+2:
+       rotw    -\n
+
+       .endm
+
+       overflow_fixup_handle_exception_pane 2
+       overflow_fixup_handle_exception_pane 1
+       overflow_fixup_handle_exception_pane 3
+
+       overflow_fixup_restore_a0_pane 2
+
+.Lset_default_fixup:
+       xsr     a3, excsave1
+       s32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       movi    a2, 0
+       s32i    a2, a3, EXC_TABLE_FIXUP
+       l32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       xsr     a3, excsave1
+       rfe
+
+       overflow_fixup_restore_a0_pane 1
+       j       .Lset_default_fixup
+       overflow_fixup_restore_a0_pane 3
+       j       .Lset_default_fixup
+
+ENDPROC(window_overflow_restore_a0_fixup)
+
  #ifdef CONFIG_MMU
  /*
   * We should never get here. Bail out!
diff --git a/arch/xtensa/kernel/vectors.S b/arch/xtensa/kernel/vectors.S
index f9e1ec3..13c115f 100644
--- a/arch/xtensa/kernel/vectors.S
+++ b/arch/xtensa/kernel/vectors.S
@@ -376,38 +376,42 @@ _DoubleExceptionVector_WindowOverflow:
        beqz    a2, 1f          # if at start of vector, don't restore
addi a0, a0, -128
-       bbsi    a0, 8, 1f       # don't restore except for overflow 8 and 12
-       bbsi    a0, 7, 2f
+       bbsi.l  a0, 8, 1f       # don't restore except for overflow 8 and 12
+
+       /*
+        * This fixup handler is for the extremely unlikely case where the
+        * overflow handler's reference thru a0 gets a hardware TLB refill
+        * that bumps out the (distinct, aliasing) TLB entry that mapped its
+        * prior references thru a9/a13, and where our reference now thru
+        * a9/a13 gets a 2nd-level miss exception (not hardware TLB refill).
+        */
+       movi    a2, window_overflow_restore_a0_fixup
+       s32i    a2, a3, EXC_TABLE_FIXUP
+       l32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
+       xsr     a3, excsave1
+
+       bbsi.l  a0, 7, 2f
/*
         * Restore a0 as saved by _WindowOverflow8().
-        *
-        * FIXME:  we really need a fixup handler for this L32E,
-        * for the extremely unlikely case where the overflow handler's
-        * reference thru a0 gets a hardware TLB refill that bumps out
-        * the (distinct, aliasing) TLB entry that mapped its prior
-        * references thru a9, and where our reference now thru a9
-        * gets a 2nd-level miss exception (not hardware TLB refill).
         */
- l32e a2, a9, -16
-       wsr     a2, depc        # replace the saved a0
-       j       1f
+       l32e    a0, a9, -16
+       wsr     a0, depc        # replace the saved a0
+       j       3f
2:
        /*
         * Restore a0 as saved by _WindowOverflow12().
-        *
-        * FIXME:  we really need a fixup handler for this L32E,
-        * for the extremely unlikely case where the overflow handler's
-        * reference thru a0 gets a hardware TLB refill that bumps out
-        * the (distinct, aliasing) TLB entry that mapped its prior
-        * references thru a13, and where our reference now thru a13
-        * gets a 2nd-level miss exception (not hardware TLB refill).
         */
- l32e a2, a13, -16
-       wsr     a2, depc        # replace the saved a0
+       l32e    a0, a13, -16
+       wsr     a0, depc        # replace the saved a0
+3:
+       xsr     a3, excsave1
+       movi    a0, 0
+       s32i    a0, a3, EXC_TABLE_FIXUP
+       s32i    a2, a3, EXC_TABLE_DOUBLE_SAVE
  1:
        /*
         * Restore WindowBase while leaving all address registers restored.

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to