| Message ID | 20090513091643.8216.46699.sendpatchset@subratamodak.linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Subrata Modak wrote: > 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)) { > > This function no longer exists; can you check if the current code is susceptible? > (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. > With !kvm_lapic_enabled(), TPR_THRESHOLD is meaningless.
--- a/arch/x86/kvm/vmx.c 2009-05-12 15:28:42.000000000 +0530 +++ b/arch/x86/kvm/vmx.c 2009-05-12 15:51:02.000000000 +0530 @@ -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; }
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(). Signed-Off-By: Subrata Modak <subrata@linux.vnet.ibm.com>, To: Avi Kivity <avi@qumranet.com> To: Yaniv Kamay <yaniv@qumranet.com> To: <kvm@vger.kernel.org> Cc: Balbir Singh <balbir@linux.vnet.ibm.com> Cc: Sachin P Sant <sachinp@linux.vnet.ibm.com> Subject: [PATCH][Resend] Fix Warnining in arch/x86/kvm/vmx.c --- --- Regards-- Subrata -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html