On 24/05/18 13:14, Roger Pau Monné wrote: > On Tue, May 22, 2018 at 12:20:42PM +0100, Andrew Cooper 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. 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, > Hm, there's no documentation at all for the 'ler' option.
:( ISTR Jan introducing it for debugging purposes, but I've never used it myself. > >> 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.coop...@citrix.com> >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Jun Nakajima <jun.nakaj...@intel.com> >> CC: Kevin Tian <kevin.t...@intel.com> >> CC: Wei Liu <wei.l...@citrix.com> >> CC: Roger Pau Monné <roger....@citrix.com> >> >> 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). >> --- >> xen/arch/x86/hvm/vmx/vmcs.c | 6 ++++++ >> xen/arch/x86/hvm/vmx/vmx.c | 3 +-- >> 2 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c >> index 8bf54c4..2035a6d 100644 >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -996,6 +996,7 @@ static int construct_vmcs(struct vcpu *v) >> struct domain *d = v->domain; >> u32 vmexit_ctl = vmx_vmexit_control; >> u32 vmentry_ctl = vmx_vmentry_control; >> + int rc; >> >> vmx_vmcs_enter(v); >> >> @@ -1266,6 +1267,11 @@ static int construct_vmcs(struct vcpu *v) >> if ( cpu_has_vmx_tsc_scaling ) >> __vmwrite(TSC_MULTIPLIER, d->arch.hvm_domain.tsc_scaling_ratio); >> >> + /* If using host debugging, restore Xen's setting on vmexit. */ >> + if ( this_cpu(ler_msr) && >> + (rc = vmx_add_host_load_msr(v, MSR_IA32_DEBUGCTLMSR)) ) >> + return rc; > Isn't this missing a vmx_vmcs_exit on error? It is indeed. Will fix. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
