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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Oct 31, 2018 at 02:58:59AM -0600, Jan Beulich wrote:
> >>> On 30.10.18 at 19:08, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 29/10/18 14:37, Jan Beulich wrote:
> >>>>> On 19.10.18 at 17:59, <andrew.cooper3@xxxxxxxxxx> 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.

How about putting in a function which calls panic? That seems to be the
least intrusive option?

Wei.

> 
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel