Fix Warnining in arch/x86/kvm/vmx.c

Hi Avi/Yaniv,

With gcc --version 4.4.1 20090429 (prerelease)

I get the following warning:
arch/x86/kvm/vmx.c: In function ‘vmx_intr_assist’:
arch/x86/kvm/vmx.c:3233: warning: ‘max_irr’ may be used uninitialized in this function arch/x86/kvm/vmx.c:3233: note: ‘max_irr’ was declared here

Investigation found that:

3231 static void update_tpr_threshold(struct kvm_vcpu *vcpu) 3232 {
3233 int max_irr, tpr;
3234
3235 if (!vm_need_tpr_shadow(vcpu->kvm))
3236 return;
3237
3238 if (!kvm_lapic_enabled(vcpu) ||
3239 ((max_irr = kvm_lapic_find_highest_irr(vcpu)) == -1)) {

	(max_irr = kvm_lapic_find_highest_irr(vcpu)) == -1

may not get a chance to evaluate if:

!kvm_lapic_enabled(vcpu)

evaluates to true (as the expressions are Or-ed).

3240 vmcs_write32(TPR_THRESHOLD, 0);
3241 return;
3242 }
3243
3244 tpr = (kvm_lapic_get_cr8(vcpu) & 0x0f) << 4;
3245 vmcs_write32(TPR_THRESHOLD, (max_irr > tpr) ? tpr >> 4 : max_irr >> 4);

Using (max_irr > tpr) and max_irr >> 4, without max_irr getting initialized can cause trouble.

3246 }

I would like to propose a small fix for this by interchanging the operands in ||, so that max_irr is initialized in all instances, and, the warning fades away, without compromising the criteria of conditional evaluation inside if().

To: Avi Kivity <[email protected]>
To: Yaniv Kamay <[email protected]>
To: <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Sachin P Sant <[email protected]>
Subject: [PATCH][Resend] Fix Warnining in arch/x86/kvm/vmx.c
---

@@ -3235,8 +3235,8 @@ static void update_tpr_threshold(struct 
 	if (!vm_need_tpr_shadow(vcpu->kvm))
 		return;
 
-	if (!kvm_lapic_enabled(vcpu) ||
-	    ((max_irr = kvm_lapic_find_highest_irr(vcpu)) == -1)) {
+	if (((max_irr = kvm_lapic_find_highest_irr(vcpu)) == -1) ||
+	     !kvm_lapic_enabled(vcpu)) {
 		vmcs_write32(TPR_THRESHOLD, 0);
 		return;
 	}

---
Regards--
Subrata

This function no longer exists; can you check if the current code is susceptible?

With !kvm_lapic_enabled(), TPR_THRESHOLD is meaningless.

Do not meddle in the internals of kernels, for they are subtle and quick to panic.