Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV

["Jan Beulich" <JBeulich@suse.com>]

>>> On 30.10.18 at 19:08, <andrew.cooper3@citrix.com> wrote:
> On 29/10/18 14:37, Jan Beulich wrote:
>>>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
>>> On 19/10/18 15:28, Wei Liu wrote:
>>>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
>>>>      /* Common SYSCALL parameters. */
>>>>      wrmsrl(MSR_STAR, XEN_MSR_STAR);
>>>>      wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>>>> +#endif
>>> It would be a wise precaution to initialise these MSRs to 0 in the !PV
>>> case, so we don't retain stale values.
>> If anything, EFER.SCE needs to be kept clear, as that's what
>> controls whether SYSCALL would raise #GP(0).
> 
> I toyed with suggesting this, but I'm not entirely certain.
> 
> SVM unilaterally switches EFER between host and guest context, so will
> preserve whatever value Xen had at VMRUN time.
> 
> Gen 2 VT-x has host/guest load/save support, so can be configured to
> exit in whichever configuration we would like.
> 
> Gen 1 VT-x uses MSR load-save lists, with an optimisation in the case
> that guest == host.  By clearing SCE in Xen context, we miss the
> optimisation in the common case for 64bit guests.
> 
>> But without a
>> PV domain around, nothing can access the host values of
>> these MSRs in the first place, so instead we could simplify
>> some context switching by never restoring host values, and
>> only ever loading guest ones. Except that, of course, VMLOAD
>> is an all-or-nothing insn, and we need to use to get TR loaded.
> 
> The VMLOAD path is a bit of a special case, in that we need to do it,
> and its rather faster than the other available options.  Conditionally
> feeding zeros into this would be fine.
> 
> That said, overall, we may want to leave some poisoned values around. 
> In the case that SCE is enabled and we do hit a spurious SYSCALL/SYSRET
> instruction, it would be better to definitely crash.

I'd be fine with poisoned (but not zero) values, if indeed we mean
to allow for a hypervisor crash in that case (which ought to be
fine, since we're talking about unreachable code anyway). Ideally
"poisoned" would be "non-canonical", but the MSRs don't allow for
non-canonical addresses to be loaded into them, so we'd need to
think of different poisoning values.

Trapping a spurious SYSRET seems impossible though, as EFER.SCE
is the only attribute we control there.

For SYSENTER/SYSEXIT, storing zeros is of course going to be
fine (yielding #GP(0) on both insns).

Jan



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