From 38b48605f3693e950bb4155ea8dac6614d796c2b Mon Sep 17 00:00:00 2001 From: Andrew Cooper Date: Sun, 18 Dec 2016 14:56:28 +0000 Subject: [PATCH] x86/vmx: Drop vmx_msr_state infrastructure To avoid leaking host MSR state into guests, guest LSTAR, STAR and SYSCALL_MASK state is unconditionally loaded when switching into guest context. Attempting to dirty-track the state is pointless; host state is always restoring upon exit from guest context, meaning that guest state is always considered dirty. Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[]. The guests MSR values are stored plainly in arch_vmx_struct, in the same way as shadow_gs and cstar are. vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure that the hardware MSR values are always up-to-date. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich Acked-by: Kevin Tian --- xen/arch/x86/hvm/vmx/vmcs.c | 3 -- xen/arch/x86/hvm/vmx/vmx.c | 79 ++++++++---------------------- xen/include/asm-x86/hvm/vmx/vmcs.h | 21 ++------ 3 files changed, 26 insertions(+), 77 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index e56456a069..db8c4f46de 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -1125,9 +1125,6 @@ static int construct_vmcs(struct vcpu *v) vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R | MSR_TYPE_W); } - /* All guest MSR state is dirty. */ - v->arch.hvm_vmx.msr_state.flags = ((1u << VMX_MSR_COUNT) - 1); - /* I/O access bitmap. */ __vmwrite(IO_BITMAP_A, __pa(d->arch.hvm_domain.io_bitmap)); __vmwrite(IO_BITMAP_B, __pa(d->arch.hvm_domain.io_bitmap) + PAGE_SIZE); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 468bb789c8..e0c2831100 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -360,24 +360,10 @@ static void vmx_vcpu_destroy(struct vcpu *v) passive_domain_destroy(v); } -static const u32 msr_index[VMX_MSR_COUNT] = -{ - [VMX_INDEX_MSR_LSTAR] = MSR_LSTAR, - [VMX_INDEX_MSR_STAR] = MSR_STAR, - [VMX_INDEX_MSR_SYSCALL_MASK] = MSR_SYSCALL_MASK -}; - -#define WRITE_MSR(address) do { \ - guest_msr_state->msrs[VMX_INDEX_MSR_ ## address] = msr_content; \ - __set_bit(VMX_INDEX_MSR_ ## address, &guest_msr_state->flags); \ - wrmsrl(MSR_ ## address, msr_content); \ - } while ( 0 ) - static enum handler_return long_mode_do_msr_read(unsigned int msr, uint64_t *msr_content) { struct vcpu *v = current; - struct vmx_msr_state *guest_msr_state = &v->arch.hvm_vmx.msr_state; switch ( msr ) { @@ -394,11 +380,11 @@ long_mode_do_msr_read(unsigned int msr, uint64_t *msr_content) break; case MSR_STAR: - *msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_STAR]; + *msr_content = v->arch.hvm_vmx.star; break; case MSR_LSTAR: - *msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_LSTAR]; + *msr_content = v->arch.hvm_vmx.lstar; break; case MSR_CSTAR: @@ -406,7 +392,7 @@ long_mode_do_msr_read(unsigned int msr, uint64_t *msr_content) break; case MSR_SYSCALL_MASK: - *msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK]; + *msr_content = v->arch.hvm_vmx.sfmask; break; default: @@ -422,7 +408,6 @@ static enum handler_return long_mode_do_msr_write(unsigned int msr, uint64_t msr_content) { struct vcpu *v = current; - struct vmx_msr_state *guest_msr_state = &v->arch.hvm_vmx.msr_state; HVM_DBG_LOG(DBG_LEVEL_MSR, "msr %#x content %#"PRIx64, msr, msr_content); @@ -444,13 +429,15 @@ long_mode_do_msr_write(unsigned int msr, uint64_t msr_content) break; case MSR_STAR: - WRITE_MSR(STAR); + v->arch.hvm_vmx.star = msr_content; + wrmsrl(MSR_STAR, msr_content); break; case MSR_LSTAR: if ( !is_canonical_address(msr_content) ) goto uncanonical_address; - WRITE_MSR(LSTAR); + v->arch.hvm_vmx.lstar = msr_content; + wrmsrl(MSR_LSTAR, msr_content); break; case MSR_CSTAR: @@ -460,7 +447,8 @@ long_mode_do_msr_write(unsigned int msr, uint64_t msr_content) break; case MSR_SYSCALL_MASK: - WRITE_MSR(SYSCALL_MASK); + v->arch.hvm_vmx.sfmask = msr_content; + wrmsrl(MSR_SYSCALL_MASK, msr_content); break; default: @@ -500,26 +488,10 @@ static void vmx_save_guest_msrs(struct vcpu *v) static void vmx_restore_guest_msrs(struct vcpu *v) { - struct vmx_msr_state *guest_msr_state; - unsigned long guest_flags; - int i; - - guest_msr_state = &v->arch.hvm_vmx.msr_state; - wrmsrl(MSR_SHADOW_GS_BASE, v->arch.hvm_vmx.shadow_gs); - - guest_flags = guest_msr_state->flags; - - while ( guest_flags ) - { - i = find_first_set_bit(guest_flags); - - HVM_DBG_LOG(DBG_LEVEL_2, - "restore guest's index %d msr %x with value %lx", - i, msr_index[i], guest_msr_state->msrs[i]); - wrmsrl(msr_index[i], guest_msr_state->msrs[i]); - __clear_bit(i, &guest_flags); - } + wrmsrl(MSR_STAR, v->arch.hvm_vmx.star); + wrmsrl(MSR_LSTAR, v->arch.hvm_vmx.lstar); + wrmsrl(MSR_SYSCALL_MASK, v->arch.hvm_vmx.sfmask); if ( (v->arch.hvm_vcpu.guest_efer ^ read_efer()) & EFER_SCE ) { @@ -757,30 +729,21 @@ static int vmx_vmcs_restore(struct vcpu *v, struct hvm_hw_cpu *c) static void vmx_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data) { - struct vmx_msr_state *guest_state = &v->arch.hvm_vmx.msr_state; - - data->shadow_gs = v->arch.hvm_vmx.shadow_gs; - data->msr_cstar = v->arch.hvm_vmx.cstar; - - /* save msrs */ + data->shadow_gs = v->arch.hvm_vmx.shadow_gs; data->msr_flags = 0; - data->msr_lstar = guest_state->msrs[VMX_INDEX_MSR_LSTAR]; - data->msr_star = guest_state->msrs[VMX_INDEX_MSR_STAR]; - data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK]; + data->msr_lstar = v->arch.hvm_vmx.lstar; + data->msr_star = v->arch.hvm_vmx.star; + data->msr_cstar = v->arch.hvm_vmx.cstar; + data->msr_syscall_mask = v->arch.hvm_vmx.sfmask; } static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data) { - struct vmx_msr_state *guest_state = &v->arch.hvm_vmx.msr_state; - - /* restore msrs */ - guest_state->flags = ((1u << VMX_MSR_COUNT) - 1); - guest_state->msrs[VMX_INDEX_MSR_LSTAR] = data->msr_lstar; - guest_state->msrs[VMX_INDEX_MSR_STAR] = data->msr_star; - guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK] = data->msr_syscall_mask; - - v->arch.hvm_vmx.cstar = data->msr_cstar; v->arch.hvm_vmx.shadow_gs = data->shadow_gs; + v->arch.hvm_vmx.star = data->msr_star; + v->arch.hvm_vmx.lstar = data->msr_lstar; + v->arch.hvm_vmx.cstar = data->msr_cstar; + v->arch.hvm_vmx.sfmask = data->msr_syscall_mask; } diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 2b58d5e845..af4e258f81 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -39,19 +39,6 @@ struct vmx_msr_entry { u64 data; }; -enum { - VMX_INDEX_MSR_LSTAR = 0, - VMX_INDEX_MSR_STAR, - VMX_INDEX_MSR_SYSCALL_MASK, - - VMX_MSR_COUNT -}; - -struct vmx_msr_state { - unsigned long flags; - unsigned long msrs[VMX_MSR_COUNT]; -}; - #define EPT_DEFAULT_MT MTRR_TYPE_WRBACK struct ept_data { @@ -123,9 +110,11 @@ struct arch_vmx_struct { u32 secondary_exec_control; u32 exception_bitmap; - struct vmx_msr_state msr_state; - unsigned long shadow_gs; - unsigned long cstar; + uint64_t shadow_gs; + uint64_t star; + uint64_t lstar; + uint64_t cstar; + uint64_t sfmask; unsigned long *msr_bitmap; unsigned int msr_count; -- 2.30.2