* [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test
@ 2019-07-11 8:15 ` Zhenzhong Duan
0 siblings, 0 replies; 20+ messages in thread
From: Zhenzhong Duan @ 2019-07-11 8:15 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86,
Zhenzhong Duan, srinivas.eeda, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner
Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call()
selftest") is used to ensure there is a gap setup in exception stack
which could be used for inserting call return address.
This gap is missed in XEN PV int3 exception entry path, then below panic
triggered:
[ 0.772876] general protection fault: 0000 [#1] SMP NOPTI
[ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11
[ 0.772893] RIP: e030:int3_magic+0x0/0x7
[ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246
[ 0.773334] Call Trace:
[ 0.773334] alternative_instructions+0x3d/0x12e
[ 0.773334] check_bugs+0x7c9/0x887
[ 0.773334] ? __get_locked_pte+0x178/0x1f0
[ 0.773334] start_kernel+0x4ff/0x535
[ 0.773334] ? set_init_arg+0x55/0x55
[ 0.773334] xen_start_kernel+0x571/0x57a
As xenint3 and int3 entry code are same except xenint3 doesn't generate
a gap, we can fix it by using int3 and drop useless xenint3.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
---
v2: fix up description.
---
arch/x86/entry/entry_64.S | 1 -
arch/x86/include/asm/traps.h | 2 +-
arch/x86/xen/enlighten_pv.c | 2 +-
arch/x86/xen/xen-asm_64.S | 1 -
4 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 0ea4831..35a66fc 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1
#ifdef CONFIG_XEN_PV
idtentry xennmi do_nmi has_error_code=0
idtentry xendebug do_debug has_error_code=0
-idtentry xenint3 do_int3 has_error_code=0
#endif
idtentry general_protection do_general_protection has_error_code=1
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 7d6f3f3..f2bd284 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -40,7 +40,7 @@
asmlinkage void xen_divide_error(void);
asmlinkage void xen_xennmi(void);
asmlinkage void xen_xendebug(void);
-asmlinkage void xen_xenint3(void);
+asmlinkage void xen_int3(void);
asmlinkage void xen_overflow(void);
asmlinkage void xen_bounds(void);
asmlinkage void xen_invalid_op(void);
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4722ba2..2138d69 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -596,7 +596,7 @@ struct trap_array_entry {
static struct trap_array_entry trap_array[] = {
{ debug, xen_xendebug, true },
- { int3, xen_xenint3, true },
+ { int3, xen_int3, true },
{ double_fault, xen_double_fault, true },
#ifdef CONFIG_X86_MCE
{ machine_check, xen_machine_check, true },
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 1e9ef0b..ebf610b 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -32,7 +32,6 @@ xen_pv_trap divide_error
xen_pv_trap debug
xen_pv_trap xendebug
xen_pv_trap int3
-xen_pv_trap xenint3
xen_pv_trap xennmi
xen_pv_trap overflow
xen_pv_trap bounds
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-11 8:15 ` Zhenzhong Duan 0 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-11 8:15 UTC (permalink / raw) To: linux-kernel Cc: xen-devel, x86, srinivas.eeda, Zhenzhong Duan, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest") is used to ensure there is a gap setup in exception stack which could be used for inserting call return address. This gap is missed in XEN PV int3 exception entry path, then below panic triggered: [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 [ 0.772893] RIP: e030:int3_magic+0x0/0x7 [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 [ 0.773334] Call Trace: [ 0.773334] alternative_instructions+0x3d/0x12e [ 0.773334] check_bugs+0x7c9/0x887 [ 0.773334] ? __get_locked_pte+0x178/0x1f0 [ 0.773334] start_kernel+0x4ff/0x535 [ 0.773334] ? set_init_arg+0x55/0x55 [ 0.773334] xen_start_kernel+0x571/0x57a As xenint3 and int3 entry code are same except xenint3 doesn't generate a gap, we can fix it by using int3 and drop useless xenint3. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Juergen Gross <jgross@suse.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> --- v2: fix up description. --- arch/x86/entry/entry_64.S | 1 - arch/x86/include/asm/traps.h | 2 +- arch/x86/xen/enlighten_pv.c | 2 +- arch/x86/xen/xen-asm_64.S | 1 - 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 0ea4831..35a66fc 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 #ifdef CONFIG_XEN_PV idtentry xennmi do_nmi has_error_code=0 idtentry xendebug do_debug has_error_code=0 -idtentry xenint3 do_int3 has_error_code=0 #endif idtentry general_protection do_general_protection has_error_code=1 diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h index 7d6f3f3..f2bd284 100644 --- a/arch/x86/include/asm/traps.h +++ b/arch/x86/include/asm/traps.h @@ -40,7 +40,7 @@ asmlinkage void xen_divide_error(void); asmlinkage void xen_xennmi(void); asmlinkage void xen_xendebug(void); -asmlinkage void xen_xenint3(void); +asmlinkage void xen_int3(void); asmlinkage void xen_overflow(void); asmlinkage void xen_bounds(void); asmlinkage void xen_invalid_op(void); diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 4722ba2..2138d69 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -596,7 +596,7 @@ struct trap_array_entry { static struct trap_array_entry trap_array[] = { { debug, xen_xendebug, true }, - { int3, xen_xenint3, true }, + { int3, xen_int3, true }, { double_fault, xen_double_fault, true }, #ifdef CONFIG_X86_MCE { machine_check, xen_machine_check, true }, diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S index 1e9ef0b..ebf610b 100644 --- a/arch/x86/xen/xen-asm_64.S +++ b/arch/x86/xen/xen-asm_64.S @@ -32,7 +32,6 @@ xen_pv_trap divide_error xen_pv_trap debug xen_pv_trap xendebug xen_pv_trap int3 -xen_pv_trap xenint3 xen_pv_trap xennmi xen_pv_trap overflow xen_pv_trap bounds -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-11 8:15 ` Zhenzhong Duan @ 2019-07-12 12:06 ` Peter Zijlstra -1 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 12:06 UTC (permalink / raw) To: Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, x86, linux-kernel, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index 4722ba2..2138d69 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -596,7 +596,7 @@ struct trap_array_entry { > > static struct trap_array_entry trap_array[] = { > { debug, xen_xendebug, true }, > - { int3, xen_xenint3, true }, > + { int3, xen_int3, true }, > { double_fault, xen_double_fault, true }, > #ifdef CONFIG_X86_MCE > { machine_check, xen_machine_check, true }, I'm confused on the purpose of trap_array[], could you elucidate me? The sole user seems to be get_trap_addr() and that talks about ISTs, but #BP isn't an IST anymore, so why does it have ist_okay=true? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 12:06 ` Peter Zijlstra 0 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 12:06 UTC (permalink / raw) To: Zhenzhong Duan Cc: linux-kernel, xen-devel, x86, srinivas.eeda, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index 4722ba2..2138d69 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -596,7 +596,7 @@ struct trap_array_entry { > > static struct trap_array_entry trap_array[] = { > { debug, xen_xendebug, true }, > - { int3, xen_xenint3, true }, > + { int3, xen_int3, true }, > { double_fault, xen_double_fault, true }, > #ifdef CONFIG_X86_MCE > { machine_check, xen_machine_check, true }, I'm confused on the purpose of trap_array[], could you elucidate me? The sole user seems to be get_trap_addr() and that talks about ISTs, but #BP isn't an IST anymore, so why does it have ist_okay=true? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-12 12:06 ` Peter Zijlstra @ 2019-07-12 13:04 ` Zhenzhong Duan -1 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-12 13:04 UTC (permalink / raw) To: Peter Zijlstra Cc: Juergen Gross, Stefano Stabellini, x86, linux-kernel, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 2019/7/12 20:06, Peter Zijlstra wrote: > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: >> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >> index 4722ba2..2138d69 100644 >> --- a/arch/x86/xen/enlighten_pv.c >> +++ b/arch/x86/xen/enlighten_pv.c >> @@ -596,7 +596,7 @@ struct trap_array_entry { >> >> static struct trap_array_entry trap_array[] = { >> { debug, xen_xendebug, true }, >> - { int3, xen_xenint3, true }, >> + { int3, xen_int3, true }, >> { double_fault, xen_double_fault, true }, >> #ifdef CONFIG_X86_MCE >> { machine_check, xen_machine_check, true }, > I'm confused on the purpose of trap_array[], could you elucidate me? Used to replace trap handler addresses by Xen specific ones and sanity check if there's an unexpected IST-using fault handler. > > The sole user seems to be get_trap_addr() and that talks about ISTs, but > #BP isn't an IST anymore, so why does it have ist_okay=true? Oh, yes, I missed that boolean, thanks. I'll try ist_okey=false for int3 and test it tomorrow. Zhenzhong _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 13:04 ` Zhenzhong Duan 0 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-12 13:04 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, xen-devel, x86, srinivas.eeda, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov On 2019/7/12 20:06, Peter Zijlstra wrote: > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: >> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >> index 4722ba2..2138d69 100644 >> --- a/arch/x86/xen/enlighten_pv.c >> +++ b/arch/x86/xen/enlighten_pv.c >> @@ -596,7 +596,7 @@ struct trap_array_entry { >> >> static struct trap_array_entry trap_array[] = { >> { debug, xen_xendebug, true }, >> - { int3, xen_xenint3, true }, >> + { int3, xen_int3, true }, >> { double_fault, xen_double_fault, true }, >> #ifdef CONFIG_X86_MCE >> { machine_check, xen_machine_check, true }, > I'm confused on the purpose of trap_array[], could you elucidate me? Used to replace trap handler addresses by Xen specific ones and sanity check if there's an unexpected IST-using fault handler. > > The sole user seems to be get_trap_addr() and that talks about ISTs, but > #BP isn't an IST anymore, so why does it have ist_okay=true? Oh, yes, I missed that boolean, thanks. I'll try ist_okey=false for int3 and test it tomorrow. Zhenzhong ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-12 13:04 ` Zhenzhong Duan @ 2019-07-12 13:09 ` Peter Zijlstra -1 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 13:09 UTC (permalink / raw) To: Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, x86, linux-kernel, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: > > On 2019/7/12 20:06, Peter Zijlstra wrote: > > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > > > index 4722ba2..2138d69 100644 > > > --- a/arch/x86/xen/enlighten_pv.c > > > +++ b/arch/x86/xen/enlighten_pv.c > > > @@ -596,7 +596,7 @@ struct trap_array_entry { > > > static struct trap_array_entry trap_array[] = { > > > { debug, xen_xendebug, true }, > > > - { int3, xen_xenint3, true }, > > > + { int3, xen_int3, true }, > > > { double_fault, xen_double_fault, true }, > > > #ifdef CONFIG_X86_MCE > > > { machine_check, xen_machine_check, true }, > > I'm confused on the purpose of trap_array[], could you elucidate me? > > Used to replace trap handler addresses by Xen specific ones and sanity check > > if there's an unexpected IST-using fault handler. git grep xen_int3, failed me. Where does that symbol come from? > > The sole user seems to be get_trap_addr() and that talks about ISTs, but > > #BP isn't an IST anymore, so why does it have ist_okay=true? > > Oh, yes, I missed that boolean, thanks. I'll try ist_okey=false for int3 and > test it tomorrow. Thanks! _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 13:09 ` Peter Zijlstra 0 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 13:09 UTC (permalink / raw) To: Zhenzhong Duan Cc: linux-kernel, xen-devel, x86, srinivas.eeda, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: > > On 2019/7/12 20:06, Peter Zijlstra wrote: > > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > > > index 4722ba2..2138d69 100644 > > > --- a/arch/x86/xen/enlighten_pv.c > > > +++ b/arch/x86/xen/enlighten_pv.c > > > @@ -596,7 +596,7 @@ struct trap_array_entry { > > > static struct trap_array_entry trap_array[] = { > > > { debug, xen_xendebug, true }, > > > - { int3, xen_xenint3, true }, > > > + { int3, xen_int3, true }, > > > { double_fault, xen_double_fault, true }, > > > #ifdef CONFIG_X86_MCE > > > { machine_check, xen_machine_check, true }, > > I'm confused on the purpose of trap_array[], could you elucidate me? > > Used to replace trap handler addresses by Xen specific ones and sanity check > > if there's an unexpected IST-using fault handler. git grep xen_int3, failed me. Where does that symbol come from? > > The sole user seems to be get_trap_addr() and that talks about ISTs, but > > #BP isn't an IST anymore, so why does it have ist_okay=true? > > Oh, yes, I missed that boolean, thanks. I'll try ist_okey=false for int3 and > test it tomorrow. Thanks! ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-12 13:09 ` Peter Zijlstra @ 2019-07-12 13:17 ` Peter Zijlstra -1 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 13:17 UTC (permalink / raw) To: Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, x86, linux-kernel, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On Fri, Jul 12, 2019 at 03:09:16PM +0200, Peter Zijlstra wrote: > On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: > > > > On 2019/7/12 20:06, Peter Zijlstra wrote: > > > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > > > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > > > > index 4722ba2..2138d69 100644 > > > > --- a/arch/x86/xen/enlighten_pv.c > > > > +++ b/arch/x86/xen/enlighten_pv.c > > > > @@ -596,7 +596,7 @@ struct trap_array_entry { > > > > static struct trap_array_entry trap_array[] = { > > > > { debug, xen_xendebug, true }, > > > > - { int3, xen_xenint3, true }, > > > > + { int3, xen_int3, true }, > > > > { double_fault, xen_double_fault, true }, > > > > #ifdef CONFIG_X86_MCE > > > > { machine_check, xen_machine_check, true }, > > > I'm confused on the purpose of trap_array[], could you elucidate me? > > > > Used to replace trap handler addresses by Xen specific ones and sanity check > > > > if there's an unexpected IST-using fault handler. > > git grep xen_int3, failed me. Where does that symbol come from? N/m I found it... must be blind today. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 13:17 ` Peter Zijlstra 0 siblings, 0 replies; 20+ messages in thread From: Peter Zijlstra @ 2019-07-12 13:17 UTC (permalink / raw) To: Zhenzhong Duan Cc: linux-kernel, xen-devel, x86, srinivas.eeda, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov On Fri, Jul 12, 2019 at 03:09:16PM +0200, Peter Zijlstra wrote: > On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: > > > > On 2019/7/12 20:06, Peter Zijlstra wrote: > > > On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: > > > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > > > > index 4722ba2..2138d69 100644 > > > > --- a/arch/x86/xen/enlighten_pv.c > > > > +++ b/arch/x86/xen/enlighten_pv.c > > > > @@ -596,7 +596,7 @@ struct trap_array_entry { > > > > static struct trap_array_entry trap_array[] = { > > > > { debug, xen_xendebug, true }, > > > > - { int3, xen_xenint3, true }, > > > > + { int3, xen_int3, true }, > > > > { double_fault, xen_double_fault, true }, > > > > #ifdef CONFIG_X86_MCE > > > > { machine_check, xen_machine_check, true }, > > > I'm confused on the purpose of trap_array[], could you elucidate me? > > > > Used to replace trap handler addresses by Xen specific ones and sanity check > > > > if there's an unexpected IST-using fault handler. > > git grep xen_int3, failed me. Where does that symbol come from? N/m I found it... must be blind today. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-12 13:09 ` Peter Zijlstra @ 2019-07-12 13:27 ` Zhenzhong Duan -1 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-12 13:27 UTC (permalink / raw) To: Peter Zijlstra Cc: Juergen Gross, Stefano Stabellini, x86, linux-kernel, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 2019/7/12 21:09, Peter Zijlstra wrote: > On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: >> On 2019/7/12 20:06, Peter Zijlstra wrote: >>> On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: >>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >>>> index 4722ba2..2138d69 100644 >>>> --- a/arch/x86/xen/enlighten_pv.c >>>> +++ b/arch/x86/xen/enlighten_pv.c >>>> @@ -596,7 +596,7 @@ struct trap_array_entry { >>>> static struct trap_array_entry trap_array[] = { >>>> { debug, xen_xendebug, true }, >>>> - { int3, xen_xenint3, true }, >>>> + { int3, xen_int3, true }, >>>> { double_fault, xen_double_fault, true }, >>>> #ifdef CONFIG_X86_MCE >>>> { machine_check, xen_machine_check, true }, >>> I'm confused on the purpose of trap_array[], could you elucidate me? >> Used to replace trap handler addresses by Xen specific ones and sanity check >> >> if there's an unexpected IST-using fault handler. > git grep xen_int3, failed me. Where does that symbol come from? Generated by "xen_pv_trap int3" in arch/x86/xen/xen-asm_64.S Zhenzhong _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 13:27 ` Zhenzhong Duan 0 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-12 13:27 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, xen-devel, x86, srinivas.eeda, Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov On 2019/7/12 21:09, Peter Zijlstra wrote: > On Fri, Jul 12, 2019 at 09:04:22PM +0800, Zhenzhong Duan wrote: >> On 2019/7/12 20:06, Peter Zijlstra wrote: >>> On Thu, Jul 11, 2019 at 04:15:21PM +0800, Zhenzhong Duan wrote: >>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >>>> index 4722ba2..2138d69 100644 >>>> --- a/arch/x86/xen/enlighten_pv.c >>>> +++ b/arch/x86/xen/enlighten_pv.c >>>> @@ -596,7 +596,7 @@ struct trap_array_entry { >>>> static struct trap_array_entry trap_array[] = { >>>> { debug, xen_xendebug, true }, >>>> - { int3, xen_xenint3, true }, >>>> + { int3, xen_int3, true }, >>>> { double_fault, xen_double_fault, true }, >>>> #ifdef CONFIG_X86_MCE >>>> { machine_check, xen_machine_check, true }, >>> I'm confused on the purpose of trap_array[], could you elucidate me? >> Used to replace trap handler addresses by Xen specific ones and sanity check >> >> if there's an unexpected IST-using fault handler. > git grep xen_int3, failed me. Where does that symbol come from? Generated by "xen_pv_trap int3" in arch/x86/xen/xen-asm_64.S Zhenzhong ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-11 8:15 ` Zhenzhong Duan @ 2019-07-12 14:06 ` Andrew Cooper -1 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2019-07-12 14:06 UTC (permalink / raw) To: Zhenzhong Duan, linux-kernel Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 11/07/2019 03:15, Zhenzhong Duan wrote: > Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() > selftest") is used to ensure there is a gap setup in exception stack > which could be used for inserting call return address. > > This gap is missed in XEN PV int3 exception entry path, then below panic > triggered: > > [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI > [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 > [ 0.772893] RIP: e030:int3_magic+0x0/0x7 > [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 > [ 0.773334] Call Trace: > [ 0.773334] alternative_instructions+0x3d/0x12e > [ 0.773334] check_bugs+0x7c9/0x887 > [ 0.773334] ? __get_locked_pte+0x178/0x1f0 > [ 0.773334] start_kernel+0x4ff/0x535 > [ 0.773334] ? set_init_arg+0x55/0x55 > [ 0.773334] xen_start_kernel+0x571/0x57a > > As xenint3 and int3 entry code are same except xenint3 doesn't generate > a gap, we can fix it by using int3 and drop useless xenint3. For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with %rcx/%r11 on the stack. To convert back to "normal" looking exceptions, the xen thunks do `pop %rcx; pop %r11; jmp do_*`... > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index 0ea4831..35a66fc 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 > #ifdef CONFIG_XEN_PV > idtentry xennmi do_nmi has_error_code=0 > idtentry xendebug do_debug has_error_code=0 > -idtentry xenint3 do_int3 has_error_code=0 > #endif What is confusing is why there are 3 extra magic versions here. At a guess, I'd say something to do with IST settings (given the vectors), but I don't see why #DB/#BP would need to be different in the first place. (NMI sure, but that is more to do with the crazy hoops needing to be jumped through to keep native functioning safely.) ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-12 14:06 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2019-07-12 14:06 UTC (permalink / raw) To: Zhenzhong Duan, linux-kernel Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 11/07/2019 03:15, Zhenzhong Duan wrote: > Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() > selftest") is used to ensure there is a gap setup in exception stack > which could be used for inserting call return address. > > This gap is missed in XEN PV int3 exception entry path, then below panic > triggered: > > [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI > [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 > [ 0.772893] RIP: e030:int3_magic+0x0/0x7 > [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 > [ 0.773334] Call Trace: > [ 0.773334] alternative_instructions+0x3d/0x12e > [ 0.773334] check_bugs+0x7c9/0x887 > [ 0.773334] ? __get_locked_pte+0x178/0x1f0 > [ 0.773334] start_kernel+0x4ff/0x535 > [ 0.773334] ? set_init_arg+0x55/0x55 > [ 0.773334] xen_start_kernel+0x571/0x57a > > As xenint3 and int3 entry code are same except xenint3 doesn't generate > a gap, we can fix it by using int3 and drop useless xenint3. For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with %rcx/%r11 on the stack. To convert back to "normal" looking exceptions, the xen thunks do `pop %rcx; pop %r11; jmp do_*`... > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index 0ea4831..35a66fc 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 > #ifdef CONFIG_XEN_PV > idtentry xennmi do_nmi has_error_code=0 > idtentry xendebug do_debug has_error_code=0 > -idtentry xenint3 do_int3 has_error_code=0 > #endif What is confusing is why there are 3 extra magic versions here. At a guess, I'd say something to do with IST settings (given the vectors), but I don't see why #DB/#BP would need to be different in the first place. (NMI sure, but that is more to do with the crazy hoops needing to be jumped through to keep native functioning safely.) ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-12 14:06 ` Andrew Cooper @ 2019-07-15 5:05 ` Zhenzhong Duan -1 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-15 5:05 UTC (permalink / raw) To: Andrew Cooper, linux-kernel Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 2019/7/12 22:06, Andrew Cooper wrote: > On 11/07/2019 03:15, Zhenzhong Duan wrote: >> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >> selftest") is used to ensure there is a gap setup in exception stack >> which could be used for inserting call return address. >> >> This gap is missed in XEN PV int3 exception entry path, then below panic >> triggered: >> >> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >> [ 0.773334] Call Trace: >> [ 0.773334] alternative_instructions+0x3d/0x12e >> [ 0.773334] check_bugs+0x7c9/0x887 >> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >> [ 0.773334] start_kernel+0x4ff/0x535 >> [ 0.773334] ? set_init_arg+0x55/0x55 >> [ 0.773334] xen_start_kernel+0x571/0x57a >> >> As xenint3 and int3 entry code are same except xenint3 doesn't generate >> a gap, we can fix it by using int3 and drop useless xenint3. > For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with > %rcx/%r11 on the stack. > > To convert back to "normal" looking exceptions, the xen thunks do `pop > %rcx; pop %r11; jmp do_*`... I will add this to commit message. > >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >> index 0ea4831..35a66fc 100644 >> --- a/arch/x86/entry/entry_64.S >> +++ b/arch/x86/entry/entry_64.S >> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >> #ifdef CONFIG_XEN_PV >> idtentry xennmi do_nmi has_error_code=0 >> idtentry xendebug do_debug has_error_code=0 >> -idtentry xenint3 do_int3 has_error_code=0 >> #endif > What is confusing is why there are 3 extra magic versions here. At a > guess, I'd say something to do with IST settings (given the vectors), > but I don't see why #DB/#BP would need to be different in the first > place. (NMI sure, but that is more to do with the crazy hoops needing > to be jumped through to keep native functioning safely.) xenint3 will be removed in this patch safely as it don't use IST now. But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? Zhenzhong _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-15 5:05 ` Zhenzhong Duan 0 siblings, 0 replies; 20+ messages in thread From: Zhenzhong Duan @ 2019-07-15 5:05 UTC (permalink / raw) To: Andrew Cooper, linux-kernel Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86, srinivas.eeda, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel, Boris Ostrovsky, Thomas Gleixner On 2019/7/12 22:06, Andrew Cooper wrote: > On 11/07/2019 03:15, Zhenzhong Duan wrote: >> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >> selftest") is used to ensure there is a gap setup in exception stack >> which could be used for inserting call return address. >> >> This gap is missed in XEN PV int3 exception entry path, then below panic >> triggered: >> >> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >> [ 0.773334] Call Trace: >> [ 0.773334] alternative_instructions+0x3d/0x12e >> [ 0.773334] check_bugs+0x7c9/0x887 >> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >> [ 0.773334] start_kernel+0x4ff/0x535 >> [ 0.773334] ? set_init_arg+0x55/0x55 >> [ 0.773334] xen_start_kernel+0x571/0x57a >> >> As xenint3 and int3 entry code are same except xenint3 doesn't generate >> a gap, we can fix it by using int3 and drop useless xenint3. > For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with > %rcx/%r11 on the stack. > > To convert back to "normal" looking exceptions, the xen thunks do `pop > %rcx; pop %r11; jmp do_*`... I will add this to commit message. > >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >> index 0ea4831..35a66fc 100644 >> --- a/arch/x86/entry/entry_64.S >> +++ b/arch/x86/entry/entry_64.S >> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >> #ifdef CONFIG_XEN_PV >> idtentry xennmi do_nmi has_error_code=0 >> idtentry xendebug do_debug has_error_code=0 >> -idtentry xenint3 do_int3 has_error_code=0 >> #endif > What is confusing is why there are 3 extra magic versions here. At a > guess, I'd say something to do with IST settings (given the vectors), > but I don't see why #DB/#BP would need to be different in the first > place. (NMI sure, but that is more to do with the crazy hoops needing > to be jumped through to keep native functioning safely.) xenint3 will be removed in this patch safely as it don't use IST now. But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? Zhenzhong ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-15 5:05 ` Zhenzhong Duan @ 2019-07-15 6:54 ` Jan Beulich -1 siblings, 0 replies; 20+ messages in thread From: Jan Beulich @ 2019-07-15 6:54 UTC (permalink / raw) To: Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, Andrew Cooper, x86@kernel.org, linux-kernel@vger.kernel.org, srinivas.eeda@oracle.com, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel@lists.xenproject.org, Thomas Gleixner, Boris Ostrovsky On 15.07.2019 07:05, Zhenzhong Duan wrote: > > On 2019/7/12 22:06, Andrew Cooper wrote: >> On 11/07/2019 03:15, Zhenzhong Duan wrote: >>> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >>> selftest") is used to ensure there is a gap setup in exception stack >>> which could be used for inserting call return address. >>> >>> This gap is missed in XEN PV int3 exception entry path, then below panic >>> triggered: >>> >>> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >>> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >>> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >>> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >>> [ 0.773334] Call Trace: >>> [ 0.773334] alternative_instructions+0x3d/0x12e >>> [ 0.773334] check_bugs+0x7c9/0x887 >>> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >>> [ 0.773334] start_kernel+0x4ff/0x535 >>> [ 0.773334] ? set_init_arg+0x55/0x55 >>> [ 0.773334] xen_start_kernel+0x571/0x57a >>> >>> As xenint3 and int3 entry code are same except xenint3 doesn't generate >>> a gap, we can fix it by using int3 and drop useless xenint3. >> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with >> %rcx/%r11 on the stack. >> >> To convert back to "normal" looking exceptions, the xen thunks do `pop >> %rcx; pop %r11; jmp do_*`... > I will add this to commit message. >> >>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >>> index 0ea4831..35a66fc 100644 >>> --- a/arch/x86/entry/entry_64.S >>> +++ b/arch/x86/entry/entry_64.S >>> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >>> #ifdef CONFIG_XEN_PV >>> idtentry xennmi do_nmi has_error_code=0 >>> idtentry xendebug do_debug has_error_code=0 >>> -idtentry xenint3 do_int3 has_error_code=0 >>> #endif >> What is confusing is why there are 3 extra magic versions here. At a >> guess, I'd say something to do with IST settings (given the vectors), >> but I don't see why #DB/#BP would need to be different in the first >> place. (NMI sure, but that is more to do with the crazy hoops needing >> to be jumped through to keep native functioning safely.) > > xenint3 will be removed in this patch safely as it don't use IST now. > > But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine > > if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit PV kernel mode always gets entered with kernel GS base already set. Hence finding out whether to switch the GS base is specifically not something that any exception entry point would need to do (and it should actively try to avoid it, for performance reasons). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-15 6:54 ` Jan Beulich 0 siblings, 0 replies; 20+ messages in thread From: Jan Beulich @ 2019-07-15 6:54 UTC (permalink / raw) To: Zhenzhong Duan Cc: Andrew Cooper, linux-kernel@vger.kernel.org, Borislav Petkov, Peter Zijlstra, Andy Lutomirski, Stefano Stabellini, x86@kernel.org, Thomas Gleixner, xen-devel@lists.xenproject.org, Boris Ostrovsky, srinivas.eeda@oracle.com, Ingo Molnar, Juergen Gross On 15.07.2019 07:05, Zhenzhong Duan wrote: > > On 2019/7/12 22:06, Andrew Cooper wrote: >> On 11/07/2019 03:15, Zhenzhong Duan wrote: >>> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >>> selftest") is used to ensure there is a gap setup in exception stack >>> which could be used for inserting call return address. >>> >>> This gap is missed in XEN PV int3 exception entry path, then below panic >>> triggered: >>> >>> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >>> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >>> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >>> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >>> [ 0.773334] Call Trace: >>> [ 0.773334] alternative_instructions+0x3d/0x12e >>> [ 0.773334] check_bugs+0x7c9/0x887 >>> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >>> [ 0.773334] start_kernel+0x4ff/0x535 >>> [ 0.773334] ? set_init_arg+0x55/0x55 >>> [ 0.773334] xen_start_kernel+0x571/0x57a >>> >>> As xenint3 and int3 entry code are same except xenint3 doesn't generate >>> a gap, we can fix it by using int3 and drop useless xenint3. >> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with >> %rcx/%r11 on the stack. >> >> To convert back to "normal" looking exceptions, the xen thunks do `pop >> %rcx; pop %r11; jmp do_*`... > I will add this to commit message. >> >>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >>> index 0ea4831..35a66fc 100644 >>> --- a/arch/x86/entry/entry_64.S >>> +++ b/arch/x86/entry/entry_64.S >>> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >>> #ifdef CONFIG_XEN_PV >>> idtentry xennmi do_nmi has_error_code=0 >>> idtentry xendebug do_debug has_error_code=0 >>> -idtentry xenint3 do_int3 has_error_code=0 >>> #endif >> What is confusing is why there are 3 extra magic versions here. At a >> guess, I'd say something to do with IST settings (given the vectors), >> but I don't see why #DB/#BP would need to be different in the first >> place. (NMI sure, but that is more to do with the crazy hoops needing >> to be jumped through to keep native functioning safely.) > > xenint3 will be removed in this patch safely as it don't use IST now. > > But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine > > if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit PV kernel mode always gets entered with kernel GS base already set. Hence finding out whether to switch the GS base is specifically not something that any exception entry point would need to do (and it should actively try to avoid it, for performance reasons). Jan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test 2019-07-15 6:54 ` Jan Beulich @ 2019-07-15 9:09 ` Andrew Cooper -1 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2019-07-15 9:09 UTC (permalink / raw) To: Jan Beulich, Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86@kernel.org, linux-kernel@vger.kernel.org, srinivas.eeda@oracle.com, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel@lists.xenproject.org, Thomas Gleixner, Boris Ostrovsky On 15/07/2019 07:54, Jan Beulich wrote: > On 15.07.2019 07:05, Zhenzhong Duan wrote: >> On 2019/7/12 22:06, Andrew Cooper wrote: >>> On 11/07/2019 03:15, Zhenzhong Duan wrote: >>>> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >>>> selftest") is used to ensure there is a gap setup in exception stack >>>> which could be used for inserting call return address. >>>> >>>> This gap is missed in XEN PV int3 exception entry path, then below panic >>>> triggered: >>>> >>>> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >>>> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >>>> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >>>> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >>>> [ 0.773334] Call Trace: >>>> [ 0.773334] alternative_instructions+0x3d/0x12e >>>> [ 0.773334] check_bugs+0x7c9/0x887 >>>> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >>>> [ 0.773334] start_kernel+0x4ff/0x535 >>>> [ 0.773334] ? set_init_arg+0x55/0x55 >>>> [ 0.773334] xen_start_kernel+0x571/0x57a >>>> >>>> As xenint3 and int3 entry code are same except xenint3 doesn't generate >>>> a gap, we can fix it by using int3 and drop useless xenint3. >>> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with >>> %rcx/%r11 on the stack. >>> >>> To convert back to "normal" looking exceptions, the xen thunks do `pop >>> %rcx; pop %r11; jmp do_*`... >> I will add this to commit message. >>>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >>>> index 0ea4831..35a66fc 100644 >>>> --- a/arch/x86/entry/entry_64.S >>>> +++ b/arch/x86/entry/entry_64.S >>>> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >>>> #ifdef CONFIG_XEN_PV >>>> idtentry xennmi do_nmi has_error_code=0 >>>> idtentry xendebug do_debug has_error_code=0 >>>> -idtentry xenint3 do_int3 has_error_code=0 >>>> #endif >>> What is confusing is why there are 3 extra magic versions here. At a >>> guess, I'd say something to do with IST settings (given the vectors), >>> but I don't see why #DB/#BP would need to be different in the first >>> place. (NMI sure, but that is more to do with the crazy hoops needing >>> to be jumped through to keep native functioning safely.) >> xenint3 will be removed in this patch safely as it don't use IST now. >> >> But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine >> >> if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? > Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit > PV kernel mode always gets entered with kernel GS base already set. Hence > finding out whether to switch the GS base is specifically not something that > any exception entry point would need to do (and it should actively try to > avoid it, for performance reasons). Indeed. The SWAPGS PVOP is implemented as a nop for x86 PV, to simply the entry assembly (rather than doubling up all entry vectors). ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test @ 2019-07-15 9:09 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2019-07-15 9:09 UTC (permalink / raw) To: Jan Beulich, Zhenzhong Duan Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, x86@kernel.org, linux-kernel@vger.kernel.org, srinivas.eeda@oracle.com, Ingo Molnar, Borislav Petkov, Andy Lutomirski, xen-devel@lists.xenproject.org, Thomas Gleixner, Boris Ostrovsky On 15/07/2019 07:54, Jan Beulich wrote: > On 15.07.2019 07:05, Zhenzhong Duan wrote: >> On 2019/7/12 22:06, Andrew Cooper wrote: >>> On 11/07/2019 03:15, Zhenzhong Duan wrote: >>>> Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() >>>> selftest") is used to ensure there is a gap setup in exception stack >>>> which could be used for inserting call return address. >>>> >>>> This gap is missed in XEN PV int3 exception entry path, then below panic >>>> triggered: >>>> >>>> [ 0.772876] general protection fault: 0000 [#1] SMP NOPTI >>>> [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 >>>> [ 0.772893] RIP: e030:int3_magic+0x0/0x7 >>>> [ 0.772905] RSP: 3507:ffffffff82203e98 EFLAGS: 00000246 >>>> [ 0.773334] Call Trace: >>>> [ 0.773334] alternative_instructions+0x3d/0x12e >>>> [ 0.773334] check_bugs+0x7c9/0x887 >>>> [ 0.773334] ? __get_locked_pte+0x178/0x1f0 >>>> [ 0.773334] start_kernel+0x4ff/0x535 >>>> [ 0.773334] ? set_init_arg+0x55/0x55 >>>> [ 0.773334] xen_start_kernel+0x571/0x57a >>>> >>>> As xenint3 and int3 entry code are same except xenint3 doesn't generate >>>> a gap, we can fix it by using int3 and drop useless xenint3. >>> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with >>> %rcx/%r11 on the stack. >>> >>> To convert back to "normal" looking exceptions, the xen thunks do `pop >>> %rcx; pop %r11; jmp do_*`... >> I will add this to commit message. >>>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >>>> index 0ea4831..35a66fc 100644 >>>> --- a/arch/x86/entry/entry_64.S >>>> +++ b/arch/x86/entry/entry_64.S >>>> @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 >>>> #ifdef CONFIG_XEN_PV >>>> idtentry xennmi do_nmi has_error_code=0 >>>> idtentry xendebug do_debug has_error_code=0 >>>> -idtentry xenint3 do_int3 has_error_code=0 >>>> #endif >>> What is confusing is why there are 3 extra magic versions here. At a >>> guess, I'd say something to do with IST settings (given the vectors), >>> but I don't see why #DB/#BP would need to be different in the first >>> place. (NMI sure, but that is more to do with the crazy hoops needing >>> to be jumped through to keep native functioning safely.) >> xenint3 will be removed in this patch safely as it don't use IST now. >> >> But debug and nmi need paranoid_entry which will read MSR_GS_BASE to determine >> >> if swapgs is needed. I guess PV guesting running in ring3 will #GP with swapgs? > Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit > PV kernel mode always gets entered with kernel GS base already set. Hence > finding out whether to switch the GS base is specifically not something that > any exception entry point would need to do (and it should actively try to > avoid it, for performance reasons). Indeed. The SWAPGS PVOP is implemented as a nop for x86 PV, to simply the entry assembly (rather than doubling up all entry vectors). ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-07-15 9:10 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-11 8:15 [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test Zhenzhong Duan 2019-07-11 8:15 ` Zhenzhong Duan 2019-07-12 12:06 ` [Xen-devel] " Peter Zijlstra 2019-07-12 12:06 ` Peter Zijlstra 2019-07-12 13:04 ` [Xen-devel] " Zhenzhong Duan 2019-07-12 13:04 ` Zhenzhong Duan 2019-07-12 13:09 ` [Xen-devel] " Peter Zijlstra 2019-07-12 13:09 ` Peter Zijlstra 2019-07-12 13:17 ` [Xen-devel] " Peter Zijlstra 2019-07-12 13:17 ` Peter Zijlstra 2019-07-12 13:27 ` [Xen-devel] " Zhenzhong Duan 2019-07-12 13:27 ` Zhenzhong Duan 2019-07-12 14:06 ` [Xen-devel] " Andrew Cooper 2019-07-12 14:06 ` Andrew Cooper 2019-07-15 5:05 ` Zhenzhong Duan 2019-07-15 5:05 ` Zhenzhong Duan 2019-07-15 6:54 ` Jan Beulich 2019-07-15 6:54 ` Jan Beulich 2019-07-15 9:09 ` Andrew Cooper 2019-07-15 9:09 ` Andrew Cooper
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.