xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception
       [not found] <cover.1459605520.git.luto@kernel.org>
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-13 11:43   ` [tip:x86/asm] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception() tip-bot for Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 2/9] x86/head: Move the early NMI fixup into C Andy Lutomirski
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

early_fixup_exception is limited by the fact that it doesn't have a
real struct pt_regs.  Change both the 32-bit and 64-bit asm and the
C code to pass and accept a real pt_regs.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/uaccess.h |  2 +-
 arch/x86/kernel/head_32.S      | 74 +++++++++++++++++++++++++++++-------------
 arch/x86/kernel/head_64.S      | 68 ++++++++++++++++++++------------------
 arch/x86/mm/extable.c          |  6 ++--
 4 files changed, 92 insertions(+), 58 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index c0f27d7ea7ff..a3afb7259751 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -111,7 +111,7 @@ struct exception_table_entry {
 
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
-extern int early_fixup_exception(unsigned long *ip);
+extern int early_fixup_exception(struct pt_regs *regs, int trapnr);
 
 /*
  * These are the main single-value transfer routines.  They automatically
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 6bc9ae24b6d2..bef3e6c49b56 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -562,29 +562,64 @@ early_idt_handler_common:
 	je hlt_loop
 	incl %ss:early_recursion_flag
 
-	push %eax		# 16(%esp)
-	push %ecx		# 12(%esp)
-	push %edx		#  8(%esp)
-	push %ds		#  4(%esp)
-	push %es		#  0(%esp)
-	movl $(__KERNEL_DS),%eax
-	movl %eax,%ds
-	movl %eax,%es
+	/* The vector number is in pt_regs->gs */
 
-	cmpl $(__KERNEL_CS),32(%esp)
+	cld
+	pushl	%fs		/* pt_regs->fs */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%es		/* pt_regs->es */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%ds		/* pt_regs->ds */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%eax		/* pt_regs->ax */
+	pushl	%ebp		/* pt_regs->bp */
+	pushl	%edi		/* pt_regs->di */
+	pushl	%esi		/* pt_regs->si */
+	pushl	%edx		/* pt_regs->dx */
+	pushl	%ecx		/* pt_regs->cx */
+	pushl	%ebx		/* pt_regs->bx */
+
+	/* Fix up DS and ES */
+	movl	$(__KERNEL_DS), %ecx
+	movl	%ecx, %ds
+	movl	%ecx, %es
+
+	/* Load the vector number into EDX */
+	movl	PT_GS(%esp), %edx
+
+	/* Load GS into pt_regs->gs and clear high bits */
+	movw	%gs, PT_GS(%esp)
+	movw	$0, PT_GS+2(%esp)
+
+	cmpl $(__KERNEL_CS),PT_CS(%esp)
 	jne 10f
 
-	leal 28(%esp),%eax	# Pointer to %eip
-	call early_fixup_exception
-	andl %eax,%eax
-	jnz ex_entry		/* found an exception entry */
+	movl	%esp, %eax	/* args are pt_regs (EAX), trapnr (EDX) */
+	call	early_fixup_exception
+	andl	%eax,%eax
+	jz	10f		/* Exception wasn't fixed up */
+
+	popl	%ebx		/* pt_regs->bx */
+	popl	%ecx		/* pt_regs->cx */
+	popl	%edx		/* pt_regs->dx */
+	popl	%esi		/* pt_regs->si */
+	popl	%edi		/* pt_regs->di */
+	popl	%ebp		/* pt_regs->bp */
+	popl	%eax		/* pt_regs->ax */
+	popl	%ds		/* pt_regs->ds */
+	popl	%es		/* pt_regs->es */
+	popl	%fs		/* pt_regs->fs */
+	popl	%gs		/* pt_regs->gs */
+	decl	%ss:early_recursion_flag
+	addl	$4, %esp	/* pop pt_regs->orig_ax */
+	iret
 
 10:
 #ifdef CONFIG_PRINTK
 	xorl %eax,%eax
-	movw %ax,2(%esp)	/* clean up the segment values on some cpus */
-	movw %ax,6(%esp)
-	movw %ax,34(%esp)
+	movw %ax,PT_FS+2(%esp)	/* clean up the segment values on some cpus */
+	movw %ax,PT_DS+2(%esp)
+	movw %ax,PT_ES+2(%esp)
 	leal  40(%esp),%eax
 	pushl %eax		/* %esp before the exception */
 	pushl %ebx
@@ -602,13 +637,6 @@ hlt_loop:
 	hlt
 	jmp hlt_loop
 
-ex_entry:
-	pop %es
-	pop %ds
-	pop %edx
-	pop %ecx
-	pop %eax
-	decl %ss:early_recursion_flag
 .Lis_nmi:
 	addl $8,%esp		/* drop vector number and error code */
 	iret
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index ffdc0e860390..f8d6dad41e8d 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -20,6 +20,7 @@
 #include <asm/processor-flags.h>
 #include <asm/percpu.h>
 #include <asm/nops.h>
+#include "../entry/calling.h"
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>
@@ -360,39 +361,52 @@ early_idt_handler_common:
 	jz  1f
 	incl early_recursion_flag(%rip)
 
-	pushq %rax		# 64(%rsp)
-	pushq %rcx		# 56(%rsp)
-	pushq %rdx		# 48(%rsp)
-	pushq %rsi		# 40(%rsp)
-	pushq %rdi		# 32(%rsp)
-	pushq %r8		# 24(%rsp)
-	pushq %r9		# 16(%rsp)
-	pushq %r10		#  8(%rsp)
-	pushq %r11		#  0(%rsp)
-
-	cmpl $__KERNEL_CS,96(%rsp)
+	/* The vector number is currently in the pt_regs->di slot. */
+	pushq %rsi				/* pt_regs->si */
+	movq 8(%rsp), %rsi			/* RSI = vector number */
+	movq %rdi, 8(%rsp)			/* pt_regs->di = RDI */
+	pushq %rdx				/* pt_regs->dx */
+	pushq %rcx				/* pt_regs->cx */
+	pushq %rax				/* pt_regs->ax */
+	pushq %r8				/* pt_regs->r8 */
+	pushq %r9				/* pt_regs->r9 */
+	pushq %r10				/* pt_regs->r10 */
+	pushq %r11				/* pt_regs->r11 */
+	pushq %rbx				/* pt_regs->bx */
+	pushq %rbp				/* pt_regs->bp */
+	pushq %r12				/* pt_regs->r12 */
+	pushq %r13				/* pt_regs->r13 */
+	pushq %r14				/* pt_regs->r14 */
+	pushq %r15				/* pt_regs->r15 */
+
+	cmpl $__KERNEL_CS,CS(%rsp)
 	jne 11f
 
-	cmpl $14,72(%rsp)	# Page fault?
+	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f
-	GET_CR2_INTO(%rdi)	# can clobber any volatile register if pv
+	GET_CR2_INTO(%rdi)	/* Can clobber any volatile register if pv */
 	call early_make_pgtable
 	andl %eax,%eax
-	jz 20f			# All good
+	jz 20f			/* All good */
 
 10:
-	leaq 88(%rsp),%rdi	# Pointer to %rip
+	movq %rsp,%rdi		/* RDI = pt_regs; RSI is already trapnr */
 	call early_fixup_exception
 	andl %eax,%eax
 	jnz 20f			# Found an exception entry
 
 11:
 #ifdef CONFIG_EARLY_PRINTK
-	GET_CR2_INTO(%r9)	# can clobber any volatile register if pv
-	movl 80(%rsp),%r8d	# error code
-	movl 72(%rsp),%esi	# vector number
-	movl 96(%rsp),%edx	# %cs
-	movq 88(%rsp),%rcx	# %rip
+	/*
+	 * On paravirt kernels, GET_CR2_INTO clobbers callee-clobbered regs.
+	 * We only care about RSI, so we need to save it.
+	 */
+	movq %rsi,%rbx		/* Save vector number */
+	GET_CR2_INTO(%r9)
+	movq ORIG_RAX(%rsp),%r8	/* error code */
+	movq %rbx,%rsi		/* vector number */
+	movq CS(%rsp),%rdx
+	movq RIP(%rsp),%rcx
 	xorl %eax,%eax
 	leaq early_idt_msg(%rip),%rdi
 	call early_printk
@@ -401,24 +415,16 @@ early_idt_handler_common:
 	call dump_stack
 #ifdef CONFIG_KALLSYMS	
 	leaq early_idt_ripmsg(%rip),%rdi
-	movq 40(%rsp),%rsi	# %rip again
+	movq RIP(%rsp),%rsi	# %rip again
 	call __print_symbol
 #endif
 #endif /* EARLY_PRINTK */
 1:	hlt
 	jmp 1b
 
-20:	# Exception table entry found or page table generated
-	popq %r11
-	popq %r10
-	popq %r9
-	popq %r8
-	popq %rdi
-	popq %rsi
-	popq %rdx
-	popq %rcx
-	popq %rax
+20:	/* Exception table entry found or page table generated */
 	decl early_recursion_flag(%rip)
+	jmp restore_regs_and_iret
 .Lis_nmi:
 	addq $16,%rsp		# drop vector number and error code
 	INTERRUPT_RETURN
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 9dd7e4b7fcde..176e48de25d4 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -90,13 +90,13 @@ int fixup_exception(struct pt_regs *regs, int trapnr)
 }
 
 /* Restricted version used during very early boot */
-int __init early_fixup_exception(unsigned long *ip)
+int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *e;
 	unsigned long new_ip;
 	ex_handler_t handler;
 
-	e = search_exception_tables(*ip);
+	e = search_exception_tables(regs->ip);
 	if (!e)
 		return 0;
 
@@ -107,7 +107,7 @@ int __init early_fixup_exception(unsigned long *ip)
 	if (handler != ex_handler_default)
 		return 0;
 
-	*ip = new_ip;
+	regs->ip = new_ip;
 	return 1;
 }
 
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 2/9] x86/head: Move the early NMI fixup into C
       [not found] <cover.1459605520.git.luto@kernel.org>
  2016-04-02 14:01 ` [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

C is nicer than asm.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/head_32.S | 7 -------
 arch/x86/kernel/head_64.S | 6 ------
 arch/x86/mm/extable.c     | 5 +++++
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index bef3e6c49b56..5e6ce845813a 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -555,9 +555,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%esp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,%ss:early_recursion_flag
 	je hlt_loop
 	incl %ss:early_recursion_flag
@@ -636,10 +633,6 @@ early_idt_handler_common:
 hlt_loop:
 	hlt
 	jmp hlt_loop
-
-.Lis_nmi:
-	addl $8,%esp		/* drop vector number and error code */
-	iret
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index f8d6dad41e8d..af87896b6a23 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -354,9 +354,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%rsp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,early_recursion_flag(%rip)
 	jz  1f
 	incl early_recursion_flag(%rip)
@@ -425,9 +422,6 @@ early_idt_handler_common:
 20:	/* Exception table entry found or page table generated */
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
-.Lis_nmi:
-	addq $16,%rsp		# drop vector number and error code
-	INTERRUPT_RETURN
 ENDPROC(early_idt_handler_common)
 
 	__INITDATA
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 176e48de25d4..d6e4e6fb4002 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -2,6 +2,7 @@
 #include <linux/spinlock.h>
 #include <linux/sort.h>
 #include <asm/uaccess.h>
+#include <asm/traps.h>
 
 typedef bool (*ex_handler_t)(const struct exception_table_entry *,
 			    struct pt_regs *, int);
@@ -96,6 +97,10 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 	unsigned long new_ip;
 	ex_handler_t handler;
 
+	/* Ignore early NMIs. */
+	if (trapnr == X86_TRAP_NMI)
+		return 1;
+
 	e = search_exception_tables(regs->ip);
 	if (!e)
 		return 0;
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found] <cover.1459605520.git.luto@kernel.org>
  2016-04-02 14:01 ` [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 2/9] x86/head: Move the early NMI fixup into C Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 18:39   ` Borislav Petkov
                     ` (2 more replies)
  2016-04-02 14:01 ` [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early Andy Lutomirski
                   ` (11 subsequent siblings)
  14 siblings, 3 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

This removes a bunch of assembly and adds some C code instead.  It
changes the actual printouts on both 32-bit and 64-bit kernels, but
they still seem okay.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/uaccess.h |  2 +-
 arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
 arch/x86/kernel/head_64.S      | 45 ++------------------------------------
 arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
 4 files changed, 32 insertions(+), 93 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a3afb7259751..83fd2cf187d2 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -111,7 +111,7 @@ struct exception_table_entry {
 
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
-extern int early_fixup_exception(struct pt_regs *regs, int trapnr);
+extern void early_fixup_exception(struct pt_regs *regs, int trapnr);
 
 /*
  * These are the main single-value transfer routines.  They automatically
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 5e6ce845813a..411dce93fee9 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -555,8 +555,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,%ss:early_recursion_flag
-	je hlt_loop
 	incl %ss:early_recursion_flag
 
 	/* The vector number is in pt_regs->gs */
@@ -588,13 +586,8 @@ early_idt_handler_common:
 	movw	%gs, PT_GS(%esp)
 	movw	$0, PT_GS+2(%esp)
 
-	cmpl $(__KERNEL_CS),PT_CS(%esp)
-	jne 10f
-
 	movl	%esp, %eax	/* args are pt_regs (EAX), trapnr (EDX) */
 	call	early_fixup_exception
-	andl	%eax,%eax
-	jz	10f		/* Exception wasn't fixed up */
 
 	popl	%ebx		/* pt_regs->bx */
 	popl	%ecx		/* pt_regs->cx */
@@ -610,29 +603,6 @@ early_idt_handler_common:
 	decl	%ss:early_recursion_flag
 	addl	$4, %esp	/* pop pt_regs->orig_ax */
 	iret
-
-10:
-#ifdef CONFIG_PRINTK
-	xorl %eax,%eax
-	movw %ax,PT_FS+2(%esp)	/* clean up the segment values on some cpus */
-	movw %ax,PT_DS+2(%esp)
-	movw %ax,PT_ES+2(%esp)
-	leal  40(%esp),%eax
-	pushl %eax		/* %esp before the exception */
-	pushl %ebx
-	pushl %ebp
-	pushl %esi
-	pushl %edi
-	movl %cr2,%eax
-	pushl %eax
-	pushl (20+6*4)(%esp)	/* trapno */
-	pushl $fault_msg
-	call printk
-#endif
-	call dump_stack
-hlt_loop:
-	hlt
-	jmp hlt_loop
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
@@ -668,10 +638,14 @@ ignore_int:
 	popl %eax
 #endif
 	iret
+
+hlt_loop:
+	hlt
+	jmp hlt_loop
 ENDPROC(ignore_int)
 __INITDATA
 	.align 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
 __REFDATA
@@ -736,19 +710,6 @@ __INITRODATA
 int_msg:
 	.asciz "Unknown interrupt or fault at: %p %p %p\n"
 
-fault_msg:
-/* fault info: */
-	.ascii "BUG: Int %d: CR2 %p\n"
-/* regs pushed in early_idt_handler: */
-	.ascii "     EDI %p  ESI %p  EBP %p  EBX %p\n"
-	.ascii "     ESP %p   ES %p   DS %p\n"
-	.ascii "     EDX %p  ECX %p  EAX %p\n"
-/* fault frame: */
-	.ascii "     vec %p  err %p  EIP %p   CS %p  flg %p\n"
-	.ascii "Stack: %p %p %p %p %p %p %p %p\n"
-	.ascii "       %p %p %p %p %p %p %p %p\n"
-	.asciz "       %p %p %p %p %p %p %p %p\n"
-
 #include "../../x86/xen/xen-head.S"
 
 /*
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index af87896b6a23..c39b6437cf03 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -354,8 +354,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
 	incl early_recursion_flag(%rip)
 
 	/* The vector number is currently in the pt_regs->di slot. */
@@ -376,9 +374,6 @@ early_idt_handler_common:
 	pushq %r14				/* pt_regs->r14 */
 	pushq %r15				/* pt_regs->r15 */
 
-	cmpl $__KERNEL_CS,CS(%rsp)
-	jne 11f
-
 	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f
 	GET_CR2_INTO(%rdi)	/* Can clobber any volatile register if pv */
@@ -389,37 +384,8 @@ early_idt_handler_common:
 10:
 	movq %rsp,%rdi		/* RDI = pt_regs; RSI is already trapnr */
 	call early_fixup_exception
-	andl %eax,%eax
-	jnz 20f			# Found an exception entry
-
-11:
-#ifdef CONFIG_EARLY_PRINTK
-	/*
-	 * On paravirt kernels, GET_CR2_INTO clobbers callee-clobbered regs.
-	 * We only care about RSI, so we need to save it.
-	 */
-	movq %rsi,%rbx		/* Save vector number */
-	GET_CR2_INTO(%r9)
-	movq ORIG_RAX(%rsp),%r8	/* error code */
-	movq %rbx,%rsi		/* vector number */
-	movq CS(%rsp),%rdx
-	movq RIP(%rsp),%rcx
-	xorl %eax,%eax
-	leaq early_idt_msg(%rip),%rdi
-	call early_printk
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
-	call dump_stack
-#ifdef CONFIG_KALLSYMS	
-	leaq early_idt_ripmsg(%rip),%rdi
-	movq RIP(%rsp),%rsi	# %rip again
-	call __print_symbol
-#endif
-#endif /* EARLY_PRINTK */
-1:	hlt
-	jmp 1b
 
-20:	/* Exception table entry found or page table generated */
+20:
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
 ENDPROC(early_idt_handler_common)
@@ -427,16 +393,9 @@ ENDPROC(early_idt_handler_common)
 	__INITDATA
 
 	.balign 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
-#ifdef CONFIG_EARLY_PRINTK
-early_idt_msg:
-	.asciz "PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"
-early_idt_ripmsg:
-	.asciz "RIP %s\n"
-#endif /* CONFIG_EARLY_PRINTK */
-
 #define NEXT_PAGE(name) \
 	.balign	PAGE_SIZE; \
 GLOBAL(name)
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index d6e4e6fb4002..8997022abebc 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -90,8 +90,10 @@ int fixup_exception(struct pt_regs *regs, int trapnr)
 	return handler(e, regs, trapnr);
 }
 
+extern unsigned int early_recursion_flag;
+
 /* Restricted version used during very early boot */
-int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
+void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *e;
 	unsigned long new_ip;
@@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 
 	/* Ignore early NMIs. */
 	if (trapnr == X86_TRAP_NMI)
-		return 1;
+		return;
+
+	if (early_recursion_flag > 2)
+		goto halt_loop;
+
+	if (regs->cs != __KERNEL_CS)
+		goto fail;
 
 	e = search_exception_tables(regs->ip);
 	if (!e)
-		return 0;
+		goto fail;
 
 	new_ip  = ex_fixup_addr(e);
 	handler = ex_fixup_handler(e);
 
 	/* special handling not supported during early boot */
 	if (handler != ex_handler_default)
-		return 0;
+		goto fail;
 
 	regs->ip = new_ip;
-	return 1;
+	return;
+
+fail:
+	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
+		     (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
+		     regs->orig_ax, read_cr2());
+
+	show_regs(regs);
+
+halt_loop:
+	while (true)
+		halt();
 }
 
 /*
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (2 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

Now that early_fixup_exception has pt_regs, we can just call
fixup_exception from it.  This will make fancy exception handlers
work early.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/mm/extable.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 8997022abebc..50dfe438bd91 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -95,10 +95,6 @@ extern unsigned int early_recursion_flag;
 /* Restricted version used during very early boot */
 void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
-	const struct exception_table_entry *e;
-	unsigned long new_ip;
-	ex_handler_t handler;
-
 	/* Ignore early NMIs. */
 	if (trapnr == X86_TRAP_NMI)
 		return;
@@ -109,19 +105,8 @@ void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 	if (regs->cs != __KERNEL_CS)
 		goto fail;
 
-	e = search_exception_tables(regs->ip);
-	if (!e)
-		goto fail;
-
-	new_ip  = ex_fixup_addr(e);
-	handler = ex_fixup_handler(e);
-
-	/* special handling not supported during early boot */
-	if (handler != ex_handler_default)
-		goto fail;
-
-	regs->ip = new_ip;
-	return;
+	if (fixup_exception(regs, trapnr))
+		return;
 
 fail:
 	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (3 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-03  8:41   ` Borislav Petkov
                     ` (2 more replies)
  2016-04-02 14:01 ` [PATCH v5 6/9] x86/msr: Carry on after a non-"safe" MSR access fails Andy Lutomirski
                   ` (9 subsequent siblings)
  14 siblings, 3 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

These hooks match the _safe variants, so name them accordingly.
This will make room for unsafe PV hooks.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
 arch/x86/include/asm/paravirt_types.h |  8 ++++----
 arch/x86/kernel/paravirt.c            |  4 ++--
 arch/x86/xen/enlighten.c              |  4 ++--
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..2e49228ed9a3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -129,34 +129,35 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
-static inline u64 paravirt_read_msr(unsigned msr, int *err)
+static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
-	return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
+	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
 }
 
-static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
+static inline int paravirt_write_msr_safe(unsigned msr,
+					  unsigned low, unsigned high)
 {
-	return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
+	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
 /* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
 	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr(msr, val1, val2);	\
+	paravirt_write_msr_safe(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
 	int _err;				\
-	val = paravirt_read_msr(msr, &_err);	\
+	val = paravirt_read_msr_safe(msr, &_err);	\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
@@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
 	wrmsr(msr, (u32)val, (u32)(val>>32));
 }
 
-#define wrmsr_safe(msr, a, b)	paravirt_write_msr(msr, a, b)
+#define wrmsr_safe(msr, a, b)	paravirt_write_msr_safe(msr, a, b)
 
 /* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b)			\
-({						\
-	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
-	(*a) = (u32)_l;				\
-	(*b) = _l >> 32;			\
-	_err;					\
+#define rdmsr_safe(msr, a, b)				\
+({							\
+	int _err;					\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	(*a) = (u32)_l;					\
+	(*b) = _l >> 32;				\
+	_err;						\
 })
 
 static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
 {
 	int err;
 
-	*p = paravirt_read_msr(msr, &err);
+	*p = paravirt_read_msr_safe(msr, &err);
 	return err;
 }
 
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 77db5616a473..5a06cccd36f0 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,10 +155,10 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR, PMC and TSR operations.
-	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
-	u64 (*read_msr)(unsigned int msr, int *err);
-	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+	/* MSR operations.
+	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	u64 (*read_msr_safe)(unsigned int msr, int *err);
+	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
 	u64 (*read_pmc)(int counter);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28b8136..8aad95478ae5 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,8 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
-	.read_msr = native_read_msr_safe,
-	.write_msr = native_write_msr_safe,
+	.read_msr_safe = native_read_msr_safe,
+	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
 	.load_tr_desc = native_load_tr_desc,
 	.set_ldt = native_set_ldt,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d09e4c9d7cc5..ff2df20d0067 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1222,8 +1222,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
-	.read_msr = xen_read_msr_safe,
-	.write_msr = xen_write_msr_safe,
+	.read_msr_safe = xen_read_msr_safe,
+	.write_msr_safe = xen_write_msr_safe,
 
 	.read_pmc = xen_read_pmc,
 
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 6/9] x86/msr: Carry on after a non-"safe" MSR access fails
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (4 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr Andy Lutomirski
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

This demotes an OOPS and likely panic due to a failed non-"safe" MSR
access to a WARN_ONCE and, for RDMSR, a return value of zero.

To be clear, this type of failure should *not* happen.  This patch
exists to minimize the chance of nasty undebuggable failures
happening when a CONFIG_PARAVIRT=y bug in the non-"safe" MSR helpers
gets fixed.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/msr.h | 10 ++++++++--
 arch/x86/mm/extable.c      | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 93fb7c1cffda..1487054a1a70 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -92,7 +92,10 @@ static inline unsigned long long native_read_msr(unsigned int msr)
 {
 	DECLARE_ARGS(val, low, high);
 
-	asm volatile("rdmsr" : EAX_EDX_RET(val, low, high) : "c" (msr));
+	asm volatile("1: rdmsr\n"
+		     "2:\n"
+		     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_rdmsr_unsafe)
+		     : EAX_EDX_RET(val, low, high) : "c" (msr));
 	if (msr_tracepoint_active(__tracepoint_read_msr))
 		do_trace_read_msr(msr, EAX_EDX_VAL(val, low, high), 0);
 	return EAX_EDX_VAL(val, low, high);
@@ -119,7 +122,10 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
 static inline void native_write_msr(unsigned int msr,
 				    unsigned low, unsigned high)
 {
-	asm volatile("wrmsr" : : "c" (msr), "a"(low), "d" (high) : "memory");
+	asm volatile("1: wrmsr\n"
+		     "2:\n"
+		     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
+		     : : "c" (msr), "a"(low), "d" (high) : "memory");
 	if (msr_tracepoint_active(__tracepoint_read_msr))
 		do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
 }
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 50dfe438bd91..98b5f45d9d79 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -50,6 +50,33 @@ bool ex_handler_ext(const struct exception_table_entry *fixup,
 }
 EXPORT_SYMBOL(ex_handler_ext);
 
+bool ex_handler_rdmsr_unsafe(const struct exception_table_entry *fixup,
+			     struct pt_regs *regs, int trapnr)
+{
+	WARN_ONCE(1, "unchecked MSR access error: RDMSR from 0x%x\n",
+		  (unsigned int)regs->cx);
+
+	/* Pretend that the read succeeded and returned 0. */
+	regs->ip = ex_fixup_addr(fixup);
+	regs->ax = 0;
+	regs->dx = 0;
+	return true;
+}
+EXPORT_SYMBOL(ex_handler_rdmsr_unsafe);
+
+bool ex_handler_wrmsr_unsafe(const struct exception_table_entry *fixup,
+			     struct pt_regs *regs, int trapnr)
+{
+	WARN_ONCE(1, "unchecked MSR access error: WRMSR to 0x%x (tried to write 0x%08x%08x)\n",
+		  (unsigned int)regs->cx,
+		  (unsigned int)regs->dx, (unsigned int)regs->ax);
+
+	/* Pretend that the write succeeded. */
+	regs->ip = ex_fixup_addr(fixup);
+	return true;
+}
+EXPORT_SYMBOL(ex_handler_wrmsr_unsafe);
+
 bool ex_has_fault_handler(unsigned long ip)
 {
 	const struct exception_table_entry *e;
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (5 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 6/9] x86/msr: Carry on after a non-"safe" MSR access fails Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

This adds paravirt hooks for unsafe MSR access.  On native, they
call native_{read,write}_msr.  On Xen, they use
xen_{read,write}_msr_safe.

Nothing uses them yet for ease of bisection.  The next patch will
use them in rdmsrl, wrmsrl, etc.

I intentionally didn't make them warn on #GP on Xen.  I think that
should be done separately by the Xen maintainers.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/msr.h            |  5 +++--
 arch/x86/include/asm/paravirt.h       | 11 +++++++++++
 arch/x86/include/asm/paravirt_types.h | 10 ++++++++--
 arch/x86/kernel/paravirt.c            |  2 ++
 arch/x86/xen/enlighten.c              | 23 +++++++++++++++++++++++
 5 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 1487054a1a70..13da359881d7 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -119,8 +119,9 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
 	return EAX_EDX_VAL(val, low, high);
 }
 
-static inline void native_write_msr(unsigned int msr,
-				    unsigned low, unsigned high)
+/* Can be uninlined because referenced by paravirt */
+notrace static inline void native_write_msr(unsigned int msr,
+					    unsigned low, unsigned high)
 {
 	asm volatile("1: wrmsr\n"
 		     "2:\n"
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 2e49228ed9a3..68297d87e85c 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -129,6 +129,17 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
+static inline u64 paravirt_read_msr(unsigned msr)
+{
+	return PVOP_CALL1(u64, pv_cpu_ops.read_msr, msr);
+}
+
+static inline void paravirt_write_msr(unsigned msr,
+				      unsigned low, unsigned high)
+{
+	return PVOP_VCALL3(pv_cpu_ops.write_msr, msr, low, high);
+}
+
 static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
 	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 5a06cccd36f0..b85aec45a1b8 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,8 +155,14 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR operations.
-	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	/* Unsafe MSR operations.  These will warn or panic on failure. */
+	u64 (*read_msr)(unsigned int msr);
+	void (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+
+	/*
+	 * Safe MSR operations.
+	 * read sets err to 0 or -EIO.  write returns 0 or -EIO.
+	 */
 	u64 (*read_msr_safe)(unsigned int msr, int *err);
 	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 8aad95478ae5..f9583917c7c4 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,6 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
+	.read_msr = native_read_msr,
+	.write_msr = native_write_msr,
 	.read_msr_safe = native_read_msr_safe,
 	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index ff2df20d0067..548cd3e02b9e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1092,6 +1092,26 @@ static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
 	return ret;
 }
 
+static u64 xen_read_msr(unsigned int msr)
+{
+	/*
+	 * This will silently swallow a #GP from RDMSR.  It may be worth
+	 * changing that.
+	 */
+	int err;
+
+	return xen_read_msr_safe(msr, &err);
+}
+
+static void xen_write_msr(unsigned int msr, unsigned low, unsigned high)
+{
+	/*
+	 * This will silently swallow a #GP from WRMSR.  It may be worth
+	 * changing that.
+	 */
+	xen_write_msr_safe(msr, low, high);
+}
+
 void xen_setup_shared_info(void)
 {
 	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
@@ -1222,6 +1242,9 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
+	.read_msr = xen_read_msr,
+	.write_msr = xen_write_msr,
+
 	.read_msr_safe = xen_read_msr_safe,
 	.write_msr_safe = xen_write_msr_safe,
 
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (6 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-13 11:46   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
  2016-04-02 14:01 ` [PATCH v5 9/9] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

Enabling CONFIG_PARAVIRT had an unintended side effect: rdmsr turned
into rdmsr_safe and wrmsr turned into wrmsr_safe, even on bare
metal.  Undo that by using the new unsafe paravirt MSR hooks.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/paravirt.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 68297d87e85c..0c99f10874e4 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -151,24 +151,21 @@ static inline int paravirt_write_msr_safe(unsigned msr,
 	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
-/* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
-	int _err;				\
-	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	u64 _l = paravirt_read_msr(msr);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr_safe(msr, val1, val2);	\
+	paravirt_write_msr(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
-	int _err;				\
-	val = paravirt_read_msr_safe(msr, &_err);	\
+	val = paravirt_read_msr(msr);		\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 9/9] x86/msr: Set the return value to zero when native_rdmsr_safe fails
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (7 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
@ 2016-04-02 14:01 ` Andy Lutomirski
  2016-04-02 14:24 ` [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Linus Torvalds
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

This will cause unchecked native_rdmsr_safe failures to return
deterministic results.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/msr.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 13da359881d7..e97e79f8a22b 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -109,7 +109,10 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
 	asm volatile("2: rdmsr ; xor %[err],%[err]\n"
 		     "1:\n\t"
 		     ".section .fixup,\"ax\"\n\t"
-		     "3:  mov %[fault],%[err] ; jmp 1b\n\t"
+		     "3: mov %[fault],%[err]\n\t"
+		     "xorl %%eax, %%eax\n\t"
+		     "xorl %%edx, %%edx\n\t"
+		     "jmp 1b\n\t"
 		     ".previous\n\t"
 		     _ASM_EXTABLE(2b, 3b)
 		     : [err] "=r" (*err), EAX_EDX_RET(val, low, high)
-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (8 preceding siblings ...)
  2016-04-02 14:01 ` [PATCH v5 9/9] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski
@ 2016-04-02 14:24 ` Linus Torvalds
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Linus Torvalds @ 2016-04-02 14:24 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

This patch series looks much nicer than the last one. I assume you
tested that the early-trap handling actually worked too? I only looked
at the patches..

Ack to it all,

           Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
@ 2016-04-02 15:13   ` Andy Lutomirski
       [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 15:13 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sat, Apr 2, 2016 at 7:24 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> This patch series looks much nicer than the last one. I assume you
> tested that the early-trap handling actually worked too? I only looked
> at the patches..
>
> Ack to it all,

I injected some BUGs in various places on 32-bit an 64-bit and it
seemed to do the right thing.  Depending on how early they were, I
either got a clean hang or I got a printout, and whether it displayed
anything didn't seem to change with and without the patches.  I think
that early_printk doesn't work from the very very beginning.

I also tried a bad wrmsrl at a couple early points.  Very very early
it just works with not warning.  A little later and it prints the
warning.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
@ 2016-04-02 15:21     ` Linus Torvalds
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Torvalds @ 2016-04-02 15:21 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sat, Apr 2, 2016 at 10:13 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> I also tried a bad wrmsrl at a couple early points.  Very very early
> it just works with not warning.  A little later and it prints the
> warning.

Ok, that sounds like the correct behavior - I'm sure the very very
early ones "warned" too, it just got dropped on the floor due to being
before the printing/logging was up and running.

And even then, it's obviously much better than having machines
mysteriously just reboot.

Thanks,
         Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
  2016-04-02 14:01 ` [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski
@ 2016-04-02 18:39   ` Borislav Petkov
       [not found]   ` <20160402183919.GA2538@pd.tnic>
  2016-04-13 11:44   ` [tip:x86/asm] x86/head: Move early exception panic code into early_fixup_exception() tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-02 18:39 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat, Apr 02, 2016 at 07:01:34AM -0700, Andy Lutomirski wrote:
> This removes a bunch of assembly and adds some C code instead.  It
> changes the actual printouts on both 32-bit and 64-bit kernels, but
> they still seem okay.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/uaccess.h |  2 +-
>  arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
>  arch/x86/kernel/head_64.S      | 45 ++------------------------------------
>  arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
>  4 files changed, 32 insertions(+), 93 deletions(-)

...

> @@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>  
>  	/* Ignore early NMIs. */
>  	if (trapnr == X86_TRAP_NMI)
> -		return 1;
> +		return;
> +
> +	if (early_recursion_flag > 2)
> +		goto halt_loop;
> +
> +	if (regs->cs != __KERNEL_CS)
> +		goto fail;
>  
>  	e = search_exception_tables(regs->ip);
>  	if (!e)
> -		return 0;
> +		goto fail;
>  
>  	new_ip  = ex_fixup_addr(e);
>  	handler = ex_fixup_handler(e);
>  
>  	/* special handling not supported during early boot */
>  	if (handler != ex_handler_default)
> -		return 0;
> +		goto fail;
>  
>  	regs->ip = new_ip;
> -	return 1;
> +	return;
> +
> +fail:
> +	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
> +		     (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
> +		     regs->orig_ax, read_cr2());
> +
> +	show_regs(regs);

To make this even better, it could be something called early_show_regs()
or so and be a simplified version of __show_regs() on both bitness but
which calls early_printk().

This way you'll be able to get out stuff to the console as early as
possible.

Btw, you don't need to dump rIP, CR2, etc in the PANIC message above
since you're going to early_show_regs() anyway.

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found] ` <20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org>
@ 2016-04-02 18:52   ` Borislav Petkov
       [not found]   ` <20160402185227.GB2538@pd.tnic>
  2016-04-13 11:44   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-02 18:52 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat, Apr 02, 2016 at 07:01:35AM -0700, Andy Lutomirski wrote:
> Now that early_fixup_exception has pt_regs, we can just call
> fixup_exception from it.  This will make fancy exception handlers
> work early.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/mm/extable.c | 19 ++-----------------
>  1 file changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 8997022abebc..50dfe438bd91 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -95,10 +95,6 @@ extern unsigned int early_recursion_flag;
>  /* Restricted version used during very early boot */
>  void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>  {
> -	const struct exception_table_entry *e;
> -	unsigned long new_ip;
> -	ex_handler_t handler;
> -
>  	/* Ignore early NMIs. */
>  	if (trapnr == X86_TRAP_NMI)
>  		return;
> @@ -109,19 +105,8 @@ void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>  	if (regs->cs != __KERNEL_CS)
>  		goto fail;
>  
> -	e = search_exception_tables(regs->ip);
> -	if (!e)
> -		goto fail;
> -
> -	new_ip  = ex_fixup_addr(e);
> -	handler = ex_fixup_handler(e);
> -
> -	/* special handling not supported during early boot */
> -	if (handler != ex_handler_default)
> -		goto fail;

Hold on, what happened to the uaccess handling not being supported
during early boot?

So before Tony changed it, the original code had:


 /* Restricted version used during very early boot */
 int __init early_fixup_exception(unsigned long *ip)
 {

...
-               if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
-                       /* uaccess handling not supported during early boot */
-                       return 0;
-               }

I'm guessing that wasn't supported early, probably because some stuff
wasn't initialized yet. Our normal, late fixup is by doing:

        current_thread_info()->uaccess_err = 1;

and I'm assuming we can't do that early. current_thread_info is probably
not setup yet...

> -
> -	regs->ip = new_ip;
> -	return;
> +	if (fixup_exception(regs, trapnr))
> +		return;

So why can we do it now, all of a sudden?

/me is scratching head.

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]   ` <20160402183919.GA2538@pd.tnic>
@ 2016-04-02 20:13     ` Andy Lutomirski
       [not found]     ` <CALCETrUJVDpzNQMsY=_jWNOMer+TdW2p4wES4OBCfYU7gq3fbg@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 20:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sat, Apr 2, 2016 at 11:39 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 07:01:34AM -0700, Andy Lutomirski wrote:
>> This removes a bunch of assembly and adds some C code instead.  It
>> changes the actual printouts on both 32-bit and 64-bit kernels, but
>> they still seem okay.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/include/asm/uaccess.h |  2 +-
>>  arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
>>  arch/x86/kernel/head_64.S      | 45 ++------------------------------------
>>  arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
>>  4 files changed, 32 insertions(+), 93 deletions(-)
>
> ...
>
>> @@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>>
>>       /* Ignore early NMIs. */
>>       if (trapnr == X86_TRAP_NMI)
>> -             return 1;
>> +             return;
>> +
>> +     if (early_recursion_flag > 2)
>> +             goto halt_loop;
>> +
>> +     if (regs->cs != __KERNEL_CS)
>> +             goto fail;
>>
>>       e = search_exception_tables(regs->ip);
>>       if (!e)
>> -             return 0;
>> +             goto fail;
>>
>>       new_ip  = ex_fixup_addr(e);
>>       handler = ex_fixup_handler(e);
>>
>>       /* special handling not supported during early boot */
>>       if (handler != ex_handler_default)
>> -             return 0;
>> +             goto fail;
>>
>>       regs->ip = new_ip;
>> -     return 1;
>> +     return;
>> +
>> +fail:
>> +     early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
>> +                  (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
>> +                  regs->orig_ax, read_cr2());
>> +
>> +     show_regs(regs);
>
> To make this even better, it could be something called early_show_regs()
> or so and be a simplified version of __show_regs() on both bitness but
> which calls early_printk().
>
> This way you'll be able to get out stuff to the console as early as
> possible.
>
> Btw, you don't need to dump rIP, CR2, etc in the PANIC message above
> since you're going to early_show_regs() anyway.

Given that I this isn't really a regression with my patches (it
probably never worked much better on 32-bit and the regs never would
have shown at all on 64-bit), I propose a different approach: make
printk work earlier.  Something like:

if (early) {
    early_printk(args);
}

or early_vprintk or whatever.

If the cost of a branch mattered, this could be alternative-patched
out later on, but that seems silly.  I also bet that a more sensible
fallback could be created in which printk would try to use an early
console if there's no real console.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]   ` <20160402185227.GB2538@pd.tnic>
@ 2016-04-02 20:16     ` Andy Lutomirski
       [not found]     ` <CALCETrV7PxgmhU_yXjJSugY88T8Do=vBDuG54fOFSyLp7nGcJA@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 20:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sat, Apr 2, 2016 at 11:52 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 07:01:35AM -0700, Andy Lutomirski wrote:
>> Now that early_fixup_exception has pt_regs, we can just call
>> fixup_exception from it.  This will make fancy exception handlers
>> work early.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/mm/extable.c | 19 ++-----------------
>>  1 file changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
>> index 8997022abebc..50dfe438bd91 100644
>> --- a/arch/x86/mm/extable.c
>> +++ b/arch/x86/mm/extable.c
>> @@ -95,10 +95,6 @@ extern unsigned int early_recursion_flag;
>>  /* Restricted version used during very early boot */
>>  void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>>  {
>> -     const struct exception_table_entry *e;
>> -     unsigned long new_ip;
>> -     ex_handler_t handler;
>> -
>>       /* Ignore early NMIs. */
>>       if (trapnr == X86_TRAP_NMI)
>>               return;
>> @@ -109,19 +105,8 @@ void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>>       if (regs->cs != __KERNEL_CS)
>>               goto fail;
>>
>> -     e = search_exception_tables(regs->ip);
>> -     if (!e)
>> -             goto fail;
>> -
>> -     new_ip  = ex_fixup_addr(e);
>> -     handler = ex_fixup_handler(e);
>> -
>> -     /* special handling not supported during early boot */
>> -     if (handler != ex_handler_default)
>> -             goto fail;
>
> Hold on, what happened to the uaccess handling not being supported
> during early boot?
>
> So before Tony changed it, the original code had:
>
>
>  /* Restricted version used during very early boot */
>  int __init early_fixup_exception(unsigned long *ip)
>  {
>
> ...
> -               if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> -                       /* uaccess handling not supported during early boot */
> -                       return 0;
> -               }
>
> I'm guessing that wasn't supported early, probably because some stuff
> wasn't initialized yet. Our normal, late fixup is by doing:
>
>         current_thread_info()->uaccess_err = 1;
>
> and I'm assuming we can't do that early. current_thread_info is probably
> not setup yet...
>
>> -
>> -     regs->ip = new_ip;
>> -     return;
>> +     if (fixup_exception(regs, trapnr))
>> +             return;
>
> So why can we do it now, all of a sudden?

I have no idea why it was explicitly unsupported, but I'm guessing it
was just to avoid duplicating the code.  Early "ext" uaccess failures
are certainly not going to work, but I don't think this is a problem
-- there's no userspace before trap_init runs, so how exactly is an
"ext" uaccess going to happen in the first place?

In any event, if it did happen in older kernels, it would have
immediately panicked due to that code.  At least with my code it just
might manage to EFAULT correctly.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]     ` <CALCETrUJVDpzNQMsY=_jWNOMer+TdW2p4wES4OBCfYU7gq3fbg@mail.gmail.com>
@ 2016-04-02 20:47       ` Borislav Petkov
       [not found]       ` <20160402204752.GC2538@pd.tnic>
  1 sibling, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-02 20:47 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> Given that I this isn't really a regression with my patches (it
> probably never worked much better on 32-bit and the regs never would
> have shown at all on 64-bit),

You're right. That thing calls printk *and* early_printk, WTF:

#ifdef CONFIG_EARLY_PRINTK

	call early_printk
	...

	call dump_stack

	...

	call __print_symbol

those last two call printk. Great.

> I propose a different approach: make
> printk work earlier.  Something like:
> 
> if (early) {
>     early_printk(args);
> }
> 
> or early_vprintk or whatever.
> 
> If the cost of a branch mattered, this could be alternative-patched
> out later on, but that seems silly.  I also bet that a more sensible
> fallback could be created in which printk would try to use an early
> console if there's no real console.

So how about this:

printk() does

	vprintk_func = this_cpu_read(printk_func);

and that's

DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default

I guess we can make that function be early_printk-something and once
printk is initialized, we overwrite it with vprintk_default.

Elegant and no need for if branches and alternatives.

Hmmm.

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]     ` <CALCETrV7PxgmhU_yXjJSugY88T8Do=vBDuG54fOFSyLp7nGcJA@mail.gmail.com>
@ 2016-04-02 20:52       ` Borislav Petkov
       [not found]       ` <20160402205248.GD2538@pd.tnic>
  1 sibling, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-02 20:52 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sat, Apr 02, 2016 at 01:16:07PM -0700, Andy Lutomirski wrote:
> I have no idea why it was explicitly unsupported, but I'm guessing it
> was just to avoid duplicating the code.  Early "ext" uaccess failures
> are certainly not going to work, but I don't think this is a problem
> -- there's no userspace before trap_init runs, so how exactly is an
> "ext" uaccess going to happen in the first place?
> 
> In any event, if it did happen in older kernels, it would have
> immediately panicked due to that code.  At least with my code it just
> might manage to EFAULT correctly.

Yeah, I was wondering what that early thing meant.

Linus or tip guys probably remember what this whole deal with early
uaccess was about. I'll try to do some git archeology tomorrow.

In any event, it would be a good idea IMO, to hold that situation down
in a comment somewhere. Once we've figured it out...

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]       ` <20160402204752.GC2538@pd.tnic>
@ 2016-04-02 20:58         ` Andy Lutomirski
       [not found]         ` <CALCETrXHR3T3PJwOWRFHTMK5WjhDyTJfaJtkiHd+g7evnqp54A@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 20:58 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jan Kara, KVM list, Peter Zijlstra, Linus Torvalds, X86 ML,
	linux-kernel, xen-devel, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

[cc Jan Kara]

On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
>> Given that I this isn't really a regression with my patches (it
>> probably never worked much better on 32-bit and the regs never would
>> have shown at all on 64-bit),
>
> You're right. That thing calls printk *and* early_printk, WTF:
>
> #ifdef CONFIG_EARLY_PRINTK
>
>         call early_printk
>         ...
>
>         call dump_stack
>
>         ...
>
>         call __print_symbol
>
> those last two call printk. Great.
>
>> I propose a different approach: make
>> printk work earlier.  Something like:
>>
>> if (early) {
>>     early_printk(args);
>> }
>>
>> or early_vprintk or whatever.
>>
>> If the cost of a branch mattered, this could be alternative-patched
>> out later on, but that seems silly.  I also bet that a more sensible
>> fallback could be created in which printk would try to use an early
>> console if there's no real console.
>
> So how about this:
>
> printk() does
>
>         vprintk_func = this_cpu_read(printk_func);
>
> and that's
>
> DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
>
> I guess we can make that function be early_printk-something and once
> printk is initialized, we overwrite it with vprintk_default.
>
> Elegant and no need for if branches and alternatives.
>
> Hmmm.

Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]       ` <20160402205248.GD2538@pd.tnic>
@ 2016-04-03  8:07         ` Borislav Petkov
       [not found]         ` <20160403080737.GA19007@pd.tnic>
  1 sibling, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-03  8:07 UTC (permalink / raw)
  To: Andy Lutomirski, H. Peter Anvin
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sat, Apr 02, 2016 at 10:52:48PM +0200, Borislav Petkov wrote:
> On Sat, Apr 02, 2016 at 01:16:07PM -0700, Andy Lutomirski wrote:
> > I have no idea why it was explicitly unsupported, but I'm guessing it
> > was just to avoid duplicating the code.  Early "ext" uaccess failures
> > are certainly not going to work, but I don't think this is a problem
> > -- there's no userspace before trap_init runs, so how exactly is an
> > "ext" uaccess going to happen in the first place?
> > 
> > In any event, if it did happen in older kernels, it would have
> > immediately panicked due to that code.  At least with my code it just
> > might manage to EFAULT correctly.
> 
> Yeah, I was wondering what that early thing meant.
> 
> Linus or tip guys probably remember what this whole deal with early
> uaccess was about. I'll try to do some git archeology tomorrow.

Yep, just as I suspected:

6a1ea279c210 ("x86, extable: Add early_fixup_exception()")

Apparently, thread_info might not have been setup yet. I'm guessing the
intention behind this no-uaccess-fixup-early is to not even attempt any
fixup due to stuff *probably* not initialized yet and so the safer thing
would be to panic instead.

I'm wondering whether making it try to EFAULT correctly is the right
thing to do... We're certainly more conservative if we panic and not
allow some silently failed attempt at recovery which looks successful,
to continue.

hpa, thoughts?

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
  2016-04-02 14:01 ` [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
@ 2016-04-03  8:41   ` Borislav Petkov
       [not found]   ` <20160403084129.GB19007@pd.tnic>
  2016-04-13 11:44   ` [tip:x86/asm] x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-03  8:41 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat, Apr 02, 2016 at 07:01:36AM -0700, Andy Lutomirski wrote:
> These hooks match the _safe variants, so name them accordingly.
> This will make room for unsafe PV hooks.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
>  arch/x86/include/asm/paravirt_types.h |  8 ++++----
>  arch/x86/kernel/paravirt.c            |  4 ++--
>  arch/x86/xen/enlighten.c              |  4 ++--
>  4 files changed, 25 insertions(+), 24 deletions(-)

...

> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 77db5616a473..5a06cccd36f0 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
>  	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
>  		      unsigned int *ecx, unsigned int *edx);
>  
> -	/* MSR, PMC and TSR operations.
> -	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
> -	u64 (*read_msr)(unsigned int msr, int *err);
> -	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
> +	/* MSR operations.
> +	   err = 0/-EIO.  wrmsr returns 0/-EIO. */

Please reformat this comment properly, while you're at it:

	/*
	 * A sentence.
	 * Another sentence.
	 */

> +	u64 (*read_msr_safe)(unsigned int msr, int *err)Please reformat this
> comment properly, while you're at it:
> +	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
>  
>  	u64 (*read_pmc)(int counter);
>  

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]         ` <20160403080737.GA19007@pd.tnic>
@ 2016-04-03 13:22           ` Andy Lutomirski
  2016-04-03 13:51           ` Linus Torvalds
       [not found]           ` <CA+55aFw19X7PO2xd1PpUav1rSyCqUqWgi9+SA7TgjaWMaxexHQ@mail.gmail.com>
  2 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-03 13:22 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, H. Peter Anvin, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sun, Apr 3, 2016 at 1:07 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 10:52:48PM +0200, Borislav Petkov wrote:
>> On Sat, Apr 02, 2016 at 01:16:07PM -0700, Andy Lutomirski wrote:
>> > I have no idea why it was explicitly unsupported, but I'm guessing it
>> > was just to avoid duplicating the code.  Early "ext" uaccess failures
>> > are certainly not going to work, but I don't think this is a problem
>> > -- there's no userspace before trap_init runs, so how exactly is an
>> > "ext" uaccess going to happen in the first place?
>> >
>> > In any event, if it did happen in older kernels, it would have
>> > immediately panicked due to that code.  At least with my code it just
>> > might manage to EFAULT correctly.
>>
>> Yeah, I was wondering what that early thing meant.
>>
>> Linus or tip guys probably remember what this whole deal with early
>> uaccess was about. I'll try to do some git archeology tomorrow.
>
> Yep, just as I suspected:
>
> 6a1ea279c210 ("x86, extable: Add early_fixup_exception()")
>
> Apparently, thread_info might not have been setup yet. I'm guessing the
> intention behind this no-uaccess-fixup-early is to not even attempt any
> fixup due to stuff *probably* not initialized yet and so the safer thing
> would be to panic instead.
>
> I'm wondering whether making it try to EFAULT correctly is the right
> thing to do... We're certainly more conservative if we panic and not
> allow some silently failed attempt at recovery which looks successful,
> to continue.
>
> hpa, thoughts?

I don't think this matters much.  There aren't many users of this
mechanism in the tree:

./arch/x86/kernel/signal.c:    get_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal.c:    put_user_try {
./arch/x86/kernel/signal_compat.c:    put_user_try {
./arch/x86/kernel/signal_compat.c:    get_user_try {
./arch/x86/kernel/vm86_32.c:    put_user_try {
./arch/x86/kernel/vm86_32.c:    get_user_try {
./arch/x86/ia32/ia32_signal.c:    get_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/ia32/ia32_signal.c:    put_user_try {
./arch/x86/include/asm/uaccess.h: * {get|put}_user_try and catch
./arch/x86/include/asm/uaccess.h: * get_user_try {
./arch/x86/include/asm/uaccess.h:#define get_user_try        uaccess_try
./arch/x86/include/asm/uaccess.h:#define put_user_try        uaccess_try

I don't see how we could get to that code in the first place without
current_thread_info() working.

If we can ever convince gcc to do jump labels properly for uaccess, it
would probably be better to just delete all that code.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
       [not found]   ` <20160403084129.GB19007@pd.tnic>
@ 2016-04-03 13:23     ` Andy Lutomirski
       [not found]     ` <CALCETrUcqTx00pKYEt49QSKKBKVEx-Uv0D6dzE1-Em6Q-wkSpQ@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-03 13:23 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sun, Apr 3, 2016 at 1:41 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 07:01:36AM -0700, Andy Lutomirski wrote:
>> These hooks match the _safe variants, so name them accordingly.
>> This will make room for unsafe PV hooks.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
>>  arch/x86/include/asm/paravirt_types.h |  8 ++++----
>>  arch/x86/kernel/paravirt.c            |  4 ++--
>>  arch/x86/xen/enlighten.c              |  4 ++--
>>  4 files changed, 25 insertions(+), 24 deletions(-)
>
> ...
>
>> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> index 77db5616a473..5a06cccd36f0 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
>>       void (*cpuid)(unsigned int *eax, unsigned int *ebx,
>>                     unsigned int *ecx, unsigned int *edx);
>>
>> -     /* MSR, PMC and TSR operations.
>> -        err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
>> -     u64 (*read_msr)(unsigned int msr, int *err);
>> -     int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
>> +     /* MSR operations.
>> +        err = 0/-EIO.  wrmsr returns 0/-EIO. */
>
> Please reformat this comment properly, while you're at it:
>
>         /*
>          * A sentence.
>          * Another sentence.
>          */

You already caught that one.  It's fixed in "x86/paravirt: Add
paravirt_{read,write}_msr".

Congrats on being deterministic :)

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]         ` <20160403080737.GA19007@pd.tnic>
  2016-04-03 13:22           ` Andy Lutomirski
@ 2016-04-03 13:51           ` Linus Torvalds
       [not found]           ` <CA+55aFw19X7PO2xd1PpUav1rSyCqUqWgi9+SA7TgjaWMaxexHQ@mail.gmail.com>
  2 siblings, 0 replies; 48+ messages in thread
From: Linus Torvalds @ 2016-04-03 13:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, X86 ML, linux-kernel, Andy Lutomirski,
	Andy Lutomirski, H. Peter Anvin, Paolo Bonzini, Andrew Morton,
	xen-devel, Arjan van de Ven

On Sun, Apr 3, 2016 at 3:07 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> I'm wondering whether making it try to EFAULT correctly is the right
> thing to do... We're certainly more conservative if we panic and not
> allow some silently failed attempt at recovery which looks successful,
> to continue.

No, please don't fail at early boot.

Early boot is just about the *worst* situation to try to debug odd
failures, exactly since things like printk may not be reliable, and
things won't get logged etc.

So particularly during early boot we should try as hard as possible
not to crash - even if it means not being able to log about a problem.
At least that way you have a hopefully working machine and can *maybe*
debug things.

            Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]           ` <CA+55aFw19X7PO2xd1PpUav1rSyCqUqWgi9+SA7TgjaWMaxexHQ@mail.gmail.com>
@ 2016-04-03 13:55             ` Andy Lutomirski
       [not found]             ` <CALCETrX_x+tV4m3SwgCfXg9y5gia8eMhhTdUnHuXpXhnULkHRw@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-03 13:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: KVM list, Peter Zijlstra, X86 ML, linux-kernel, xen-devel,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sun, Apr 3, 2016 at 6:51 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sun, Apr 3, 2016 at 3:07 AM, Borislav Petkov <bp@alien8.de> wrote:
>>
>> I'm wondering whether making it try to EFAULT correctly is the right
>> thing to do... We're certainly more conservative if we panic and not
>> allow some silently failed attempt at recovery which looks successful,
>> to continue.
>
> No, please don't fail at early boot.
>
> Early boot is just about the *worst* situation to try to debug odd
> failures, exactly since things like printk may not be reliable, and
> things won't get logged etc.
>
> So particularly during early boot we should try as hard as possible
> not to crash - even if it means not being able to log about a problem.
> At least that way you have a hopefully working machine and can *maybe*
> debug things.
>

In this regard, at least, my patch is the right approach.  Calling the
handler, whatever it is, is less likely to panic than refusing to call
it.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
       [not found]     ` <CALCETrUcqTx00pKYEt49QSKKBKVEx-Uv0D6dzE1-Em6Q-wkSpQ@mail.gmail.com>
@ 2016-04-03 14:07       ` Borislav Petkov
  0 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-03 14:07 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Sun, Apr 03, 2016 at 06:23:36AM -0700, Andy Lutomirski wrote:
> You already caught that one.  It's fixed in "x86/paravirt: Add
> paravirt_{read,write}_msr".
> 
> Congrats on being deterministic :)

LOL.

TBH, I had a strange deja-vu while typing... :-)

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]             ` <CALCETrX_x+tV4m3SwgCfXg9y5gia8eMhhTdUnHuXpXhnULkHRw@mail.gmail.com>
@ 2016-04-03 14:10               ` Borislav Petkov
  2016-04-03 14:17               ` Linus Torvalds
       [not found]               ` <20160403141041.GE19007@pd.tnic>
  2 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-03 14:10 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, H. Peter Anvin, Paolo Bonzini,
	Andrew Morton, Linus Torvalds

On Sun, Apr 03, 2016 at 06:55:00AM -0700, Andy Lutomirski wrote:
> > No, please don't fail at early boot.
> >
> > Early boot is just about the *worst* situation to try to debug odd
> > failures, exactly since things like printk may not be reliable, and
> > things won't get logged etc.
> >
> > So particularly during early boot we should try as hard as possible
> > not to crash - even if it means not being able to log about a problem.
> > At least that way you have a hopefully working machine and can *maybe*
> > debug things.
> >
> 
> In this regard, at least, my patch is the right approach.  Calling the
> handler, whatever it is, is less likely to panic than refusing to call
> it.

Ok, good.

But can we pretty please document this whole situation, i.e., the
fact that we're trying really really hard not to fail early boot for
debuggability reasons - either in the commit message or better in the
code, for future reference. I think this is an important aspect to hold
down.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]             ` <CALCETrX_x+tV4m3SwgCfXg9y5gia8eMhhTdUnHuXpXhnULkHRw@mail.gmail.com>
  2016-04-03 14:10               ` Borislav Petkov
@ 2016-04-03 14:17               ` Linus Torvalds
       [not found]               ` <20160403141041.GE19007@pd.tnic>
  2 siblings, 0 replies; 48+ messages in thread
From: Linus Torvalds @ 2016-04-03 14:17 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, X86 ML, linux-kernel, xen-devel,
	Borislav Petkov, Andy Lutomirski, H. Peter Anvin, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sun, Apr 3, 2016 at 8:55 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> In this regard, at least, my patch is the right approach.  Calling the
> handler, whatever it is, is less likely to panic than refusing to call
> it.

Agreed, the patch series looks like a good thing in general.

            Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]         ` <CALCETrXHR3T3PJwOWRFHTMK5WjhDyTJfaJtkiHd+g7evnqp54A@mail.gmail.com>
@ 2016-04-04 11:52           ` Jan Kara
       [not found]           ` <20160404115206.GG8372@quack.suse.cz>
  1 sibling, 0 replies; 48+ messages in thread
From: Jan Kara @ 2016-04-04 11:52 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jan Kara, KVM list, Peter Zijlstra, Linus Torvalds, X86 ML,
	linux-kernel, pmladek, xen-devel, Borislav Petkov,
	Andy Lutomirski, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat 02-04-16 13:58:19, Andy Lutomirski wrote:
> [cc Jan Kara]
> 
> On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> > On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> >> Given that I this isn't really a regression with my patches (it
> >> probably never worked much better on 32-bit and the regs never would
> >> have shown at all on 64-bit),
> >
> > You're right. That thing calls printk *and* early_printk, WTF:
> >
> > #ifdef CONFIG_EARLY_PRINTK
> >
> >         call early_printk
> >         ...
> >
> >         call dump_stack
> >
> >         ...
> >
> >         call __print_symbol
> >
> > those last two call printk. Great.
> >
> >> I propose a different approach: make
> >> printk work earlier.  Something like:
> >>
> >> if (early) {
> >>     early_printk(args);
> >> }
> >>
> >> or early_vprintk or whatever.
> >>
> >> If the cost of a branch mattered, this could be alternative-patched
> >> out later on, but that seems silly.  I also bet that a more sensible
> >> fallback could be created in which printk would try to use an early
> >> console if there's no real console.
> >
> > So how about this:
> >
> > printk() does
> >
> >         vprintk_func = this_cpu_read(printk_func);
> >
> > and that's
> >
> > DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
> >
> > I guess we can make that function be early_printk-something and once
> > printk is initialized, we overwrite it with vprintk_default.
> >
> > Elegant and no need for if branches and alternatives.
> >
> > Hmmm.
> 
> Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?

Sounds like a good idea to me. I've also consulted this with Petr Mladek
(added to CC) who is using printk_func per-cpu variable in his
printk-from-NMI patches and he also doesn't see a problem with this.

I was just wondering about one thing - this way we add more early printks
if I understand your intention right. Are we guaranteed that they happen
only from a single CPU? Because currently there is no locking in
early_printk() and thus we can end up writing to early console several
messages in parallel from different CPUs. Not sure what's going to happen
in that case...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]           ` <20160404115206.GG8372@quack.suse.cz>
@ 2016-04-04 12:46             ` Peter Zijlstra
  2016-04-04 15:32             ` Andy Lutomirski
       [not found]             ` <CALCETrVriftUPNZ8pCQ1x9TWDHnRx8fWUvJgCQToPEOxqMLb3w@mail.gmail.com>
  2 siblings, 0 replies; 48+ messages in thread
From: Peter Zijlstra @ 2016-04-04 12:46 UTC (permalink / raw)
  To: Jan Kara
  Cc: KVM list, Linus Torvalds, X86 ML, linux-kernel, pmladek,
	Andy Lutomirski, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, xen-devel, Arjan van de Ven

On Mon, Apr 04, 2016 at 01:52:06PM +0200, Jan Kara wrote:
> Sounds like a good idea to me. I've also consulted this with Petr Mladek
> (added to CC) who is using printk_func per-cpu variable in his
> printk-from-NMI patches and he also doesn't see a problem with this.

There's a few printk() variants that do not go through this; which means
they're broken for a number of cases, including the kdb printk
redirection, this NMI stuff etc.

> I was just wondering about one thing - this way we add more early printks
> if I understand your intention right. Are we guaranteed that they happen
> only from a single CPU? Because currently there is no locking in
> early_printk() and thus we can end up writing to early console several
> messages in parallel from different CPUs. Not sure what's going to happen
> in that case...

You get lovely per char interleaving on you serial line ;-)

What I've done in the past was something like the below; that way you
only get the normal task->softirq->irq->nmi nesting, which is mostly
decipherable.

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index bfbf284e4218..c4c3269ff104 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1907,17 +1907,36 @@ struct console *early_console;
 asmlinkage __visible void early_printk(const char *fmt, ...)
 {
 	va_list ap;
+	static int print_cpu = -1;
 	char buf[512];
-	int n;
+	int n, cpu;
 
 	if (!early_console)
 		return;
 
+	preempt_disable();
+	cpu = raw_smp_processor_id();
+	for (;;) {
+		if (READ_ONCE(print_cpu) == cpu)
+			break;
+
+		if (READ_ONCE(print_cpu) == -1 &&
+		    cmpxchg(&print_cpu, -1, cpu) == -1) {
+			cpu = -1;
+			break;
+		}
+
+		cpu_relax();
+	}
+
 	va_start(ap, fmt);
 	n = vscnprintf(buf, sizeof(buf), fmt, ap);
 	va_end(ap);
 
 	early_console->write(early_console, buf, n);
+
+	smp_store_release(&print_cpu, cpu);
+	preempt_enable();
 }
 #endif
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]           ` <20160404115206.GG8372@quack.suse.cz>
  2016-04-04 12:46             ` Peter Zijlstra
@ 2016-04-04 15:32             ` Andy Lutomirski
       [not found]             ` <CALCETrVriftUPNZ8pCQ1x9TWDHnRx8fWUvJgCQToPEOxqMLb3w@mail.gmail.com>
  2 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-04 15:32 UTC (permalink / raw)
  To: Jan Kara
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	Petr Mladek, xen-devel, Borislav Petkov, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Apr 4, 2016 4:51 AM, "Jan Kara" <jack@suse.cz> wrote:
>
> On Sat 02-04-16 13:58:19, Andy Lutomirski wrote:
> > [cc Jan Kara]
> >
> > On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> > > On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> > >> Given that I this isn't really a regression with my patches (it
> > >> probably never worked much better on 32-bit and the regs never would
> > >> have shown at all on 64-bit),
> > >
> > > You're right. That thing calls printk *and* early_printk, WTF:
> > >
> > > #ifdef CONFIG_EARLY_PRINTK
> > >
> > >         call early_printk
> > >         ...
> > >
> > >         call dump_stack
> > >
> > >         ...
> > >
> > >         call __print_symbol
> > >
> > > those last two call printk. Great.
> > >
> > >> I propose a different approach: make
> > >> printk work earlier.  Something like:
> > >>
> > >> if (early) {
> > >>     early_printk(args);
> > >> }
> > >>
> > >> or early_vprintk or whatever.
> > >>
> > >> If the cost of a branch mattered, this could be alternative-patched
> > >> out later on, but that seems silly.  I also bet that a more sensible
> > >> fallback could be created in which printk would try to use an early
> > >> console if there's no real console.
> > >
> > > So how about this:
> > >
> > > printk() does
> > >
> > >         vprintk_func = this_cpu_read(printk_func);
> > >
> > > and that's
> > >
> > > DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
> > >
> > > I guess we can make that function be early_printk-something and once
> > > printk is initialized, we overwrite it with vprintk_default.
> > >
> > > Elegant and no need for if branches and alternatives.
> > >
> > > Hmmm.
> >
> > Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?
>
> Sounds like a good idea to me. I've also consulted this with Petr Mladek
> (added to CC) who is using printk_func per-cpu variable in his
> printk-from-NMI patches and he also doesn't see a problem with this.
>
> I was just wondering about one thing - this way we add more early printks
> if I understand your intention right. Are we guaranteed that they happen
> only from a single CPU? Because currently there is no locking in
> early_printk() and thus we can end up writing to early console several
> messages in parallel from different CPUs. Not sure what's going to happen
> in that case...

Adding locking would be easy enough, wouldn't it?

But do any platforms really boot a second CPU before switching to real
printk?  Given that I see all the smpboot stuff in dmesg, I guess real
printk happens first.  I admit I haven't actually checked.

--Andy

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]             ` <CALCETrVriftUPNZ8pCQ1x9TWDHnRx8fWUvJgCQToPEOxqMLb3w@mail.gmail.com>
@ 2016-04-04 15:36               ` Arjan van de Ven
  2016-04-04 16:00               ` Peter Zijlstra
       [not found]               ` <20160404160042.GB3448@twins.programming.kicks-ass.net>
  2 siblings, 0 replies; 48+ messages in thread
From: Arjan van de Ven @ 2016-04-04 15:36 UTC (permalink / raw)
  To: Andy Lutomirski, Jan Kara
  Cc: KVM list, Peter Zijlstra, X86 ML, linux-kernel, Petr Mladek,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Linus Torvalds

On 4/4/2016 8:32 AM, Andy Lutomirski wrote:
>
> Adding locking would be easy enough, wouldn't it?
>
> But do any platforms really boot a second CPU before switching to real
> printk?  Given that I see all the smpboot stuff in dmesg, I guess real
> printk happens first.  I admit I haven't actually checked.

adding locking also makes things more fragile in terms of getting the last thing out
before you go down in flaming death....

until it's a proven problem, this early, get the message out at all is more important
than getting it out perfectly, sometimes.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early
       [not found]               ` <20160403141041.GE19007@pd.tnic>
@ 2016-04-04 15:47                 ` Andy Lutomirski
  0 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-04 15:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, X86 ML, linux-kernel,
	xen-devel, Andy Lutomirski, H. Peter Anvin, Paolo Bonzini,
	Andrew Morton, Linus Torvalds

On Sun, Apr 3, 2016 at 7:10 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Apr 03, 2016 at 06:55:00AM -0700, Andy Lutomirski wrote:
>> > No, please don't fail at early boot.
>> >
>> > Early boot is just about the *worst* situation to try to debug odd
>> > failures, exactly since things like printk may not be reliable, and
>> > things won't get logged etc.
>> >
>> > So particularly during early boot we should try as hard as possible
>> > not to crash - even if it means not being able to log about a problem.
>> > At least that way you have a hopefully working machine and can *maybe*
>> > debug things.
>> >
>>
>> In this regard, at least, my patch is the right approach.  Calling the
>> handler, whatever it is, is less likely to panic than refusing to call
>> it.
>
> Ok, good.
>
> But can we pretty please document this whole situation, i.e., the
> fact that we're trying really really hard not to fail early boot for
> debuggability reasons - either in the commit message or better in the
> code, for future reference. I think this is an important aspect to hold
> down.

I emailed out a followup patch to add a comment.

--Andy

>
> Thanks.
>
> --
> Regards/Gruss,
>     Boris.
>
> ECO tip #101: Trim your mails when you reply.



-- 
Andy Lutomirski
AMA Capital Management, LLC

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]             ` <CALCETrVriftUPNZ8pCQ1x9TWDHnRx8fWUvJgCQToPEOxqMLb3w@mail.gmail.com>
  2016-04-04 15:36               ` Arjan van de Ven
@ 2016-04-04 16:00               ` Peter Zijlstra
       [not found]               ` <20160404160042.GB3448@twins.programming.kicks-ass.net>
  2 siblings, 0 replies; 48+ messages in thread
From: Peter Zijlstra @ 2016-04-04 16:00 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jan Kara, KVM list, Linus Torvalds, X86 ML, linux-kernel,
	Petr Mladek, xen-devel, Borislav Petkov, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Mon, Apr 04, 2016 at 08:32:21AM -0700, Andy Lutomirski wrote:

> Adding locking would be easy enough, wouldn't it?

See patch in this thread..

> But do any platforms really boot a second CPU before switching to real
> printk? 

I _only_ use early_printk() as printk() is a quagmire of fail :-)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (10 preceding siblings ...)
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
@ 2016-04-04 16:23 ` Borislav Petkov
       [not found] ` <880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org>
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-04 16:23 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat, Apr 02, 2016 at 07:01:31AM -0700, Andy Lutomirski wrote:
> There are two parts here:
> 
> ***** FIRST PART: EARLY EXCEPTIONS *****
> 
> The first few patches move some early panic code into C, add pt_regs
> to early exception handling, and make fancy exception handlers work early.
> 
> ***** SECOND PART: MSRs *****
> 
> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.


...

FWIW:

Reviewed-by: Borislav Petkov <bp@suse.de>

Definitely a step in the right direction, regardless of how we're going
to be doing early_printk(), which is a tangential topic. This pile could
be taken for a spin in tip.

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr
       [not found] ` <880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org>
@ 2016-04-04 16:33   ` David Vrabel
       [not found]   ` <5702974A.1030003@citrix.com>
  2016-04-13 11:45   ` [tip:x86/asm] x86/paravirt: Add paravirt_{read, write}_msr() tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: David Vrabel @ 2016-04-04 16:33 UTC (permalink / raw)
  To: Andy Lutomirski, X86 ML
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, linux-kernel,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Linus Torvalds

On 02/04/16 15:01, Andy Lutomirski wrote:
> This adds paravirt hooks for unsafe MSR access.  On native, they
> call native_{read,write}_msr.  On Xen, they use
> xen_{read,write}_msr_safe.
> 
> Nothing uses them yet for ease of bisection.  The next patch will
> use them in rdmsrl, wrmsrl, etc.
> 
> I intentionally didn't make them warn on #GP on Xen.  I think that
> should be done separately by the Xen maintainers.
...
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

> @@ -1092,6 +1092,26 @@ static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
>  	return ret;
>  }
>  
> +static u64 xen_read_msr(unsigned int msr)
> +{
> +	/*
> +	 * This will silently swallow a #GP from RDMSR.  It may be worth
> +	 * changing that.

This probably isn't important.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr
       [not found]   ` <5702974A.1030003@citrix.com>
@ 2016-04-04 16:40     ` Andy Lutomirski
  0 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-04 16:40 UTC (permalink / raw)
  To: David Vrabel
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, X86 ML, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Linus Torvalds

On Mon, Apr 4, 2016 at 9:33 AM, David Vrabel <david.vrabel@citrix.com> wrote:
> On 02/04/16 15:01, Andy Lutomirski wrote:
>> This adds paravirt hooks for unsafe MSR access.  On native, they
>> call native_{read,write}_msr.  On Xen, they use
>> xen_{read,write}_msr_safe.
>>
>> Nothing uses them yet for ease of bisection.  The next patch will
>> use them in rdmsrl, wrmsrl, etc.
>>
>> I intentionally didn't make them warn on #GP on Xen.  I think that
>> should be done separately by the Xen maintainers.
> ...
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
>
>> @@ -1092,6 +1092,26 @@ static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
>>       return ret;
>>  }
>>
>> +static u64 xen_read_msr(unsigned int msr)
>> +{
>> +     /*
>> +      * This will silently swallow a #GP from RDMSR.  It may be worth
>> +      * changing that.
>
> This probably isn't important.
>

It might be nice to do a WARN_ON_ONCE like this series does for the
native case to help shake out latent bugs.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]               ` <20160404160042.GB3448@twins.programming.kicks-ass.net>
@ 2016-04-04 19:38                 ` Borislav Petkov
       [not found]                 ` <20160404193803.GK351@pd.tnic>
  1 sibling, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-04 19:38 UTC (permalink / raw)
  To: Peter Zijlstra, Andy Lutomirski
  Cc: Jan Kara, KVM list, Linus Torvalds, X86 ML, linux-kernel,
	Petr Mladek, xen-devel, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

On Mon, Apr 04, 2016 at 06:00:42PM +0200, Peter Zijlstra wrote:
> On Mon, Apr 04, 2016 at 08:32:21AM -0700, Andy Lutomirski wrote:
> 
> > Adding locking would be easy enough, wouldn't it?
> 
> See patch in this thread..
> 
> > But do any platforms really boot a second CPU before switching to real
> > printk? 
> 
> I _only_ use early_printk() as printk() is a quagmire of fail :-)

And since I'm the king of minimalistic changes... this below
works. However, the problem is that we need to pull up all that
early_param parsing in order to enable the early console, i.e.
"earlyprintk=ttyS0,115200" cmdline parsing.

And we can do all that after having setup the IDT. So I'd need to do
some early dancing with cmdline_find_option_bool(boot_command_line, ...
in asm or so. Need to think about it more.

---
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 1f4422d5c8d0..ad534226653b 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -130,6 +130,13 @@ static void __init copy_bootdata(char *real_mode_data)
 	}
 }
 
+static int _early_printk(const char *fmt, va_list args)
+{
+	early_printk(fmt, args);
+
+	return 0;
+}
+
 asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 {
 	int i;
@@ -164,6 +171,10 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 	load_idt((const struct desc_ptr *)&idt_descr);
 
 	copy_bootdata(__va(real_mode_data));
+	parse_early_param();
+	this_cpu_write(printk_func, _early_printk);
+
+	printk("This is a test!\n");
 
 	/*
 	 * Load microcode early on BSP.
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 2367ae07eb76..998d6c675549 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -882,6 +882,7 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	__flush_tlb_all();
 #else
+	this_cpu_write(printk_func, vprintk_default);
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif
 
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9ccbdf2c1453..97df81c97b2f 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -169,6 +169,7 @@ void __init setup_log_buf(int early);
 __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
 void dump_stack_print_info(const char *log_lvl);
 void show_regs_print_info(const char *log_lvl);
+int vprintk_default(const char *fmt, va_list args);
 #else
 static inline __printf(1, 0)
 int vprintk(const char *s, va_list args)

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]                 ` <20160404193803.GK351@pd.tnic>
@ 2016-04-04 21:31                   ` Andy Lutomirski
       [not found]                   ` <CALCETrVvFTrK1pVmjnfWwwq+5P6+1ghMjh4Wwqm05d=00ejYSw@mail.gmail.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-04 21:31 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jan Kara, KVM list, Peter Zijlstra, Linus Torvalds, X86 ML,
	linux-kernel, Petr Mladek, xen-devel, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Mon, Apr 4, 2016 at 12:38 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Apr 04, 2016 at 06:00:42PM +0200, Peter Zijlstra wrote:
>> On Mon, Apr 04, 2016 at 08:32:21AM -0700, Andy Lutomirski wrote:
>>
>> > Adding locking would be easy enough, wouldn't it?
>>
>> See patch in this thread..
>>
>> > But do any platforms really boot a second CPU before switching to real
>> > printk?
>>
>> I _only_ use early_printk() as printk() is a quagmire of fail :-)
>
> And since I'm the king of minimalistic changes... this below
> works. However, the problem is that we need to pull up all that
> early_param parsing in order to enable the early console, i.e.
> "earlyprintk=ttyS0,115200" cmdline parsing.
>
> And we can do all that after having setup the IDT. So I'd need to do
> some early dancing with cmdline_find_option_bool(boot_command_line, ...
> in asm or so. Need to think about it more.
>

I'd be nervous about moving early param parsing, especially as part of
the same patch.

Could you do it by moving just the earlyprintk stuff a la
fpu__init_parse_early_param()?

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception
       [not found]                   ` <CALCETrVvFTrK1pVmjnfWwwq+5P6+1ghMjh4Wwqm05d=00ejYSw@mail.gmail.com>
@ 2016-04-04 21:40                     ` Borislav Petkov
  0 siblings, 0 replies; 48+ messages in thread
From: Borislav Petkov @ 2016-04-04 21:40 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jan Kara, KVM list, Peter Zijlstra, Linus Torvalds, X86 ML,
	linux-kernel, Petr Mladek, xen-devel, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Mon, Apr 04, 2016 at 02:31:46PM -0700, Andy Lutomirski wrote:
> Could you do it by moving just the earlyprintk stuff a la
> fpu__init_parse_early_param()?

Yeah, something like that. I'll play with this more tomorrow. Btw, no
need to make any of this part of your patchset - I'd like early changes
like that to cook longer for obvious reasons anyway...

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (12 preceding siblings ...)
       [not found] ` <880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org>
@ 2016-04-05 15:38 ` Boris Ostrovsky
       [not found] ` <20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org>
  14 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2016-04-05 15:38 UTC (permalink / raw)
  To: Andy Lutomirski, X86 ML
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, linux-kernel,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Linus Torvalds

On 04/02/2016 10:01 AM, Andy Lutomirski wrote:
>
> Andy Lutomirski (9):
>    x86/head: Pass a real pt_regs and trapnr to early_fixup_exception
>    x86/head: Move the early NMI fixup into C
>    x86/head: Move early exception panic code into early_fixup_exception
>    x86/traps: Enable all exception handler callbacks early
>    x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
>    x86/msr: Carry on after a non-"safe" MSR access fails
>    x86/paravirt: Add paravirt_{read,write}_msr
>    x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
>    x86/msr: Set the return value to zero when native_rdmsr_safe fails

With Xen (on top of eb1af3b):

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception()
  2016-04-02 14:01 ` [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception Andy Lutomirski
@ 2016-04-13 11:43   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, kvm, peterz, torvalds, boris.ostrovsky, linux-kernel,
	Xen-devel, bp, luto, hpa, pbonzini, tglx, arjan, akpm

Commit-ID:  7bbcdb1ca4d2fd69094ee89c18601b396531ca9f
Gitweb:     http://git.kernel.org/tip/7bbcdb1ca4d2fd69094ee89c18601b396531ca9f
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:32 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:44 +0200

x86/head: Pass a real pt_regs and trapnr to early_fixup_exception()

early_fixup_exception() is limited by the fact that it doesn't have a
real struct pt_regs.  Change both the 32-bit and 64-bit asm and the
C code to pass and accept a real pt_regs.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/e3fb680fcfd5e23e38237e8328b64a25cc121d37.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/uaccess.h |  2 +-
 arch/x86/kernel/head_32.S      | 74 +++++++++++++++++++++++++++++-------------
 arch/x86/kernel/head_64.S      | 68 ++++++++++++++++++++------------------
 arch/x86/mm/extable.c          |  6 ++--
 4 files changed, 92 insertions(+), 58 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a969ae6..b6fb311 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -110,7 +110,7 @@ struct exception_table_entry {
 
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
-extern int early_fixup_exception(unsigned long *ip);
+extern int early_fixup_exception(struct pt_regs *regs, int trapnr);
 
 /*
  * These are the main single-value transfer routines.  They automatically
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 54cdbd2..0904536 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -568,29 +568,64 @@ early_idt_handler_common:
 	je hlt_loop
 	incl %ss:early_recursion_flag
 
-	push %eax		# 16(%esp)
-	push %ecx		# 12(%esp)
-	push %edx		#  8(%esp)
-	push %ds		#  4(%esp)
-	push %es		#  0(%esp)
-	movl $(__KERNEL_DS),%eax
-	movl %eax,%ds
-	movl %eax,%es
+	/* The vector number is in pt_regs->gs */
 
-	cmpl $(__KERNEL_CS),32(%esp)
+	cld
+	pushl	%fs		/* pt_regs->fs */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%es		/* pt_regs->es */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%ds		/* pt_regs->ds */
+	movw	$0, 2(%esp)	/* clear high bits (some CPUs leave garbage) */
+	pushl	%eax		/* pt_regs->ax */
+	pushl	%ebp		/* pt_regs->bp */
+	pushl	%edi		/* pt_regs->di */
+	pushl	%esi		/* pt_regs->si */
+	pushl	%edx		/* pt_regs->dx */
+	pushl	%ecx		/* pt_regs->cx */
+	pushl	%ebx		/* pt_regs->bx */
+
+	/* Fix up DS and ES */
+	movl	$(__KERNEL_DS), %ecx
+	movl	%ecx, %ds
+	movl	%ecx, %es
+
+	/* Load the vector number into EDX */
+	movl	PT_GS(%esp), %edx
+
+	/* Load GS into pt_regs->gs and clear high bits */
+	movw	%gs, PT_GS(%esp)
+	movw	$0, PT_GS+2(%esp)
+
+	cmpl $(__KERNEL_CS),PT_CS(%esp)
 	jne 10f
 
-	leal 28(%esp),%eax	# Pointer to %eip
-	call early_fixup_exception
-	andl %eax,%eax
-	jnz ex_entry		/* found an exception entry */
+	movl	%esp, %eax	/* args are pt_regs (EAX), trapnr (EDX) */
+	call	early_fixup_exception
+	andl	%eax,%eax
+	jz	10f		/* Exception wasn't fixed up */
+
+	popl	%ebx		/* pt_regs->bx */
+	popl	%ecx		/* pt_regs->cx */
+	popl	%edx		/* pt_regs->dx */
+	popl	%esi		/* pt_regs->si */
+	popl	%edi		/* pt_regs->di */
+	popl	%ebp		/* pt_regs->bp */
+	popl	%eax		/* pt_regs->ax */
+	popl	%ds		/* pt_regs->ds */
+	popl	%es		/* pt_regs->es */
+	popl	%fs		/* pt_regs->fs */
+	popl	%gs		/* pt_regs->gs */
+	decl	%ss:early_recursion_flag
+	addl	$4, %esp	/* pop pt_regs->orig_ax */
+	iret
 
 10:
 #ifdef CONFIG_PRINTK
 	xorl %eax,%eax
-	movw %ax,2(%esp)	/* clean up the segment values on some cpus */
-	movw %ax,6(%esp)
-	movw %ax,34(%esp)
+	movw %ax,PT_FS+2(%esp)	/* clean up the segment values on some cpus */
+	movw %ax,PT_DS+2(%esp)
+	movw %ax,PT_ES+2(%esp)
 	leal  40(%esp),%eax
 	pushl %eax		/* %esp before the exception */
 	pushl %ebx
@@ -608,13 +643,6 @@ hlt_loop:
 	hlt
 	jmp hlt_loop
 
-ex_entry:
-	pop %es
-	pop %ds
-	pop %edx
-	pop %ecx
-	pop %eax
-	decl %ss:early_recursion_flag
 .Lis_nmi:
 	addl $8,%esp		/* drop vector number and error code */
 	iret
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 22fbf9d..9e8636d 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -20,6 +20,7 @@
 #include <asm/processor-flags.h>
 #include <asm/percpu.h>
 #include <asm/nops.h>
+#include "../entry/calling.h"
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>
@@ -357,39 +358,52 @@ early_idt_handler_common:
 	jz  1f
 	incl early_recursion_flag(%rip)
 
-	pushq %rax		# 64(%rsp)
-	pushq %rcx		# 56(%rsp)
-	pushq %rdx		# 48(%rsp)
-	pushq %rsi		# 40(%rsp)
-	pushq %rdi		# 32(%rsp)
-	pushq %r8		# 24(%rsp)
-	pushq %r9		# 16(%rsp)
-	pushq %r10		#  8(%rsp)
-	pushq %r11		#  0(%rsp)
-
-	cmpl $__KERNEL_CS,96(%rsp)
+	/* The vector number is currently in the pt_regs->di slot. */
+	pushq %rsi				/* pt_regs->si */
+	movq 8(%rsp), %rsi			/* RSI = vector number */
+	movq %rdi, 8(%rsp)			/* pt_regs->di = RDI */
+	pushq %rdx				/* pt_regs->dx */
+	pushq %rcx				/* pt_regs->cx */
+	pushq %rax				/* pt_regs->ax */
+	pushq %r8				/* pt_regs->r8 */
+	pushq %r9				/* pt_regs->r9 */
+	pushq %r10				/* pt_regs->r10 */
+	pushq %r11				/* pt_regs->r11 */
+	pushq %rbx				/* pt_regs->bx */
+	pushq %rbp				/* pt_regs->bp */
+	pushq %r12				/* pt_regs->r12 */
+	pushq %r13				/* pt_regs->r13 */
+	pushq %r14				/* pt_regs->r14 */
+	pushq %r15				/* pt_regs->r15 */
+
+	cmpl $__KERNEL_CS,CS(%rsp)
 	jne 11f
 
-	cmpl $14,72(%rsp)	# Page fault?
+	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f
-	GET_CR2_INTO(%rdi)	# can clobber any volatile register if pv
+	GET_CR2_INTO(%rdi)	/* Can clobber any volatile register if pv */
 	call early_make_pgtable
 	andl %eax,%eax
-	jz 20f			# All good
+	jz 20f			/* All good */
 
 10:
-	leaq 88(%rsp),%rdi	# Pointer to %rip
+	movq %rsp,%rdi		/* RDI = pt_regs; RSI is already trapnr */
 	call early_fixup_exception
 	andl %eax,%eax
 	jnz 20f			# Found an exception entry
 
 11:
 #ifdef CONFIG_EARLY_PRINTK
-	GET_CR2_INTO(%r9)	# can clobber any volatile register if pv
-	movl 80(%rsp),%r8d	# error code
-	movl 72(%rsp),%esi	# vector number
-	movl 96(%rsp),%edx	# %cs
-	movq 88(%rsp),%rcx	# %rip
+	/*
+	 * On paravirt kernels, GET_CR2_INTO clobbers callee-clobbered regs.
+	 * We only care about RSI, so we need to save it.
+	 */
+	movq %rsi,%rbx		/* Save vector number */
+	GET_CR2_INTO(%r9)
+	movq ORIG_RAX(%rsp),%r8	/* error code */
+	movq %rbx,%rsi		/* vector number */
+	movq CS(%rsp),%rdx
+	movq RIP(%rsp),%rcx
 	xorl %eax,%eax
 	leaq early_idt_msg(%rip),%rdi
 	call early_printk
@@ -398,24 +412,16 @@ early_idt_handler_common:
 	call dump_stack
 #ifdef CONFIG_KALLSYMS	
 	leaq early_idt_ripmsg(%rip),%rdi
-	movq 40(%rsp),%rsi	# %rip again
+	movq RIP(%rsp),%rsi	# %rip again
 	call __print_symbol
 #endif
 #endif /* EARLY_PRINTK */
 1:	hlt
 	jmp 1b
 
-20:	# Exception table entry found or page table generated
-	popq %r11
-	popq %r10
-	popq %r9
-	popq %r8
-	popq %rdi
-	popq %rsi
-	popq %rdx
-	popq %rcx
-	popq %rax
+20:	/* Exception table entry found or page table generated */
 	decl early_recursion_flag(%rip)
+	jmp restore_regs_and_iret
 .Lis_nmi:
 	addq $16,%rsp		# drop vector number and error code
 	INTERRUPT_RETURN
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 82447b3..1366e06 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -83,13 +83,13 @@ int fixup_exception(struct pt_regs *regs, int trapnr)
 }
 
 /* Restricted version used during very early boot */
-int __init early_fixup_exception(unsigned long *ip)
+int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *e;
 	unsigned long new_ip;
 	ex_handler_t handler;
 
-	e = search_exception_tables(*ip);
+	e = search_exception_tables(regs->ip);
 	if (!e)
 		return 0;
 
@@ -100,6 +100,6 @@ int __init early_fixup_exception(unsigned long *ip)
 	if (handler != ex_handler_default)
 		return 0;
 
-	*ip = new_ip;
+	regs->ip = new_ip;
 	return 1;
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/head: Move early exception panic code into early_fixup_exception()
  2016-04-02 14:01 ` [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski
  2016-04-02 18:39   ` Borislav Petkov
       [not found]   ` <20160402183919.GA2538@pd.tnic>
@ 2016-04-13 11:44   ` tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: boris.ostrovsky, kvm, peterz, arjan, mingo, linux-kernel,
	Xen-devel, bp, luto, hpa, pbonzini, akpm, torvalds, tglx

Commit-ID:  0e861fbb5bda79b871341ef2a9a8059765cbe8a4
Gitweb:     http://git.kernel.org/tip/0e861fbb5bda79b871341ef2a9a8059765cbe8a4
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:34 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:44 +0200

x86/head: Move early exception panic code into early_fixup_exception()

This removes a bunch of assembly and adds some C code instead.  It
changes the actual printouts on both 32-bit and 64-bit kernels, but
they still seem okay.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/4085070316fc3ab29538d3fcfe282648d1d4ee2e.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/uaccess.h |  2 +-
 arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
 arch/x86/kernel/head_64.S      | 45 ++------------------------------------
 arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
 4 files changed, 32 insertions(+), 93 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index b6fb311..d794fd1 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -110,7 +110,7 @@ struct exception_table_entry {
 
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
-extern int early_fixup_exception(struct pt_regs *regs, int trapnr);
+extern void early_fixup_exception(struct pt_regs *regs, int trapnr);
 
 /*
  * These are the main single-value transfer routines.  They automatically
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 184291c..6770865 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -561,8 +561,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,%ss:early_recursion_flag
-	je hlt_loop
 	incl %ss:early_recursion_flag
 
 	/* The vector number is in pt_regs->gs */
@@ -594,13 +592,8 @@ early_idt_handler_common:
 	movw	%gs, PT_GS(%esp)
 	movw	$0, PT_GS+2(%esp)
 
-	cmpl $(__KERNEL_CS),PT_CS(%esp)
-	jne 10f
-
 	movl	%esp, %eax	/* args are pt_regs (EAX), trapnr (EDX) */
 	call	early_fixup_exception
-	andl	%eax,%eax
-	jz	10f		/* Exception wasn't fixed up */
 
 	popl	%ebx		/* pt_regs->bx */
 	popl	%ecx		/* pt_regs->cx */
@@ -616,29 +609,6 @@ early_idt_handler_common:
 	decl	%ss:early_recursion_flag
 	addl	$4, %esp	/* pop pt_regs->orig_ax */
 	iret
-
-10:
-#ifdef CONFIG_PRINTK
-	xorl %eax,%eax
-	movw %ax,PT_FS+2(%esp)	/* clean up the segment values on some cpus */
-	movw %ax,PT_DS+2(%esp)
-	movw %ax,PT_ES+2(%esp)
-	leal  40(%esp),%eax
-	pushl %eax		/* %esp before the exception */
-	pushl %ebx
-	pushl %ebp
-	pushl %esi
-	pushl %edi
-	movl %cr2,%eax
-	pushl %eax
-	pushl (20+6*4)(%esp)	/* trapno */
-	pushl $fault_msg
-	call printk
-#endif
-	call dump_stack
-hlt_loop:
-	hlt
-	jmp hlt_loop
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
@@ -674,10 +644,14 @@ ignore_int:
 	popl %eax
 #endif
 	iret
+
+hlt_loop:
+	hlt
+	jmp hlt_loop
 ENDPROC(ignore_int)
 __INITDATA
 	.align 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
 __REFDATA
@@ -742,19 +716,6 @@ __INITRODATA
 int_msg:
 	.asciz "Unknown interrupt or fault at: %p %p %p\n"
 
-fault_msg:
-/* fault info: */
-	.ascii "BUG: Int %d: CR2 %p\n"
-/* regs pushed in early_idt_handler: */
-	.ascii "     EDI %p  ESI %p  EBP %p  EBX %p\n"
-	.ascii "     ESP %p   ES %p   DS %p\n"
-	.ascii "     EDX %p  ECX %p  EAX %p\n"
-/* fault frame: */
-	.ascii "     vec %p  err %p  EIP %p   CS %p  flg %p\n"
-	.ascii "Stack: %p %p %p %p %p %p %p %p\n"
-	.ascii "       %p %p %p %p %p %p %p %p\n"
-	.asciz "       %p %p %p %p %p %p %p %p\n"
-
 #include "../../x86/xen/xen-head.S"
 
 /*
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 2308437..3de91a7 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -351,8 +351,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
 	incl early_recursion_flag(%rip)
 
 	/* The vector number is currently in the pt_regs->di slot. */
@@ -373,9 +371,6 @@ early_idt_handler_common:
 	pushq %r14				/* pt_regs->r14 */
 	pushq %r15				/* pt_regs->r15 */
 
-	cmpl $__KERNEL_CS,CS(%rsp)
-	jne 11f
-
 	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f
 	GET_CR2_INTO(%rdi)	/* Can clobber any volatile register if pv */
@@ -386,37 +381,8 @@ early_idt_handler_common:
 10:
 	movq %rsp,%rdi		/* RDI = pt_regs; RSI is already trapnr */
 	call early_fixup_exception
-	andl %eax,%eax
-	jnz 20f			# Found an exception entry
-
-11:
-#ifdef CONFIG_EARLY_PRINTK
-	/*
-	 * On paravirt kernels, GET_CR2_INTO clobbers callee-clobbered regs.
-	 * We only care about RSI, so we need to save it.
-	 */
-	movq %rsi,%rbx		/* Save vector number */
-	GET_CR2_INTO(%r9)
-	movq ORIG_RAX(%rsp),%r8	/* error code */
-	movq %rbx,%rsi		/* vector number */
-	movq CS(%rsp),%rdx
-	movq RIP(%rsp),%rcx
-	xorl %eax,%eax
-	leaq early_idt_msg(%rip),%rdi
-	call early_printk
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
-	call dump_stack
-#ifdef CONFIG_KALLSYMS	
-	leaq early_idt_ripmsg(%rip),%rdi
-	movq RIP(%rsp),%rsi	# %rip again
-	call __print_symbol
-#endif
-#endif /* EARLY_PRINTK */
-1:	hlt
-	jmp 1b
 
-20:	/* Exception table entry found or page table generated */
+20:
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
 ENDPROC(early_idt_handler_common)
@@ -424,16 +390,9 @@ ENDPROC(early_idt_handler_common)
 	__INITDATA
 
 	.balign 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
-#ifdef CONFIG_EARLY_PRINTK
-early_idt_msg:
-	.asciz "PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"
-early_idt_ripmsg:
-	.asciz "RIP %s\n"
-#endif /* CONFIG_EARLY_PRINTK */
-
 #define NEXT_PAGE(name) \
 	.balign	PAGE_SIZE; \
 GLOBAL(name)
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 4be0419..da442f3 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -83,8 +83,10 @@ int fixup_exception(struct pt_regs *regs, int trapnr)
 	return handler(e, regs, trapnr);
 }
 
+extern unsigned int early_recursion_flag;
+
 /* Restricted version used during very early boot */
-int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
+void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *e;
 	unsigned long new_ip;
@@ -92,19 +94,36 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 
 	/* Ignore early NMIs. */
 	if (trapnr == X86_TRAP_NMI)
-		return 1;
+		return;
+
+	if (early_recursion_flag > 2)
+		goto halt_loop;
+
+	if (regs->cs != __KERNEL_CS)
+		goto fail;
 
 	e = search_exception_tables(regs->ip);
 	if (!e)
-		return 0;
+		goto fail;
 
 	new_ip  = ex_fixup_addr(e);
 	handler = ex_fixup_handler(e);
 
 	/* special handling not supported during early boot */
 	if (handler != ex_handler_default)
-		return 0;
+		goto fail;
 
 	regs->ip = new_ip;
-	return 1;
+	return;
+
+fail:
+	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
+		     (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
+		     regs->orig_ax, read_cr2());
+
+	show_regs(regs);
+
+halt_loop:
+	while (true)
+		halt();
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/traps: Enable all exception handler callbacks early
       [not found] ` <20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org>
  2016-04-02 18:52   ` [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early Borislav Petkov
       [not found]   ` <20160402185227.GB2538@pd.tnic>
@ 2016-04-13 11:44   ` tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, akpm, kvm, peterz, torvalds, linux-kernel, Xen-devel, bp,
	luto, hpa, pbonzini, boris.ostrovsky, arjan, mingo

Commit-ID:  ae7ef45e12354a1e2f6013b46df0c9f5bbb6ffbe
Gitweb:     http://git.kernel.org/tip/ae7ef45e12354a1e2f6013b46df0c9f5bbb6ffbe
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:35 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:45 +0200

x86/traps: Enable all exception handler callbacks early

Now that early_fixup_exception() has pt_regs, we can just call
fixup_exception() from it.  This will make fancy exception handlers
work early.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/extable.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index da442f3..061a237 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -88,10 +88,6 @@ extern unsigned int early_recursion_flag;
 /* Restricted version used during very early boot */
 void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
-	const struct exception_table_entry *e;
-	unsigned long new_ip;
-	ex_handler_t handler;
-
 	/* Ignore early NMIs. */
 	if (trapnr == X86_TRAP_NMI)
 		return;
@@ -102,19 +98,8 @@ void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 	if (regs->cs != __KERNEL_CS)
 		goto fail;
 
-	e = search_exception_tables(regs->ip);
-	if (!e)
-		goto fail;
-
-	new_ip  = ex_fixup_addr(e);
-	handler = ex_fixup_handler(e);
-
-	/* special handling not supported during early boot */
-	if (handler != ex_handler_default)
-		goto fail;
-
-	regs->ip = new_ip;
-	return;
+	if (fixup_exception(regs, trapnr))
+		return;
 
 fail:
 	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks
  2016-04-02 14:01 ` [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
  2016-04-03  8:41   ` Borislav Petkov
       [not found]   ` <20160403084129.GB19007@pd.tnic>
@ 2016-04-13 11:44   ` tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, akpm, kvm, peterz, arjan, linux-kernel, Xen-devel, bp,
	luto, hpa, pbonzini, tglx, torvalds, boris.ostrovsky

Commit-ID:  c2ee03b2a94d7ba692cf6206bbe069d5bfcc20ed
Gitweb:     http://git.kernel.org/tip/c2ee03b2a94d7ba692cf6206bbe069d5bfcc20ed
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:36 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:45 +0200

x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks

These callbacks match the _safe variants, so name them accordingly.
This will make room for unsafe PV callbacks.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/9ee3fb6a196a514c93325bdfa15594beecf04876.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
 arch/x86/include/asm/paravirt_types.h |  8 ++++----
 arch/x86/kernel/paravirt.c            |  4 ++--
 arch/x86/xen/enlighten.c              |  4 ++--
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 601f1b8..81ef2d5 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -130,34 +130,35 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
-static inline u64 paravirt_read_msr(unsigned msr, int *err)
+static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
-	return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
+	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
 }
 
-static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
+static inline int paravirt_write_msr_safe(unsigned msr,
+					  unsigned low, unsigned high)
 {
-	return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
+	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
 /* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
 	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr(msr, val1, val2);	\
+	paravirt_write_msr_safe(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
 	int _err;				\
-	val = paravirt_read_msr(msr, &_err);	\
+	val = paravirt_read_msr_safe(msr, &_err);	\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
@@ -165,23 +166,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
 	wrmsr(msr, (u32)val, (u32)(val>>32));
 }
 
-#define wrmsr_safe(msr, a, b)	paravirt_write_msr(msr, a, b)
+#define wrmsr_safe(msr, a, b)	paravirt_write_msr_safe(msr, a, b)
 
 /* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b)			\
-({						\
-	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
-	(*a) = (u32)_l;				\
-	(*b) = _l >> 32;			\
-	_err;					\
+#define rdmsr_safe(msr, a, b)				\
+({							\
+	int _err;					\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	(*a) = (u32)_l;					\
+	(*b) = _l >> 32;				\
+	_err;						\
 })
 
 static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
 {
 	int err;
 
-	*p = paravirt_read_msr(msr, &err);
+	*p = paravirt_read_msr_safe(msr, &err);
 	return err;
 }
 
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index e8c2326..09c9e1d 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,10 +155,10 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR, PMC and TSR operations.
-	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
-	u64 (*read_msr)(unsigned int msr, int *err);
-	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+	/* MSR operations.
+	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	u64 (*read_msr_safe)(unsigned int msr, int *err);
+	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
 	u64 (*read_pmc)(int counter);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28..8aad954 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,8 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
-	.read_msr = native_read_msr_safe,
-	.write_msr = native_write_msr_safe,
+	.read_msr_safe = native_read_msr_safe,
+	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
 	.load_tr_desc = native_load_tr_desc,
 	.set_ldt = native_set_ldt,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 9b8f1ea..13f756f 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1222,8 +1222,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
-	.read_msr = xen_read_msr_safe,
-	.write_msr = xen_write_msr_safe,
+	.read_msr_safe = xen_read_msr_safe,
+	.write_msr_safe = xen_write_msr_safe,
 
 	.read_pmc = xen_read_pmc,
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/paravirt: Add paravirt_{read, write}_msr()
       [not found] ` <880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org>
  2016-04-04 16:33   ` [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr David Vrabel
       [not found]   ` <5702974A.1030003@citrix.com>
@ 2016-04-13 11:45   ` tip-bot for Andy Lutomirski
  2 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: boris.ostrovsky, tglx, kvm, peterz, torvalds, linux-kernel,
	Xen-devel, bp, luto, hpa, pbonzini, akpm, arjan, mingo

Commit-ID:  dd2f4a004b016bbfb64f1de49cb45e66232e40a6
Gitweb:     http://git.kernel.org/tip/dd2f4a004b016bbfb64f1de49cb45e66232e40a6
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:38 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:46 +0200

x86/paravirt: Add paravirt_{read,write}_msr()

This adds paravirt callbacks for unsafe MSR access.  On native, they
call native_{read,write}_msr().  On Xen, they use xen_{read,write}_msr_safe().

Nothing uses them yet for ease of bisection.  The next patch will
use them in rdmsrl(), wrmsrl(), etc.

I intentionally didn't make them warn on #GP on Xen.  I think that
should be done separately by the Xen maintainers.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/msr.h            |  5 +++--
 arch/x86/include/asm/paravirt.h       | 11 +++++++++++
 arch/x86/include/asm/paravirt_types.h | 10 ++++++++--
 arch/x86/kernel/paravirt.c            |  2 ++
 arch/x86/xen/enlighten.c              | 23 +++++++++++++++++++++++
 5 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 25f169c..00050c0 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -111,8 +111,9 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
 	return EAX_EDX_VAL(val, low, high);
 }
 
-static inline void native_write_msr(unsigned int msr,
-				    unsigned low, unsigned high)
+/* Can be uninlined because referenced by paravirt */
+notrace static inline void native_write_msr(unsigned int msr,
+					    unsigned low, unsigned high)
 {
 	asm volatile("1: wrmsr\n"
 		     "2:\n"
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 81ef2d5..97839fa 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -130,6 +130,17 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
+static inline u64 paravirt_read_msr(unsigned msr)
+{
+	return PVOP_CALL1(u64, pv_cpu_ops.read_msr, msr);
+}
+
+static inline void paravirt_write_msr(unsigned msr,
+				      unsigned low, unsigned high)
+{
+	return PVOP_VCALL3(pv_cpu_ops.write_msr, msr, low, high);
+}
+
 static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
 	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 09c9e1d..b4a23ea 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,8 +155,14 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR operations.
-	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	/* Unsafe MSR operations.  These will warn or panic on failure. */
+	u64 (*read_msr)(unsigned int msr);
+	void (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+
+	/*
+	 * Safe MSR operations.
+	 * read sets err to 0 or -EIO.  write returns 0 or -EIO.
+	 */
 	u64 (*read_msr_safe)(unsigned int msr, int *err);
 	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 8aad954..f958391 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,6 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
+	.read_msr = native_read_msr,
+	.write_msr = native_write_msr,
 	.read_msr_safe = native_read_msr_safe,
 	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 13f756f..6ab6722 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1092,6 +1092,26 @@ static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
 	return ret;
 }
 
+static u64 xen_read_msr(unsigned int msr)
+{
+	/*
+	 * This will silently swallow a #GP from RDMSR.  It may be worth
+	 * changing that.
+	 */
+	int err;
+
+	return xen_read_msr_safe(msr, &err);
+}
+
+static void xen_write_msr(unsigned int msr, unsigned low, unsigned high)
+{
+	/*
+	 * This will silently swallow a #GP from WRMSR.  It may be worth
+	 * changing that.
+	 */
+	xen_write_msr_safe(msr, low, high);
+}
+
 void xen_setup_shared_info(void)
 {
 	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
@@ -1222,6 +1242,9 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
+	.read_msr = xen_read_msr,
+	.write_msr = xen_write_msr,
+
 	.read_msr_safe = xen_read_msr_safe,
 	.write_msr_safe = xen_write_msr_safe,
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [tip:x86/asm] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
  2016-04-02 14:01 ` [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
@ 2016-04-13 11:46   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 48+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-04-13 11:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: boris.ostrovsky, kvm, peterz, arjan, mingo, linux-kernel,
	Xen-devel, bp, luto, hpa, pbonzini, tglx, torvalds, akpm

Commit-ID:  4985ce15a397e9b6541548efe3b9ffac2dda9127
Gitweb:     http://git.kernel.org/tip/4985ce15a397e9b6541548efe3b9ffac2dda9127
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:39 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:46 +0200

x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y

Enabling CONFIG_PARAVIRT had an unintended side effect: rdmsr() turned
into rdmsr_safe() and wrmsr() turned into wrmsr_safe(), even on bare
metal.  Undo that by using the new unsafe paravirt MSR callbacks.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/414fabd6d3527703077c6c2a797223d0a9c3b081.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/paravirt.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 97839fa..3c73141 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -152,24 +152,21 @@ static inline int paravirt_write_msr_safe(unsigned msr,
 	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
-/* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
-	int _err;				\
-	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	u64 _l = paravirt_read_msr(msr);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr_safe(msr, val1, val2);	\
+	paravirt_write_msr(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
-	int _err;				\
-	val = paravirt_read_msr_safe(msr, &_err);	\
+	val = paravirt_read_msr(msr);		\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
@ 2016-04-02 14:01 Andy Lutomirski
  0 siblings, 0 replies; 48+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

There are two parts here:

***** FIRST PART: EARLY EXCEPTIONS *****

The first few patches move some early panic code into C, add pt_regs
to early exception handling, and make fancy exception handlers work early.

***** SECOND PART: MSRs *****

Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
turns all rdmsr and wrmsr operations into the safe variants without
any checks that the operations actually succeed.

With CONFIG_PARAVIRT=n, unchecked MSR failures OOPS and probably
cause boot to fail if they happen before init starts.

Neither behavior is very good, and it's particularly unfortunate that
the behavior changes depending on CONFIG_PARAVIRT.

In particular, KVM guests might be unwittingly depending on the
PARAVIRT=y behavior because CONFIG_KVM_GUEST currently depends on
CONFIG_PARAVIRT, and, because accesses in that case are completely
unchecked, we wouldn't even see a warning.

This series changes the native behavior, regardless of
CONFIG_PARAVIRT.  A non-"safe" MSR failure will give an informative
warning once and will be fixed up -- native_read_msr will return
zero, and both reads and writes will continue where they left off.

If panic_on_oops is set, they will still OOPS and panic.

By using the shiny new custom exception handler infrastructure,
there should be no overhead on the success paths.

I didn't change the behavior on Xen, but, with this series applied,
it would be straightforward for the Xen maintainers to make the
corresponding change -- knowledge of whether the access is "safe" is
now propagated into the pvops.

Doing this is probably a prerequisite to sanely decoupling
CONFIG_KVM_GUEST and CONFIG_PARAVIRT, which would probably make
Arjan and the rest of the Clear Containers people happy :)

There's also room to reduce the code size of the "safe" variants
using custom exception handlers in the future.

Changes from v4:
 - Fix a missing \n (Joe Perches)
 - No more panic_on_oops
 - Works early

Changes from v3:
 - WARN_ONCE instead of WARN (Ingo)
 - In the warning text, s/unsafe/unchecked/ (Ingo, sort of)

Changes from earlier versions: lots of changes!

Andy Lutomirski (9):
  x86/head: Pass a real pt_regs and trapnr to early_fixup_exception
  x86/head: Move the early NMI fixup into C
  x86/head: Move early exception panic code into early_fixup_exception
  x86/traps: Enable all exception handler callbacks early
  x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
  x86/msr: Carry on after a non-"safe" MSR access fails
  x86/paravirt: Add paravirt_{read,write}_msr
  x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
  x86/msr: Set the return value to zero when native_rdmsr_safe fails

 arch/x86/include/asm/msr.h            |  20 ++++--
 arch/x86/include/asm/paravirt.h       |  45 +++++++------
 arch/x86/include/asm/paravirt_types.h |  14 ++--
 arch/x86/include/asm/uaccess.h        |   2 +-
 arch/x86/kernel/head_32.S             | 116 ++++++++++++++--------------------
 arch/x86/kernel/head_64.S             |  95 ++++++++--------------------
 arch/x86/kernel/paravirt.c            |   6 +-
 arch/x86/mm/extable.c                 |  64 +++++++++++++++----
 arch/x86/xen/enlighten.c              |  27 +++++++-
 9 files changed, 208 insertions(+), 181 deletions(-)

-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2016-04-13 11:46 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1459605520.git.luto@kernel.org>
2016-04-02 14:01 ` [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception Andy Lutomirski
2016-04-13 11:43   ` [tip:x86/asm] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception() tip-bot for Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 2/9] x86/head: Move the early NMI fixup into C Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski
2016-04-02 18:39   ` Borislav Petkov
     [not found]   ` <20160402183919.GA2538@pd.tnic>
2016-04-02 20:13     ` Andy Lutomirski
     [not found]     ` <CALCETrUJVDpzNQMsY=_jWNOMer+TdW2p4wES4OBCfYU7gq3fbg@mail.gmail.com>
2016-04-02 20:47       ` Borislav Petkov
     [not found]       ` <20160402204752.GC2538@pd.tnic>
2016-04-02 20:58         ` Andy Lutomirski
     [not found]         ` <CALCETrXHR3T3PJwOWRFHTMK5WjhDyTJfaJtkiHd+g7evnqp54A@mail.gmail.com>
2016-04-04 11:52           ` Jan Kara
     [not found]           ` <20160404115206.GG8372@quack.suse.cz>
2016-04-04 12:46             ` Peter Zijlstra
2016-04-04 15:32             ` Andy Lutomirski
     [not found]             ` <CALCETrVriftUPNZ8pCQ1x9TWDHnRx8fWUvJgCQToPEOxqMLb3w@mail.gmail.com>
2016-04-04 15:36               ` Arjan van de Ven
2016-04-04 16:00               ` Peter Zijlstra
     [not found]               ` <20160404160042.GB3448@twins.programming.kicks-ass.net>
2016-04-04 19:38                 ` Borislav Petkov
     [not found]                 ` <20160404193803.GK351@pd.tnic>
2016-04-04 21:31                   ` Andy Lutomirski
     [not found]                   ` <CALCETrVvFTrK1pVmjnfWwwq+5P6+1ghMjh4Wwqm05d=00ejYSw@mail.gmail.com>
2016-04-04 21:40                     ` Borislav Petkov
2016-04-13 11:44   ` [tip:x86/asm] x86/head: Move early exception panic code into early_fixup_exception() tip-bot for Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
2016-04-03  8:41   ` Borislav Petkov
     [not found]   ` <20160403084129.GB19007@pd.tnic>
2016-04-03 13:23     ` Andy Lutomirski
     [not found]     ` <CALCETrUcqTx00pKYEt49QSKKBKVEx-Uv0D6dzE1-Em6Q-wkSpQ@mail.gmail.com>
2016-04-03 14:07       ` Borislav Petkov
2016-04-13 11:44   ` [tip:x86/asm] x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks tip-bot for Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 6/9] x86/msr: Carry on after a non-"safe" MSR access fails Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
2016-04-13 11:46   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2016-04-02 14:01 ` [PATCH v5 9/9] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski
2016-04-02 14:24 ` [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Linus Torvalds
     [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
2016-04-02 15:13   ` Andy Lutomirski
     [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
2016-04-02 15:21     ` Linus Torvalds
2016-04-04 16:23 ` Borislav Petkov
     [not found] ` <880eebc5dcd2ad9f310d41345f82061ea500e9fa.1459605520.git.luto@kernel.org>
2016-04-04 16:33   ` [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr David Vrabel
     [not found]   ` <5702974A.1030003@citrix.com>
2016-04-04 16:40     ` Andy Lutomirski
2016-04-13 11:45   ` [tip:x86/asm] x86/paravirt: Add paravirt_{read, write}_msr() tip-bot for Andy Lutomirski
2016-04-05 15:38 ` [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Boris Ostrovsky
     [not found] ` <20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org>
2016-04-02 18:52   ` [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early Borislav Petkov
     [not found]   ` <20160402185227.GB2538@pd.tnic>
2016-04-02 20:16     ` Andy Lutomirski
     [not found]     ` <CALCETrV7PxgmhU_yXjJSugY88T8Do=vBDuG54fOFSyLp7nGcJA@mail.gmail.com>
2016-04-02 20:52       ` Borislav Petkov
     [not found]       ` <20160402205248.GD2538@pd.tnic>
2016-04-03  8:07         ` Borislav Petkov
     [not found]         ` <20160403080737.GA19007@pd.tnic>
2016-04-03 13:22           ` Andy Lutomirski
2016-04-03 13:51           ` Linus Torvalds
     [not found]           ` <CA+55aFw19X7PO2xd1PpUav1rSyCqUqWgi9+SA7TgjaWMaxexHQ@mail.gmail.com>
2016-04-03 13:55             ` Andy Lutomirski
     [not found]             ` <CALCETrX_x+tV4m3SwgCfXg9y5gia8eMhhTdUnHuXpXhnULkHRw@mail.gmail.com>
2016-04-03 14:10               ` Borislav Petkov
2016-04-03 14:17               ` Linus Torvalds
     [not found]               ` <20160403141041.GE19007@pd.tnic>
2016-04-04 15:47                 ` Andy Lutomirski
2016-04-13 11:44   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2016-04-02 14:01 [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Andy Lutomirski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).