All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: gleb@kernel.org, mtosatti@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 6/8] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update
Date: Tue, 15 Sep 2015 17:45:05 +0200	[thread overview]
Message-ID: <55F83D01.1030200@redhat.com> (raw)
In-Reply-To: <1441778757-5866-7-git-send-email-guangrong.xiao@linux.intel.com>



On 09/09/2015 08:05, Xiao Guangrong wrote:
> Unify the update in vmx_cpuid_update()
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>

What if we instead start fresh from vmx_secondary_exec_control, like this:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4504cbc1b287..98c4d3f1653a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8660,11 +8660,26 @@ static int vmx_get_lpage_level(void)
 		return PT_PDPE_LEVEL;
 }
 
+static void vmcs_set_secondary_exec_control(u32 new_ctl)
+{
+	/*
+	 * These bits in the secondary execution controls field
+	 * are dynamic, the others are mostly based on the hypervisor
+	 * architecture and the guest's CPUID.  Do not touch the
+	 * dynamic bits.
+	 */
+	u32 mask = SECONDARY_EXEC_ENABLE_PML | SECONDARY_EXEC_SHADOW_VMCS;
+	u32 cur_ctl = vmcs_read32(SECONDARY_EXEC_CONTROL);
+
+	vmcs_write32(SECONDARY_EXEC_CONTROL,
+		     (new_ctl & ~mask) | (cur_ctl & mask));
+}
+
 static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	u32 clear_exe_ctrl = 0;
+	u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx);
 
 	vmx->rdtscp_enabled = false;
 	if (vmx_rdtscp_supported()) {
@@ -8672,7 +8681,7 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 		if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
 			vmx->rdtscp_enabled = true;
 		else
-			clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
+			secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP;
 
 		if (nested) {
 			if (vmx->rdtscp_enabled)
@@ -8689,18 +8698,13 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 	if (vmx_invpcid_supported() &&
 	    (!best || !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
 	    !guest_cpuid_has_pcid(vcpu))) {
-		clear_exe_ctrl |= SECONDARY_EXEC_ENABLE_INVPCID;
+		secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID;
 
 		if (best)
 			best->ebx &= ~bit(X86_FEATURE_INVPCID);
 	}
 
-	if (clear_exe_ctrl) {
-		u32 exec_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-
-		exec_ctl &= ~clear_exe_ctrl;
-		vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctl);
-	}
+	vmcs_set_secondary_exec_control(secondary_exec_ctl);
 
 	if (static_cpu_has(X86_FEATURE_PCOMMIT) && nested) {
 		if (guest_cpuid_has_pcommit(vcpu))

> ---
>  arch/x86/kvm/vmx.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 97e3340..5a074d0 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8673,19 +8673,15 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpuid_entry2 *best;
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> -	u32 exec_control;
> +	u32 clear_exe_ctrl = 0;
>  
>  	vmx->rdtscp_enabled = false;
>  	if (vmx_rdtscp_supported()) {
> -		exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>  		best = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
>  		if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
>  			vmx->rdtscp_enabled = true;
> -		else {
> -			exec_control &= ~SECONDARY_EXEC_RDTSCP;
> -			vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
> -					exec_control);
> -		}
> +		else
> +			clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
>  
>  		if (nested && !vmx->rdtscp_enabled)
>  			vmx->nested.nested_vmx_secondary_ctls_high &=
> @@ -8697,14 +8693,19 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  	if (vmx_invpcid_supported() &&
>  	    (!best || !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
>  	    !guest_cpuid_has_pcid(vcpu))) {
> -		exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> -		exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> -		vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
> +		clear_exe_ctrl |= SECONDARY_EXEC_ENABLE_INVPCID;
>  
>  		if (best)
>  			best->ebx &= ~bit(X86_FEATURE_INVPCID);
>  	}
>  
> +	if (clear_exe_ctrl) {
> +		u32 exec_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> +
> +		exec_ctl &= ~clear_exe_ctrl;
> +		vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctl);
> +	}
> +
>  	if (!guest_cpuid_has_pcommit(vcpu) && nested)
>  		vmx->nested.nested_vmx_secondary_ctls_high &=
>  			~SECONDARY_EXEC_PCOMMIT;
> 

  reply	other threads:[~2015-09-15 15:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09  6:05 [PATCH v2 0/8] KVM: x86: enable cflushopt/clwb/pcommit and simplify code Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 1/8] KVM: x86: allow guest to use cflushopt and clwb Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 2/8] KVM: x86: add pcommit support Xiao Guangrong
2015-09-15 15:30   ` Paolo Bonzini
2015-09-09  6:05 ` [PATCH v2 3/8] KVM: VMX: drop rdtscp_enabled check in prepare_vmcs02() Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 4/8] KVM: VMX: simplify rdtscp handling in vmx_cpuid_update() Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 5/8] KVM: VMX: simplify invpcid " Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 6/8] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update Xiao Guangrong
2015-09-15 15:45   ` Paolo Bonzini [this message]
2015-09-09  6:05 ` [PATCH v2 7/8] KVM: VMX: clean up bit operation on SECONDARY_VM_EXEC_CONTROL Xiao Guangrong
2015-09-09  6:05 ` [PATCH v2 8/8] KVM: VMX: drop rdtscp_enabled field Xiao Guangrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F83D01.1030200@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=gleb@kernel.org \
    --cc=guangrong.xiao@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.