Re: [PATCH v2 6/8] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update

[Paolo Bonzini]

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

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;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/