[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.8] x86/vmx: Improvements to LBR MSR handling
commit b2894035270a3fbe2d3c969f4c8c19ef77280a2e Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> AuthorDate: Mon May 7 11:57:00 2018 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Tue Aug 14 17:27:26 2018 +0100 x86/vmx: Improvements to LBR MSR handling The main purpose of this patch is to only ever insert the LBR MSRs into the guest load/save list once, as a future patch wants to change the behaviour of vmx_add_guest_msr(). The repeated processing of lbr_info and the guests MSR load/save list is redundant, and a guest using LBR itself will have to re-enable MSR_DEBUGCTL.LBR in its #DB handler, meaning that Xen will repeat this redundant processing every time the guest gets a debug exception. Rename lbr_fixup_enabled to lbr_flags to be a little more generic, and use one bit to indicate that the MSRs have been inserted into the load/save list. Shorten the existing FIXUP* identifiers to reduce code volume. Furthermore, handing the guest #MC on an error isn't a legitimate action. Two of the three failure cases are definitely hypervisor bugs, and the third is a boundary case which shouldn't occur in practice. The guest also won't execute correctly, so handle errors by cleanly crashing the guest. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> (cherry picked from commit be73a842e642772d7372004c9c105de35b771020) --- xen/arch/x86/hvm/vmx/vmx.c | 63 ++++++++++++++++++++++++++++++-------- xen/include/asm-x86/hvm/vmx/vmcs.h | 3 ++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 1becfaa9a0..d16039a6e0 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2756,6 +2756,8 @@ static const struct lbr_info *last_branch_msr_get(void) return NULL; } +#define LBR_MSRS_INSERTED (1u << 0) + static int is_last_branch_msr(u32 ecx) { const struct lbr_info *lbr = last_branch_msr_get(); @@ -2996,7 +2998,6 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) __vmwrite(GUEST_SYSENTER_EIP, msr_content); break; case MSR_IA32_DEBUGCTLMSR: { - int i, rc = 0; uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF; if ( boot_cpu_has(X86_FEATURE_RTM) ) @@ -3007,23 +3008,59 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) if ( vpmu_do_wrmsr(msr, msr_content, supported) ) break; } - if ( msr_content & IA32_DEBUGCTLMSR_LBR ) + + /* + * When a guest first enables LBR, arrange to save and restore the LBR + * MSRs and allow the guest direct access. + * + * MSR_DEBUGCTL and LBR has existed almost as long as MSRs have + * existed, and there is no architectural way to hide the feature, or + * fail the attempt to enable LBR. + * + * Unknown host LBR MSRs or hitting -ENOSPC with the guest load/save + * list are definitely hypervisor bugs, whereas -ENOMEM for allocating + * the load/save list is simply unlucky (and shouldn't occur with + * sensible management by the toolstack). + * + * Either way, there is nothing we can do right now to recover, and + * the guest won't execute correctly either. Simply crash the domain + * to make the failure obvious. + */ + if ( !(v->arch.hvm_vmx.lbr_flags & LBR_MSRS_INSERTED) && + (msr_content & IA32_DEBUGCTLMSR_LBR) ) { const struct lbr_info *lbr = last_branch_msr_get(); - if ( lbr == NULL ) - break; - for ( ; (rc == 0) && lbr->count; lbr++ ) - for ( i = 0; (rc == 0) && (i < lbr->count); i++ ) - if ( (rc = vmx_add_guest_msr(v, lbr->base + i)) == 0 ) - vmx_disable_intercept_for_msr(v, lbr->base + i, MSR_TYPE_R | MSR_TYPE_W); - } + if ( unlikely(!lbr) ) + { + gprintk(XENLOG_ERR, "Unknown Host LBR MSRs\n"); + domain_crash(v->domain); + return X86EMUL_OKAY; + } - if ( rc < 0 ) - hvm_inject_hw_exception(TRAP_machine_check, HVM_DELIVER_NO_ERROR_CODE); - else - __vmwrite(GUEST_IA32_DEBUGCTL, msr_content); + for ( ; lbr->count; lbr++ ) + { + unsigned int i; + + for ( i = 0; i < lbr->count; i++ ) + { + int rc = vmx_add_guest_msr(v, lbr->base + i); + + if ( unlikely(rc) ) + { + gprintk(XENLOG_ERR, + "Guest load/save list error %d\n", rc); + domain_crash(v->domain); + return X86EMUL_OKAY; + } + + vmx_disable_intercept_for_msr(v, lbr->base + i, + MSR_TYPE_R | MSR_TYPE_W); + } + } + } + __vmwrite(GUEST_IA32_DEBUGCTL, msr_content); break; } case MSR_IA32_FEATURE_CONTROL: diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 374a16f359..f15f735b1a 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -218,6 +218,9 @@ struct arch_vmx_struct { uint8_t vmx_realmode; /* Are we emulating rather than VMENTERing? */ uint8_t vmx_emulate; + + uint8_t lbr_flags; + /* Bitmask of segments that we can't safely use in virtual 8086 mode */ uint16_t vm86_segment_mask; /* Shadow CS, SS, DS, ES, FS, GS, TR while in virtual 8086 mode */ -- generated by git-patchbot for /home/xen/git/xen.git#stable-4.8 _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |