[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 5/9] x86/vmx: Fix handing of MSR_DEBUGCTL on VMExit



>>> On 22.05.18 at 13:20, <andrew.cooper3@xxxxxxxxxx> wrote:
> Currently, whenever the guest writes a nonzero value to MSR_DEBUGCTL, Xen
> updates a host MSR load list entry with the current hardware value of
> MSR_DEBUGCTL.  This is wrong.
> 
> On VMExit, hardware automatically resets MSR_DEBUGCTL to 0.

I'm pretty certain that back when I write this, the SDM didn't spell this out.

>  The only case
> where different behaviour is needed is if Xen is debugging itself, and this
> needs setting up unconditionally for the lifetime of the VM.
> 
> The `ler` command line boolean is the only way to configure any use of
> MSR_DEBUGCTL for Xen, so tie the host load list entry to this setting in
> construct_vmcs().  Any runtime update of Xen's MSR_DEBUGCTL setting requires
> more complicated synchronisation across all the running VMs.
> 
> In the exceedingly common case, this avoids the unnecessary overhead of having
> a host load entry performing the same zeroing operation that hardware has
> already performed as part of the VMExit.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> Notes for backporting: This change probably does want backporting, but depends
> on the previous patch "Support remote access to the MSR lists", and adds an
> extra rdmsr to the vcpu construction path (resolved in a later patch).

I wonder if for backporting purposes we couldn't stick this function
invocation somewhere else, like vmx_ctxt_switch_to() or
vmx_do_resume(). I realize the potential allocation failure is a problem here,
but for that we could either allocate the memory at the place you now
invoke vmx_add_host_load_msr(), or take the brute force approach and
crash the domain upon failure (the net effect won't be much different to
allocation failing during domain destruction - the domain won't start in either
case).

I mention this because it seems to me that pulling in the previous patch
would in turn require pulling in earlier ones.

Jan



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.