Re: [PATCH v2 4/7] x86: NOP out most XPTI entry/exit code when it's not in use

[Andrew Cooper <andrew.cooper3@citrix.com>]

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.

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.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel