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