Re: [PATCH 2/3] KVM: VMX: modify preemption timer bit only when arming timer

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 28/08/2018 00:21, Sean Christopherson wrote:
> Provide a singular location where the VMX preemption timer bit is
> set/cleared so that future usages of the preemption timer can ensure
> the VMCS bit is up-to-date without having to modify unrelated code
> paths.  For example, the preemption timer can be used to force an
> immediate VMExit.  Cache the status of the timer to avoid redundant
> VMREAD and VMWRITE, e.g. if the timer stays armed across multiple
> VMEnters/VMExits.

It's mostly aesthetic, but I slightly prefer this:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 8f8aa6391a47..7a66a7e817af 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10596,10 +10596,9 @@ static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
 					msrs[i].host, false);
 }
 
-static void vmx_arm_hv_timer(struct kvm_vcpu *vcpu)
+static void vmx_update_hv_timer(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	bool arm_timer;
 	u64 tscl;
 	u32 delta_tsc;
 
@@ -10613,16 +10612,16 @@ static void vmx_arm_hv_timer(struct kvm_vcpu *vcpu)
 			delta_tsc = 0;
 
 		vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, delta_tsc);
+		if (!vmx->hv_timer_armed)
+			vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL,
+				      PIN_BASED_VMX_PREEMPTION_TIMER);
+		vmx->hv_timer_armed = true;
+	} else {
+		if (vmx->hv_timer_armed)
+			vmcs_clear_bits(PIN_BASED_VM_EXEC_CONTROL,
+					PIN_BASED_VMX_PREEMPTION_TIMER);
+		vmx->hv_timer_armed = false;
 	}
-
-	arm_timer = (vmx->hv_deadline_tsc != -1);
-	if (arm_timer && !vmx->hv_timer_armed)
-		vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL,
-			PIN_BASED_VMX_PREEMPTION_TIMER);
-	else if (!arm_timer && vmx->hv_timer_armed)
-		vmcs_clear_bits(PIN_BASED_VM_EXEC_CONTROL,
-			PIN_BASED_VMX_PREEMPTION_TIMER);
-	vmx->hv_timer_armed = arm_timer;
 }
 
 static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
@@ -10682,7 +10681,7 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	atomic_switch_perf_msrs(vmx);
 
-	vmx_arm_hv_timer(vcpu);
+	vmx_update_hv_timer(vcpu);
 
 	/*
 	 * If this vCPU has touched SPEC_CTRL, restore the guest's value if

The generated code is probably pretty much the same, since the compiler
can figure out most of this.

Paolo