On 07/08/16 14:01, Paolo Bonzini wrote: > Because the MSR is listed in msrs_to_save, it is exported to userspace > for both AMD and Intel processors. However, on AMD currently getting > it will fail. >
Is MSR_IA32_FEATURE_CONTROL already be excluded from msrs_to_save[] by kvm_init_msr_list() on AMD hosts? Haozhong > vmx_set_msr must keep the case label in order to handle the "exit > nested on reset by writing 0" case. > > Signed-off-by: Paolo Bonzini <[email protected]> > --- > arch/x86/include/asm/kvm_host.h | 8 ++++++++ > arch/x86/kvm/vmx.c | 41 > ++++++++++------------------------------- > arch/x86/kvm/x86.c | 11 +++++++++++ > arch/x86/kvm/x86.h | 8 ++++++++ > 4 files changed, 37 insertions(+), 31 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index b771667e8e99..f77e5f5a6b01 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -486,6 +486,14 @@ struct kvm_vcpu_arch { > u64 ia32_xss; > > /* > + * Only bits masked by msr_ia32_feature_control_valid_bits can be set in > + * msr_ia32_feature_control. FEATURE_CONTROL_LOCKED is always included > + * in msr_ia32_feature_control_valid_bits. > + */ > + u64 msr_ia32_feature_control; > + u64 msr_ia32_feature_control_valid_bits; > + > + /* > * Paging state of the vcpu > * > * If the vcpu runs in guest mode with two level paging this still saves > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 15793a4c5df0..939cd8b835c2 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -611,14 +611,6 @@ struct vcpu_vmx { > bool guest_pkru_valid; > u32 guest_pkru; > u32 host_pkru; > - > - /* > - * Only bits masked by msr_ia32_feature_control_valid_bits can be set in > - * msr_ia32_feature_control. FEATURE_CONTROL_LOCKED is always included > - * in msr_ia32_feature_control_valid_bits. > - */ > - u64 msr_ia32_feature_control; > - u64 msr_ia32_feature_control_valid_bits; > }; > > enum segment_cache_field { > @@ -2932,14 +2924,6 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 > msr_index, u64 *pdata) > return 0; > } > > -static inline bool vmx_feature_control_msr_valid(struct kvm_vcpu *vcpu, > - uint64_t val) > -{ > - uint64_t valid_bits = to_vmx(vcpu)->msr_ia32_feature_control_valid_bits; > - > - return !(val & ~valid_bits); > -} > - > /* > * Reads an msr value (of 'msr_index') into 'pdata'. > * Returns 0 on success, non-0 otherwise. > @@ -2983,14 +2967,11 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > break; > case MSR_IA32_MCG_EXT_CTL: > if (!msr_info->host_initiated && > - !(to_vmx(vcpu)->msr_ia32_feature_control & > + !(vcpu->arch.msr_ia32_feature_control & > FEATURE_CONTROL_LMCE)) > return 1; > msr_info->data = vcpu->arch.mcg_ext_ctl; > break; > - case MSR_IA32_FEATURE_CONTROL: > - msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; > - break; > case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC: > if (!nested_vmx_allowed(vcpu)) > return 1; > @@ -3081,18 +3062,18 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > break; > case MSR_IA32_MCG_EXT_CTL: > if ((!msr_info->host_initiated && > - !(to_vmx(vcpu)->msr_ia32_feature_control & > + !(vcpu->arch.msr_ia32_feature_control & > FEATURE_CONTROL_LMCE)) || > (data & ~MCG_EXT_CTL_LMCE_EN)) > return 1; > vcpu->arch.mcg_ext_ctl = data; > break; > case MSR_IA32_FEATURE_CONTROL: > - if (!vmx_feature_control_msr_valid(vcpu, data) || > - (to_vmx(vcpu)->msr_ia32_feature_control & > + if (!feature_control_msr_valid(vcpu, data) || > + (vcpu->arch.msr_ia32_feature_control & > FEATURE_CONTROL_LOCKED && !msr_info->host_initiated)) > return 1; > - vmx->msr_ia32_feature_control = data; > + vcpu->arch.msr_ia32_feature_control = data; > if (msr_info->host_initiated && data == 0) > vmx_leave_nested(vcpu); > break; > @@ -6964,7 +6945,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu) > return 1; > } > > - if ((vmx->msr_ia32_feature_control & VMXON_NEEDED_FEATURES) > + if ((vcpu->arch.msr_ia32_feature_control & VMXON_NEEDED_FEATURES) > != VMXON_NEEDED_FEATURES) { > kvm_inject_gp(vcpu, 0); > return 1; > @@ -9081,8 +9062,6 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm > *kvm, unsigned int id) > goto free_vmcs; > } > > - vmx->msr_ia32_feature_control_valid_bits = FEATURE_CONTROL_LOCKED; > - > return &vmx->vcpu; > > free_vmcs: > @@ -9232,10 +9211,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > } > > if (nested_vmx_allowed(vcpu)) > - to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |= > + vcpu->arch.msr_ia32_feature_control_valid_bits |= > FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX; > else > - to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &= > + vcpu->arch.msr_ia32_feature_control_valid_bits &= > ~FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX; > } > > @@ -11135,10 +11114,10 @@ out: > static void vmx_setup_mce(struct kvm_vcpu *vcpu) > { > if (vcpu->arch.mcg_cap & MCG_LMCE_P) > - to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |= > + vcpu->arch.msr_ia32_feature_control_valid_bits |= > FEATURE_CONTROL_LMCE; > else > - to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &= > + vcpu->arch.msr_ia32_feature_control_valid_bits &= > ~FEATURE_CONTROL_LMCE; > } > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 411f786950ab..388d9ffd7551 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2097,6 +2097,13 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_TSCDEADLINE: > kvm_set_lapic_tscdeadline_msr(vcpu, data); > break; > + case MSR_IA32_FEATURE_CONTROL: > + if (!feature_control_msr_valid(vcpu, data) || > + (vcpu->arch.msr_ia32_feature_control & > + FEATURE_CONTROL_LOCKED && !msr_info->host_initiated)) > + return 1; > + vcpu->arch.msr_ia32_feature_control = data; > + break; > case MSR_IA32_TSC_ADJUST: > if (guest_cpuid_has_tsc_adjust(vcpu)) { > if (!msr_info->host_initiated) { > @@ -2364,6 +2371,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_TSC_ADJUST: > msr_info->data = (u64)vcpu->arch.ia32_tsc_adjust_msr; > break; > + case MSR_IA32_FEATURE_CONTROL: > + msr_info->data = vcpu->arch.msr_ia32_feature_control; > + break; > case MSR_IA32_MISC_ENABLE: > msr_info->data = vcpu->arch.ia32_misc_enable_msr; > break; > @@ -7705,6 +7715,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > > vcpu->arch.ia32_tsc_adjust_msr = 0x0; > vcpu->arch.pv_time_enabled = false; > + vcpu->arch.msr_ia32_feature_control_valid_bits = FEATURE_CONTROL_LOCKED; > > vcpu->arch.guest_supported_xcr0 = 0; > vcpu->arch.guest_xstate_size = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET; > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index 7ce3634ab5fe..a46c43aa762c 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -157,6 +157,14 @@ static inline bool kvm_check_has_quirk(struct kvm *kvm, > u64 quirk) > return !(kvm->arch.disabled_quirks & quirk); > } > > +static inline bool feature_control_msr_valid(struct kvm_vcpu *vcpu, > + uint64_t val) > +{ > + uint64_t valid_bits = vcpu->arch.msr_ia32_feature_control_valid_bits; > + > + return !(val & ~valid_bits); > +} > + > void kvm_before_handle_nmi(struct kvm_vcpu *vcpu); > void kvm_after_handle_nmi(struct kvm_vcpu *vcpu); > void kvm_set_pending_timer(struct kvm_vcpu *vcpu); > -- > 1.8.3.1 > >
