linux-kernel - Re: [PATCH 1/2] KVM: x86: move MSR_IA32_FEATURE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 8 Jul 2016 14:59:29 +0200
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	Haozhong Zhang <haozhong.zhang@...el.com>
Subject: Re: [PATCH 1/2] KVM: x86: move MSR_IA32_FEATURE_CONTROL handling to
 x86.c

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@...hat.com>
> ---
> 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;