Re: [PATCH 1/2] KVM: x86: move MSR_IA32_FEATURE_CONTROL handling to x86.c

[Radim KrÄmÃÅ]

2016-07-08 14:01+0200, Paolo Bonzini:
> 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.
> 
> 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 <pbonzini@xxxxxxxxxx>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -3081,18 +3062,18 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	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;
> diff --git 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_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;

I'd avoid code duplication.  Either with

  kvm_x86_ops->msr_ia32_feature_control_write_trap(vcpu);

here, or with (simpler, but slightly harder to untangle)

  ret = kvm_set_msr_common(vcpu, msr_info);

in before vmx_leave_nested() in vmx_set_msr().

> +		break;