Machine check recovery when kernel accesses poison

Copy the existing page fault fixup mechanisms to create a new table to be used when fixing machine checks. Note:
1) At this time we only provide a macro to annotate assembly code
2) We assume all fixups will in code builtin to the kernel.
3) Only for x86_64
4) New code under CONFIG_MCE_KERNEL_RECOVERY (default 'n')

Signed-off-by Tony Luck <[email protected]>

Looks generally good.

Reviewed-by Andy Lutomirski <[email protected]>

Quoted 5 lines

arch/x86/mm/extable.c +5 -0

+#ifdef CONFIG_MCE_KERNEL_RECOVERY
+int fixup_mcexception(struct pt_regs *regs)
+{
+       const struct exception_table_entry *fixup;
+       unsigned long new_ip;

arch/x86/mm/extable.c int fixup_exception(struct pt_regs *regs) · line 63

Quoted 6 lines

arch/x86/mm/extable.c +6 -0

+
+       fixup = search_mcexception_tables(regs->ip);
+       if (fixup) {
+               new_ip = ex_fixup_addr(fixup);
+
+               regs->ip = new_ip;

You could very easily save a line of code here :)

Quoted 3 lines

Looks generally good.

Reviewed-by Andy Lutomirski <[email protected]>

You say that to part 1/3 ... what happens when you get to part 3/3 and you read my attempts at writing x86 assembly code?

Copy the existing page fault fixup mechanisms to create a new table to be used when fixing machine checks. Note:
1) At this time we only provide a macro to annotate assembly code
2) We assume all fixups will in code builtin to the kernel.
3) Only for x86_64
4) New code under CONFIG_MCE_KERNEL_RECOVERY (default 'n')

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/Kconfig                  | 10 ++++++++++
 arch/x86/include/asm/asm.h        | 10 ++++++++--
 arch/x86/include/asm/mce.h        | 14 ++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c  | 16 ++++++++++++++++
 arch/x86/kernel/vmlinux.lds.S     |  6 +++++-
 arch/x86/mm/extable.c             | 19 +++++++++++++++++++
 include/asm-generic/vmlinux.lds.h | 12 +++++++-----
 7 files changed, 79 insertions(+), 8 deletions(-)

arch/x86/Kconfig +0 -0

index 96d058a87100..42d26b4d1ec4 100644

arch/x86/Kconfig +6 -0

+	---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.
+

Quoted 26 lines · Tony Luck

Copy the existing page fault fixup mechanisms to create a new table to be used when fixing machine checks. Note:
1) At this time we only provide a macro to annotate assembly code
2) We assume all fixups will in code builtin to the kernel.
3) Only for x86_64
4) New code under CONFIG_MCE_KERNEL_RECOVERY (default 'n')

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/Kconfig                  | 10 ++++++++++
 arch/x86/include/asm/asm.h        | 10 ++++++++--
 arch/x86/include/asm/mce.h        | 14 ++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c  | 16 ++++++++++++++++
 arch/x86/kernel/vmlinux.lds.S     |  6 +++++-
 arch/x86/mm/extable.c             | 19 +++++++++++++++++++
 include/asm-generic/vmlinux.lds.h | 12 +++++++-----
 7 files changed, 79 insertions(+), 8 deletions(-)

arch/x86/Kconfig +0 -0

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.
 

arch/x86/Kconfig config X86_MCE_INJECT · line 1007

Quoted 4 lines

arch/x86/Kconfig +4 -0

+config MCE_KERNEL_RECOVERY
+     bool "Recovery from machine checks in special kernel memory copy functions"
+     default n
+     depends on X86_MCE && X86_64

Still no dependency on CONFIG_LIBNVDIMM.

I suggested we reverse the dependency and have the driver optionally "select MCE_KERNEL_RECOVERY". There may be other drivers outside of LIBNVDIMM that want this functionality enabled.

Copy the existing page fault fixup mechanisms to create a new table to be used when fixing machine checks. Note:
1) At this time we only provide a macro to annotate assembly code
2) We assume all fixups will in code builtin to the kernel.
3) Only for x86_64
4) New code under CONFIG_MCE_KERNEL_RECOVERY (default 'n')

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/Kconfig                  | 10 ++++++++++
 arch/x86/include/asm/asm.h        | 10 ++++++++--
 arch/x86/include/asm/mce.h        | 14 ++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c  | 16 ++++++++++++++++
 arch/x86/kernel/vmlinux.lds.S     |  6 +++++-
 arch/x86/mm/extable.c             | 19 +++++++++++++++++++
 include/asm-generic/vmlinux.lds.h | 12 +++++++-----
 7 files changed, 79 insertions(+), 8 deletions(-)

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 fixup entry.

Add code to check for this situation and respond by altering the return IP to the fixup address.

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.

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce-severity.c | 21 +++++++++-
 arch/x86/kernel/cpu/mcheck/mce.c          | 70 ++++++++++++++++---------------
 2 files changed, 55 insertions(+), 36 deletions(-)

Using __copy_user_nocache() as inspiration create a memory copy routine for use by kernel code with annotations to allow for recovery from machine checks.

Notes:
1) We align the source address rather than the destination. This means we never have to deal with a memory read that spans two cache lines ... so we can provide a precise indication of where the error occurred without having to re-execute at a byte-by-byte level to find the exact spot like the original did.
2) We 'or' BIT(63) into the return because this is the first in a series of machine check safe functions. Some will copy from user addresses, so may need to indicate an invalid user address instead of a machine check.
3) This code doesn't play any cache games. Future functions can use non-temporal loads/stores to meet needs of different callers.
4) Provide helpful macros to decode the return value.

Signed-off-by Tony Luck <[email protected]>

---
 arch/x86/include/asm/mcsafe_copy.h |  11 +++
 arch/x86/kernel/x8664_ksyms_64.c   |   5 ++
 arch/x86/lib/Makefile              |   1 +
 arch/x86/lib/mcsafe_copy.S         | 142 +++++++++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+)
 create mode 100644 arch/x86/include/asm/mcsafe_copy.h
 create mode 100644 arch/x86/lib/mcsafe_copy.S

arch/x86/include/asm/mcsafe_copy.h new file +0 -0

new file mode 100644
index 000000000000..d4dbd5a667a3

arch/x86/lib/mcsafe_copy.S new file +117 -0

@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2015 Intel Corporation
+ * Author: Tony Luck
+ *
+ * This software may be redistributed and/or modified under the terms of
+ * the GNU General Public License ("GPL") version 2 only as published by the
+ * Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+
+/*
+ * mcsafe_memcpy - memory copy with machine check exception handling
+ * Note that we only catch machine checks when reading the source addresses.
+ * Writes to target are posted and don't generate machine checks.
+ */
+ENTRY(mcsafe_memcpy)
+	cmpl $8,%edx
+	jb 20f		/* less then 8 bytes, go to byte copy loop */
+
+	/* check for bad alignment of source */
+	movl %esi,%ecx
+	andl $7,%ecx
+	jz 102f				/* already aligned */
+	subl $8,%ecx
+	negl %ecx
+	subl %ecx,%edx
+0:	movb (%rsi),%al
+	movb %al,(%rdi)
+	incq %rsi
+	incq %rdi
+	decl %ecx
+	jnz 0b
+102:
+	movl %edx,%ecx
+	andl $63,%edx
+	shrl $6,%ecx
+	jz 17f
+1:	movq (%rsi),%r8
+2:	movq 1*8(%rsi),%r9
+3:	movq 2*8(%rsi),%r10
+4:	movq 3*8(%rsi),%r11
+	mov %r8,(%rdi)
+	mov %r9,1*8(%rdi)
+	mov %r10,2*8(%rdi)
+	mov %r11,3*8(%rdi)
+9:	movq 4*8(%rsi),%r8
+10:	movq 5*8(%rsi),%r9
+11:	movq 6*8(%rsi),%r10
+12:	movq 7*8(%rsi),%r11
+	mov %r8,4*8(%rdi)
+	mov %r9,5*8(%rdi)
+	mov %r10,6*8(%rdi)
+	mov %r11,7*8(%rdi)
+	leaq 64(%rsi),%rsi
+	leaq 64(%rdi),%rdi
+	decl %ecx
+	jnz 1b
+17:	movl %edx,%ecx
+	andl $7,%edx
+	shrl $3,%ecx
+	jz 20f
+18:	movq (%rsi),%r8
+	mov %r8,(%rdi)
+	leaq 8(%rsi),%rsi
+	leaq 8(%rdi),%rdi
+	decl %ecx
+	jnz 18b
+20:	andl %edx,%edx
+	jz 23f
+	movl %edx,%ecx
+21:	movb (%rsi),%al
+	movb %al,(%rdi)
+	incq %rsi
+	incq %rdi
+	decl %ecx
+	jnz 21b
+23:	xorl %eax,%eax
+	sfence
+	ret
+
+	.section .fixup,"ax"
+30:
+	addl %ecx,%edx
+	jmp 100f
+31:
+	shll $6,%ecx
+	addl %ecx,%edx
+	jmp 100f
+32:
+	shll $6,%ecx
+	leal -8(%ecx,%edx),%edx
+	jmp 100f
+33:
+	shll $6,%ecx
+	leal -16(%ecx,%edx),%edx
+	jmp 100f
+34:
+	shll $6,%ecx
+	leal -24(%ecx,%edx),%edx
+	jmp 100f
+35:
+	shll $6,%ecx
+	leal -32(%ecx,%edx),%edx
+	jmp 100f
+36:
+	shll $6,%ecx
+	leal -40(%ecx,%edx),%edx
+	jmp 100f
+37:
+	shll $6,%ecx
+	leal -48(%ecx,%edx),%edx
+	jmp 100f
+38:
+	shll $6,%ecx
+	leal -56(%ecx,%edx),%edx

Quoted 1 line

+#define COPY_MCHECK_ERRBIT BIT(63)

What happened to the landing pads Andy was talking about? They sound like cleaner design than that bit 63...

I interpreted that comment as "stop playing with %rax in the fault handler ... just change the IP to point the the .fixup location" ... the target of the fixup being the "landing pad".

Right now this function has only one set of fault fixups (for machine checks). When I tackle copy_from_user() it will sprout a second set for page faults, and then will look a bit more like Andy's dual landing pad example.

I still need an indicator to the caller which type of fault happened since their actions will be different. So BIT(63) lives on ... but is now set in the .fixup section rather than in the machine check code.

I'll move the function and #defines as you suggest - we don't need new files for these. Also will fix the assembly code. [In my defense that load immediate 0x8000000000000000 and 'or' was what gcc -O2 generates from a simple bit of C code to set bit 63 ... perhaps it is faster, or perhaps gcc is on drugs. In this case code compactness wins over possible speed difference].

Quoted 9 lines · Tony Luck

I interpreted that comment as "stop playing with %rax in the fault handler ... just change the IP to point the the .fixup location" ... the target of the fixup being the "landing pad".

Right now this function has only one set of fault fixups (for machine checks). When I tackle copy_from_user() it will sprout a second set for page faults, and then will look a bit more like Andy's dual landing pad example.

Quoted 4 lines

I still need an indicator to the caller which type of fault happened since their actions will be different. So BIT(63) lives on ... but is now set in the .fixup section rather than in the machine check code.

You mean this previous example of yours:

int copy_from_user(void *to, void *from, unsigned long n)

{
        u64 ret = mcsafe_memcpy(to, from, n);

        if (COPY_HAD_MCHECK(r)) {
                if (memory_failure(COPY_MCHECK_PADDR(ret) >> PAGE_SIZE, ...))
                        force_sig(SIGBUS, current);
                return something;
        } else
                return ret;
}

?

So what's wrong with mcsafe_memcpy() returning a proper retval which says what type of fault happened?

I know, memcpy returns the ptr to @dest like a parrot but your version mcsafe_memcpy() will be different. It can even be called __mcsafe_memcpy and have a wrapper around it which fiddles out the proper retvals and returns @dest after all. It would still be cleaner this way IMHO.

I know, memcpy returns the ptr to @dest like a parrot

Quoted 1 line

I know, memcpy returns the ptr to @dest like a parrot

Maybe I need to change the name to remove the "memcpy" substring to avoid this confusion. How about "mcsafe_copy()"? Perhaps with a "__" prefix to point out it is a building block that will get various wrappers around it??

Dan wants a copy_from_nvdimm() that either completes the copy, or indicates where a machine check occurred.

I'm going to want a copy_from_user() that has two fault options (user gave a bad address -> -EFAULT, or the source address had an uncorrected error -> SIGBUS).