[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 4/7] x86: NOP out most XPTI entry/exit code when it's not in use
>>> On 01.03.18 at 20:42, <andrew.cooper3@xxxxxxxxxx> wrote: > On 07/02/18 16:13, Jan Beulich wrote: >> --- a/xen/arch/x86/x86_64/compat/entry.S >> +++ b/xen/arch/x86/x86_64/compat/entry.S >> @@ -199,7 +199,7 @@ ENTRY(compat_post_handle_exception) >> >> /* See lstar_enter for entry register state. */ >> ENTRY(cstar_enter) >> - /* sti could live here when we don't switch page tables below. */ >> + ALTERNATIVE nop, sti, X86_FEATURE_NO_XPTI >> CR4_PV32_RESTORE >> movq 8(%rsp),%rax /* Restore %rax. */ >> movq $FLAT_KERNEL_SS,8(%rsp) >> @@ -214,6 +214,7 @@ ENTRY(cstar_enter) >> /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */ >> >> GET_STACK_END(bx) >> +.Lcstar_cr3_start: >> mov STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx >> neg %rcx >> jz .Lcstar_cr3_okay >> @@ -223,6 +224,8 @@ ENTRY(cstar_enter) >> movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx) >> .Lcstar_cr3_okay: >> sti >> +.Lcstar_cr3_end: >> + ALTERNATIVE_NOP .Lcstar_cr3_start, .Lcstar_cr3_end, > X86_FEATURE_NO_XPTI > > This is much clearer with the nop infrastructure abstracted away. > > However, I remain unconvinced that this dynamic handling of interrupt > re-enabling is worth the hoops you've jumped through to make it happen. > It might be interesting to hear others thoughts on the matter. > > In particular, we've got a race window depending on the order in which > the alternatives list is traversed where we might be unsafe. But that ordering dependency is not just an issue with the interrupt enabling: Note how certain pairs of ALTERNATIVE_NOP are carefully ordered to first NOP out a later piece of code, and only then the earlier one. There's an argument to be made that the solitary writing back of %r15 could be left in place, as with the other pieces of code patched out plus the earlier zeroing of registers, %r15 will then only ever be zero, which is safe to write back. That zeroing code, however, wasn't in the tree yet when this patch was first submitted. Bottom line though - right now processing in order is a strict requirement. Note that multiple patches to the same patch site also depend on such ordering. > On a tangent (which probably wont affect the result of this patch), > given your thoughts to allow guests to notice and extend their own > featureset, what about Xen? If so, we're going to need something more > clever than simply nopping out the code. Well, if we want a runtime disable of xpti (like has been requested by one of our customers, and ISTR you saying something like this as well), then I don't think we should do this patching. But that could be achieved by simply not setting the NO_XPTI feature (and dropping cpu_has_no_xpti and its use), e.g. via an "xpti=dynamic" command line option extension. As per the feature request we've got this would need to be no more than the option of a one-time disable; anything more involved (like flipping between modes) would certainly be more complicated than is probably worth it. Of course then there's this other consideration that Jürgen has had with his series: If we want to make the page table switching domain dependent (and in particular don't do so for Dom0), then this patch either needs to go away altogether, or there would need to be further restriction on when to set NO_XPTI (and trigger patching). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |