From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755163AbcGHM7h (ORCPT ); Fri, 8 Jul 2016 08:59:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55581 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754855AbcGHM7d (ORCPT ); Fri, 8 Jul 2016 08:59:33 -0400 Date: Fri, 8 Jul 2016 14:59:29 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Haozhong Zhang Subject: Re: [PATCH 1/2] KVM: x86: move MSR_IA32_FEATURE_CONTROL handling to x86.c Message-ID: <20160708125929.GA10741@potion> References: <1467979301-19376-1-git-send-email-pbonzini@redhat.com> <1467979301-19376-2-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467979301-19376-2-git-send-email-pbonzini@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 08 Jul 2016 12:59:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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;