Machine check recovery when kernel accesses poison

This series is initially targeted at the folks doing filesystems on top of NVDIMMs. They really want to be able to return -EIO when there is a h/w error (just like spinning rust, and SSD does).

I plan to use the same infrastructure to write a machine check aware "copy_from_user()" that will SIGBUS the calling application when a syscall touches poison in user space (just like we do when the application touches the poison itself).

Changes V5-V6
Andy: Provoked massive re-write by providing what is now part1 of this
patch series. This frees up two bits in the exception table fixup field that can be used to tag exception table entries as different "classes". This means we don't need my separate exception table fro machine checks. Also avoids duplicating fixup actions for #PF and #MC cases that were in version 5.
Andy: Use C99 array initializers to tie the various class fixup
functions back to the defintions of each class. Also give the functions meanningful names (not fixup_class0() etc.).
Boris: Cleaned up my lousy assembly code removing many spurious 'l'
modifiers on instructions.
Boris: Provided some helper functions for the machine check severity
calculation that make the code more readable.
Boris: Have __mcsafe_copy() return a structure with the 'remaining bytes'
in a separate field from the fault indicator. Boris had suggested Linux -EFAULT/-EINVAL ... but I thought it made more sense to return the exception number (X86_TRAP_MC, etc.) This finally kills off BIT(63) which has been controversial throughout all the early versions of this patch series.

Changes V4-V5
Tony: Extended __mcsafe_copy() to have fixup entries for both machine
check and page fault.

Changes V3-V4:

Andy:   Simplify fixup_mcexception() by dropping used-once local variable
Andy:   "Reviewed-by" tag added to part1
Boris:  Moved new functions to memcpy_64.S and declaration to asm/string_64.h
Boris:  Changed name s/mcsafe_memcpy/__mcsafe_copy/ to make it clear that this

is an internal function and that return value doesn't follow memcpy() semantics.
Boris: "Reviewed-by" tag added to parts 1&2

Changes V2-V3:

Andy: Don't hack "regs->ax = BIT(63) | addr;" in the machine check
handler. Now have better fixup code that computes the number of remaining bytes (just like page-fault fixup).

Andy:   #define for BIT(63). Done, plus couple of extra macros using it.
Boris:  Don't clutter up generic code (like mm/extable.c) with this.

I moved everything under arch/x86 (the asm-generic change is a more generic #define).
Boris: Dependencies for CONFIG_MCE_KERNEL_RECOVERY are too generic.
I made it a real menu item with default "n". Dan Williams will use "select MCE_KERNEL_RECOVERY" from his persistent filesystem code.
Boris: Simplify conditionals in mce.c by moving tolerant/kill_it
checks earlier, with a skip to end if they aren't set.

Boris:  Miscellaneous grammar/punctuation. Fixed.
Boris:  Don't leak spurious __start_mcextable symbols into kernels

that didn't configure MCE_KERNEL_RECOVERY. Done.
Tony: New code doesn't belong in user_copy_64.S/uaccess*.h. Moved
to new .S/.h files
Elliott:Cacheing behavior non-optimal. Could use movntdqa, vmovntdqa
or vmovntdqa on source addresses. I didn't fix this yet. Think of the current mcsafe_memcpy() as the first of several functions. This one is useful for small copies (meta-data) where the overhead of saving SSE/AVX state isn't justified.

Changes V1->V2:

0-day:  Reported build errors and warnings on 32-bit systems. Fixed
0-day:  Reported bloat to tinyconfig. Fixed
Boris:  Suggestions to use extra macros to reduce code duplication in _ASM_*EXTABLE. Done
Boris:  Re-write "tolerant==3" check to reduce indentation level. See below.
Andy:   Check IP is valid before searching kernel exception tables. Done.
Andy:   Explain use of BIT(63) on return value from mcsafe_memcpy(). Done (added decode macros).
Andy:   Untangle mess of code in tail of do_machine_check() to make it

clear what is going on (e.g. that we only enter the ist_begin_non_atomic() if we were called from user code, not from kernel!). Done.

Andy Lutomirski (1):
x86: Clean up extable entry format (and free up a bit)

Tony Luck (3):
x86: Cleanup and add a new exception class
x86, mce: Check for faults tagged in EXTABLE_CLASS_FAULT exception
table entries
x86, mce: Add __mcsafe_copy()

 arch/x86/Kconfig                          |  10 +++
 arch/x86/include/asm/asm.h                |  80 ++++++++++++------
 arch/x86/include/asm/string_64.h          |  10 +++
 arch/x86/include/asm/uaccess.h            |  17 +++-
 arch/x86/kernel/cpu/mcheck/mce-severity.c |  32 ++++++-
 arch/x86/kernel/cpu/mcheck/mce.c          |  71 ++++++++--------
 arch/x86/kernel/kprobes/core.c            |   2 +-
 arch/x86/kernel/traps.c                   |   6 +-
 arch/x86/kernel/x8664_ksyms_64.c          |   4 +
 arch/x86/lib/memcpy_64.S                  | 136 ++++++++++++++++++++++++++++++
 arch/x86/mm/extable.c                     |  66 ++++++++++-----
 arch/x86/mm/fault.c                       |   2 +-
 12 files changed, 347 insertions(+), 89 deletions(-)

2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

This adds two bits of fixup class information to a fixup entry, generalizing the uaccess_err hack currently in place.

Forward-ported-from-3.9-by: Tony Luck <[email protected]>

---
 arch/x86/include/asm/asm.h | 70 ++++++++++++++++++++++++++++++----------------
 arch/x86/mm/extable.c      | 21 ++++++++------
 2 files changed, 59 insertions(+), 32 deletions(-)

index 189679aba703..b64121ffb2da 100644
@@ -43,19 +43,47 @@
 #define _ASM_DI		__ASM_REG(di)
 
 /* Exception table entry */
-#ifdef __ASSEMBLY__
-# define _ASM_EXTABLE(from,to)					\
-	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
-	.long (from) - . ;					\
-	.long (to) - . ;					\
-	.popsection
 
-# define _ASM_EXTABLE_EX(from,to)				\
-	.pushsection "__ex_table","a" ;				\
-	.balign 8 ;						\
-	.long (from) - . ;					\
-	.long (to) - . + 0x7ffffff0 ;				\
+/*
+ * An exception table entry is 64 bits.  The first 32 bits are the offset
+ * from that entry to the potentially faulting instruction.  sortextable
+ * relies on that exact encoding.  The second 32 bits encode the fault
+ * handler address.
+ *
+ * We want to stick two extra bits of handler class into the fault handler
+ * address.  All of these are generated by relocations, so we can only
+ * rely on addition.  We therefore emit:
+ *
+ * (target - here) + (class) + 0x20000000
+ *
+ * This has the property that the two high bits are the class and the
+ * rest is easy to decode.
+ */
+
+/* There are two bits of extable entry class, added to a signed offset. */
+#define _EXTABLE_CLASS_DEFAULT	0		/* standard uaccess fixup */
+#define _EXTABLE_CLASS_EX	0x80000000	/* uaccess + set uaccess_err */
+
+/*
+ * The biases are the class constants + 0x20000000, as signed integers.
+ * This can't use ordinary arithmetic -- the assembler isn't that smart.
+ */
+#define _EXTABLE_BIAS_DEFAULT	0x20000000
+#define _EXTABLE_BIAS_EX	0x20000000 - 0x80000000
+
+#define _ASM_EXTABLE(from,to)						\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_DEFAULT)
+
+#define _ASM_EXTABLE_EX(from,to)					\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_EX)
+
+#ifdef __ASSEMBLY__
+# define _EXPAND_EXTABLE_BIAS(x) x
+# define _ASM_EXTABLE_CLASS(from,to,bias)				\
+	.pushsection "__ex_table","a" ;					\
+	.balign 8 ;							\
+	.long (from) - . ;						\
+	.long (to) - . + _EXPAND_EXTABLE_BIAS(bias) ;			\
 	.popsection
 
 # define _ASM_NOKPROBE(entry)					\

@@ -89,18 +117,12 @@
 	.endm
 
 #else
-# define _ASM_EXTABLE(from,to)					\
-	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
-	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - .\n"				\
-	" .popsection\n"
-
-# define _ASM_EXTABLE_EX(from,to)				\
-	" .pushsection \"__ex_table\",\"a\"\n"			\
-	" .balign 8\n"						\
-	" .long (" #from ") - .\n"				\
-	" .long (" #to ") - . + 0x7ffffff0\n"			\
+# define _EXPAND_EXTABLE_BIAS(x) #x
+# define _ASM_EXTABLE_CLASS(from,to,bias)				\
+	" .pushsection \"__ex_table\",\"a\"\n"				\
+	" .balign 8\n"							\
+	" .long (" #from ") - .\n"					\
+	" .long (" #to ") - . + " _EXPAND_EXTABLE_BIAS(bias) "\n"	\
 	" .popsection\n"
 /* For C file, we already have NOKPROBE_SYMBOL macro */
 #endif

index 903ec1e9c326..95e2ede71206 100644
@@ -8,16 +8,24 @@ ex_insn_addr(const struct exception_table_entry *x)
 {
 	return (unsigned long)&x->insn + x->insn;
 }
+static inline unsigned int
+ex_class(const struct exception_table_entry *x)
+{
+	return (unsigned int)x->fixup & 0xC0000000;
+}
+
 static inline unsigned long
 ex_fixup_addr(const struct exception_table_entry *x)
 {
-	return (unsigned long)&x->fixup + x->fixup;
+	long offset = (long)((u32)x->fixup & 0x3fffffff) - (long)0x20000000;
+	return (unsigned long)&x->fixup + offset;
 }
 
 int fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *fixup;
 	unsigned long new_ip;
+	unsigned int class;
 
 #ifdef CONFIG_PNPBIOS
 	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {

@@ -35,12 +43,12 @@ int fixup_exception(struct pt_regs *regs)
 
 	fixup = search_exception_tables(regs->ip);
 	if (fixup) {
+		class = ex_class(fixup);
 		new_ip = ex_fixup_addr(fixup);
 
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
+		if (class == _EXTABLE_CLASS_EX) {
 			/* Special hack for uaccess_err */
 			current_thread_info()->uaccess_err = 1;
-			new_ip -= 0x7ffffff0;
 		}
 		regs->ip = new_ip;
 		return 1;

@@ -53,18 +61,15 @@ int fixup_exception(struct pt_regs *regs)
 int __init early_fixup_exception(unsigned long *ip)
 {
 	const struct exception_table_entry *fixup;
-	unsigned long new_ip;
 
 	fixup = search_exception_tables(*ip);
 	if (fixup) {
-		new_ip = ex_fixup_addr(fixup);
-
-		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
+		if (ex_class(fixup) == _EXTABLE_CLASS_EX) {
 			/* uaccess handling not supported during early boot */
 			return 0;
 		}
 
-		*ip = new_ip;
+		*ip = ex_fixup_addr(fixup);
 		return 1;
 	}
 

2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Crivens! I messed up when "git cherrypick"ing this and "git format-patch"ing it.

I didn't mean to forge Andy's From line when sending this out (just to have a
From: line to give him credit.for the patch).

Big OOPs ... this is "From:" me ... not Andy!

-Tony

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

But in any case it's missing your SOB line.

If Andy is still the primary author (much of his original patch survived, you resolved conflicts or minor changes) then you can send this as:

From: Tony Luck <[email protected]>
Subject: [PATCH v6 1/4] x86: Clean up extable entry format (and free up a bit)

From: Andy Lutomirski <[email protected]>

... changelog ...

This carries all the information, has a proper SOB chain, and preserves authorship. Also, it's clear from the tags that you made material changes, so any resulting breakage is yours (and mine), not Andy's! ;-)

If the changes to the patch are major, so that your new work is larger than Andy's original work, you can still credit him via a non-standard tag, like:

From: Tony Luck <[email protected]>
Subject: [PATCH v6 1/4] x86: Clean up extable entry format (and free up a bit)

This patch is based on Andy Lutomirski's patch sent against v3.9:

... changelog ...

Originally-from: Andy Lutomirski <[email protected]>

Thanks,

Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I think so. The .fixup section is placed in the end of .text, and the ex_table itself is pretty much right after. So all the "fixup" offsets will be small negative
numbers (the "insn" ones are also negative, but will be bigger since they potentially need to reach all the way to the start of .text).

Adding 0x20000000 makes everything positive (so our legacy exception table entries have bit31==bit30==0) and perhaps makes it fractionally clearer how we manipulate the top bits for the other classes ... but only slightly. I got
very confused by it too).

It is all made more complex because these values need to be something that "ld" can relocate when vmlinux is put together from all the ".o" files. So we can't just use "x | BIT(30)" etc.

Not to the assembler :-(

Andy - this part is your code and I'm not sure what the trick is here.

We added it to begin with ... need to subtract to get back to the original offset.

ex_class (after part2) is just "(u32)x->fixup >> 30" (because I wanted a result in [0..3])

Because relocations ... the linker can only add/subtract values when making vmlinux ... it can't OR bits in.

-Tony

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

All of that's correct, including the part where it's confusing. The comments aren't the best.

How about adding a comment like:

----- begin comment -----

The offset to the fixup is signed, and we're trying to use the high bits for a different purpose. In C, we could just do:

u32 class_and_offset = ((target - here) & 0x3fffffff) | class;

Then, to decode it, we'd mask off the class and sign-extend to recover the offset.

In asm, we can't do that, because this all gets laundered through the linker, and there's no relocation type that supports this chicanery. Instead we cheat a bit. We first add a large number to the offset (0x20000000). The result is still nominally signed, but now it's always positive, and the two high bits are always clear. We can then set high bits by ordinary addition or subtraction instead of using bitwise operations. As far as the linker is concerned, all we're doing is adding a large constant to the difference between here (".") and the target, and that's a valid relocation type.

In the C code, we just mask off the class bits and subtract 0x20000000 to get the offset.

----- end comment -----

I don't remember. I think it was just some preprocessor crud to force all the macros to expand fully before the assembler sees it. If it builds without it, feel free to delete it.

Hopefully it's clearer with the comment above.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

But presumably those constants get folded together, so the linker is dealing with only one offset. It doesn't (I assume) know that our source code added 0x20000000 and then added/subtracted some more.

It looks like we could just use:
class0: +0x40000000
class1: +0x80000000 (or subtract ... whatever doesn't make the linker cranky)
class2: -0x40000000
class3: don't add/subtract anything

ex_class() stays the same (just looks at bit31/bit30) ex_fixup_addr() has to use ex_class() to decide what to add/subtract (if anything).

Would that work? Would it be more or less confusing?

-Tony

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yes, indeed.

That probably works, but to me, at least, it's a bit more confusing. It also means that you need a table or some branches to compute the offset, whereas the "mask top two bits and add a constant" approach is straightforward, short, and fast.

Also, I'm not 100% convinced that the 0x80000000 case can ever work reliably. I don't know exactly what the condition that triggers the warning is, but the logical one would be to warn if the actual offset plus or minus the addend, as appropriate, overflows in a signed sense. Whether it overflows depends on the sign of the offset, and *that* depends on the actual layout of all the sections.

Mine avoids this issue by being shifted by 0x20000000, so nothing ends up right on the edge.

--Andy

Andy Lutomirski
AMA Capital Management, LLC

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yeah, that makes more sense, thanks.

That nasty "." current position thing stays in the way to do it cleanly. :-)

Anyway, ok, I see it now. It still feels a bit hacky to me. I probably would've added the third int to the exception table instead. It would've been much more straightforward and clean this way and I'd gladly pay the additional 6K growth.

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Josh will argue with you if he sees that :)

We could maybe come up with a way to compress the table and get that space and more back, but maybe that should be a follow-up that someone else can do if they're inspired.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Except Josh doesn't need allyesconfigs. tinyconfig's __ex_table is 2K.

Yeah, one needs to look after ones' TODO list. :-)

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Besides I just saved him 1.5K:

https://lkml.kernel.org/r/[email protected]

:-)

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

If we do the make-it-bigger approach, we get a really nice simplification. Screw the whole 'class' idea -- just store an offset to a handler.

bool extable_handler_default(struct pt_regs *regs, unsigned int fault, unsigned long error_code, unsigned long info)

{
    if (fault == X86_TRAP_MC)
        return false;

    ...
}

bool extable_handler_mc_copy(struct pt_regs *regs, unsigned int fault, unsigned long error_code, unsigned long info);
bool extable_handler_getput_ex(struct pt_regs *regs, unsigned int fault, unsigned long error_code, unsigned long info);

and then shove ".long extable_handler_whatever - ." into the extable entry.

Major bonus points to whoever can figure out how to make extable_handler_iret work -- the current implementation of that is a real turd. (Hint: it's not clear to me that it's even possible without preserving at least part of the asm special case.)

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

And to make it even cooler and more generic, you can make the exception table entry look like this:

{ <offset to fault address>, <offset to handler>, <offset to an opaque pointer> }

and that opaque pointer would be a void * to some struct we pass to that handler and filled with stuff it needs. For starters, it would contain the return address where the fixup wants us to go.

The struct will have to be statically allocated but ok, that's fine.

And this way you can do all the sophisticated handling you desire.

What's extable_handler_iret? IRET-ting from an exception? Where do we do that?

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The trick is definitely needed in the case of

# define _EXPAND_EXTABLE_BIAS(x) #x

Trying to expand it inline and get rid of the macro led to horrible failure. The __ASSEMBLY__ case where the macro does nothing isn't required ... but does provide a certain amount of symmetry when looking at the two versions of _ASM_EXTABLE_CLASS

-Tony

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Make per-class functions for exception table fixup. Add #defines and general prettiness to make it clear how all the parts tie together.

Add a new class that fills %rax with the fault number of the exception.

---
 arch/x86/include/asm/asm.h     | 24 ++++++++++-----
 arch/x86/include/asm/uaccess.h | 17 ++++++++---
 arch/x86/kernel/kprobes/core.c |  2 +-
 arch/x86/kernel/traps.c        |  6 ++--
 arch/x86/mm/extable.c          | 67 +++++++++++++++++++++++++++---------------
 arch/x86/mm/fault.c            |  2 +-
 6 files changed, 79 insertions(+), 39 deletions(-)

index b64121ffb2da..1888278d0559 100644
@@ -44,6 +44,7 @@
 
 /* Exception table entry */
 
+#define	_EXTABLE_BIAS	0x20000000
 /*
  * An exception table entry is 64 bits.  The first 32 bits are the offset
  * from that entry to the potentially faulting instruction.  sortextable

@@ -54,26 +55,35 @@
  * address.  All of these are generated by relocations, so we can only
  * rely on addition.  We therefore emit:
  *
- * (target - here) + (class) + 0x20000000
+ * (target - here) + (class) + _EXTABLE_BIAS
  *
  * This has the property that the two high bits are the class and the
  * rest is easy to decode.
  */
 
-/* There are two bits of extable entry class, added to a signed offset. */
-#define _EXTABLE_CLASS_DEFAULT	0		/* standard uaccess fixup */
-#define _EXTABLE_CLASS_EX	0x80000000	/* uaccess + set uaccess_err */
+/*
+ * There are two bits of extable entry class giving four classes
+ */
+#define EXTABLE_CLASS_DEFAULT	0	/* standard uaccess fixup */
+#define EXTABLE_CLASS_FAULT	1	/* provide fault number as well as fixup */
+#define EXTABLE_CLASS_EX	2	/* uaccess + set uaccess_err */
+#define EXTABLE_CLASS_UNUSED	3	/* available for something else */
 
 /*
- * The biases are the class constants + 0x20000000, as signed integers.
+ * The biases are the class constants + _EXTABLE_BIAS, as signed integers.
  * This can't use ordinary arithmetic -- the assembler isn't that smart.
  */
-#define _EXTABLE_BIAS_DEFAULT	0x20000000
-#define _EXTABLE_BIAS_EX	0x20000000 - 0x80000000
+#define _EXTABLE_BIAS_DEFAULT	_EXTABLE_BIAS
+#define _EXTABLE_BIAS_FAULT	_EXTABLE_BIAS + 0x40000000
+#define _EXTABLE_BIAS_EX	_EXTABLE_BIAS - 0x80000000
+#define _EXTABLE_BIAS_UNUSED	_EXTABLE_BIAS - 0x40000000
 
 #define _ASM_EXTABLE(from,to)						\
 	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_DEFAULT)
 
+#define _ASM_EXTABLE_FAULT(from,to)					\
+	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_FAULT)
+
 #define _ASM_EXTABLE_EX(from,to)					\
 	_ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_EX)
 

index a8df874f3e88..b023300cd6f0 100644
@@ -93,9 +93,12 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  * The exception table consists of pairs of addresses relative to the
  * exception table enty itself: the first is the address of an
  * instruction that is allowed to fault, and the second is the address
- * at which the program should continue.  No registers are modified,
- * so it is entirely up to the continuation code to figure out what to
- * do.
+ * at which the program should continue.  The exception table is linked
+ * soon after the fixup section, so we don't need a full 32-bit offset
+ * for the fixup. We steal the two upper bits so we can tag exception
+ * table entries with different classes. In the default class no registers
+ * are modified, so it is entirely up to the continuation code to figure
+ * out what to do.
  *
  * All the routines below use bits of fixup code that are out of line
  * with the main instruction path.  This means when everything is well,

@@ -110,7 +113,13 @@ struct exception_table_entry {
 #define ARCH_HAS_SORT_EXTABLE
 #define ARCH_HAS_SEARCH_EXTABLE
 
-extern int fixup_exception(struct pt_regs *regs);
+static inline unsigned int
+ex_class(const struct exception_table_entry *x)
+{
+	return (u32)x->fixup >> 30;
+}
+
+extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern int early_fixup_exception(unsigned long *ip);
 
 /*

index 1deffe6cc873..0f05deeff5ce 100644
@@ -988,7 +988,7 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * In case the user-specified fault handler returned
 		 * zero, try to fix up.
 		 */
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, trapnr))
 			return 1;
 
 		/*

index 346eec73f7db..df25081e5970 100644
@@ -199,7 +199,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
 	}
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, trapnr)) {
 			tsk->thread.error_code = error_code;
 			tsk->thread.trap_nr = trapnr;
 			die(str, regs, error_code);

@@ -453,7 +453,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
 	tsk = current;
 	if (!user_mode(regs)) {
-		if (fixup_exception(regs))
+		if (fixup_exception(regs, X86_TRAP_GP))
 			return;
 
 		tsk->thread.error_code = error_code;

@@ -699,7 +699,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	conditional_sti(regs);
 
 	if (!user_mode(regs)) {
-		if (!fixup_exception(regs)) {
+		if (!fixup_exception(regs, X86_TRAP_DE)) {
 			task->thread.error_code = error_code;
 			task->thread.trap_nr = trapnr;
 			die(str, regs, error_code);

index eef44d9a3f77..495946c3f9dd 100644
@@ -656,7 +656,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	int sig;
 
 	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs)) {
+	if (fixup_exception(regs, X86_TRAP_PF)) {
 		/*
 		 * Any interrupt that takes a fault gets the fixup. This makes
 		 * the below recursive fault logic only apply to a faults from

2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

PiBTbyB5b3UncmUgdG91Y2hpbmcgdGhvc2UgYWdhaW4gaW4gcGF0Y2ggMi4gV2h5IG5vdCBhZGQg dGhvc2UgZGVmaW5lcyB0bw0KPiBwYXRjaCAxIGRpcmVjdGx5IGFuZCBkaW1pbmlzaCB0aGUgY2h1 cm4/DQoNClRvIHByZXNlcnZlIGF1dGhvcnNoaXAuIEFuZHkgZGlkIHBhdGNoIDEgKHRoZSBjbGV2 ZXIgcGFydCkuIFBhdGNoIDIgaXMganVzdCBzeW50YWN0aWMNCnN1Z2FyIG9uIHRvcCBvZiBpdC4N Cg0KLVRvbnkNCg==

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

That you can do in the way Ingo suggested.

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

I also personally don't care that much. You're welcome to modify my patch.

If you modify it so much that it's mostly your patch, then change the
From: string and credit me. If not, leave the author as me and make a note in the log message.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Extend the severity checking code to add a new context IN_KERN_RECOV which is used to indicate that the machine check was triggered by code in the kernel with a EXTABLE_CLASS_FAULT fixup entry.

Major re-work to the tail code in do_machine_check() to make all this readable/maintainable. One functional change is that tolerant=3 no longer stops recovery actions. Revert to only skipping sending SIGBUS to the current process.

---
 arch/x86/kernel/cpu/mcheck/mce-severity.c | 32 +++++++++++++-
 arch/x86/kernel/cpu/mcheck/mce.c          | 71 ++++++++++++++++---------------
 2 files changed, 67 insertions(+), 36 deletions(-)

index 9c682c222071..b6eee8e9b2a6 100644
@@ -13,7 +13,9 @@
 #include <linux/seq_file.h>
 #include <linux/init.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <asm/mce.h>
+#include <asm/uaccess.h>
 
 #include "mce-internal.h"
 

@@ -29,7 +31,7 @@
  * panic situations)
  */
 
-enum context { IN_KERNEL = 1, IN_USER = 2 };
+enum context { IN_KERNEL = 1, IN_USER = 2, IN_KERNEL_RECOV = 3 };
 enum ser { SER_REQUIRED = 1, NO_SER = 2 };
 enum exception { EXCP_CONTEXT = 1, NO_EXCP = 2 };
 

@@ -48,6 +50,7 @@ static struct severity {
 #define MCESEV(s, m, c...) { .sev = MCE_ ## s ## _SEVERITY, .msg = m, ## c }
 #define  KERNEL		.context = IN_KERNEL
 #define  USER		.context = IN_USER
+#define  KERNEL_RECOV	.context = IN_KERNEL_RECOV
 #define  SER		.ser = SER_REQUIRED
 #define  NOSER		.ser = NO_SER
 #define  EXCP		.excp = EXCP_CONTEXT

@@ -87,6 +90,10 @@ static struct severity {
 		EXCP, KERNEL, MCGMASK(MCG_STATUS_RIPV, 0)
 		),
 	MCESEV(
+		PANIC, "In kernel and no restart IP",
+		EXCP, KERNEL_RECOV, MCGMASK(MCG_STATUS_RIPV, 0)
+		),
+	MCESEV(
 		DEFERRED, "Deferred error",
 		NOSER, MASK(MCI_STATUS_UC|MCI_STATUS_DEFERRED|MCI_STATUS_POISON, MCI_STATUS_DEFERRED)
 		),

@@ -123,6 +130,11 @@ static struct severity {
 		MCGMASK(MCG_STATUS_RIPV|MCG_STATUS_EIPV, MCG_STATUS_RIPV)
 		),
 	MCESEV(
+		AR, "Action required: data load in error recoverable area of kernel",
+		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),
+		KERNEL_RECOV
+		),
+	MCESEV(
 		AR, "Action required: data load error in a user process",
 		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),
 		USER

@@ -170,6 +182,18 @@ static struct severity {
 		)	/* always matches. keep at end */
 };
 
+#define mc_recoverable(mcg) (((mcg) & (MCG_STATUS_RIPV|MCG_STATUS_EIPV)) == \
+				(MCG_STATUS_RIPV|MCG_STATUS_EIPV))
+
+static inline bool mce_in_kernel_recov(unsigned long addr)
+{
+	const struct exception_table_entry *fixup;
+
+	fixup = search_exception_tables(addr);
+
+	return fixup && ex_class(fixup) == EXTABLE_CLASS_FAULT;
+}
+
 /*
  * If mcgstatus indicated that ip/cs on the stack were
  * no good, then "m->cs" will be zero and we will have

@@ -183,7 +207,11 @@ static struct severity {
  */
 static int error_context(struct mce *m)
 {
-	return ((m->cs & 3) == 3) ? IN_USER : IN_KERNEL;
+	if ((m->cs & 3) == 3)
+		return IN_USER;
+	if (mc_recoverable(m->mcgstatus) && mce_in_kernel_recov(m->ip))
+		return IN_KERNEL_RECOV;
+	return IN_KERNEL;
 }
 
 /*

2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Make use of the EXTABLE_FAULT exception table entries. This routine returns a structure to indicate the result of the copy:

struct mcsafe_ret {
	u64 trapnr;
	u64 remain;
};

If the copy is successful, then both 'trapnr' and 'remain' are zero.

If we faulted during the copy, then 'trapnr' will say which type of trap (X86_TRAP_PF or X86_TRAP_MC) and 'remain' says how many bytes were not copied.

---
 arch/x86/Kconfig                 |  10 +++
 arch/x86/include/asm/string_64.h |  10 +++
 arch/x86/kernel/x8664_ksyms_64.c |   4 ++
 arch/x86/lib/memcpy_64.S         | 136 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 160 insertions(+)

index 96d058a87100..42d26b4d1ec4 100644
@@ -1001,6 +1001,16 @@ config X86_MCE_INJECT
 	  If you don't know what a machine check is and you don't do kernel
 	  QA it is safe to say n.
 
+config MCE_KERNEL_RECOVERY
+	bool "Recovery from machine checks in special kernel memory copy functions"
+	default n
+	depends on X86_MCE && X86_64
+	---help---
+	  This option provides a new memory copy function mcsafe_memcpy()
+	  that is annotated to allow the machine check handler to return
+	  to an alternate code path to return an error to the caller instead
+	  of crashing the system. Say yes if you have a driver that uses this.
+
 config X86_THERMAL_VECTOR
 	def_bool y
 	depends on X86_MCE_INTEL

index ff8b9a17dc4b..3887f304d8cd 100644
@@ -78,6 +78,16 @@ int strcmp(const char *cs, const char *ct);
 #define memset(s, c, n) __memset(s, c, n)
 #endif
 
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+struct mcsafe_ret {
+	u64 trapnr;
+	u64 remain;
+};
+
+struct mcsafe_ret __mcsafe_copy(void *dst, const void __user *src, unsigned size);
+extern void __mcsafe_copy_end(void);
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_STRING_64_H */

index a0695be19864..3d42d0ef3333 100644
@@ -37,6 +37,10 @@ EXPORT_SYMBOL(__copy_user_nocache);
 EXPORT_SYMBOL(_copy_from_user);
 EXPORT_SYMBOL(_copy_to_user);
 
+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+EXPORT_SYMBOL(__mcsafe_copy);
+#endif
+
 EXPORT_SYMBOL(copy_page);
 EXPORT_SYMBOL(clear_page);
 

2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>