Re: BUG with Win7 and user-return-notifier

[Avi Kivity <avi@redhat.com>]

On 10/29/2009 05:45 PM, Jan Kiszka wrote:
>
>> static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
>> {
>> ...
>> 	for (i = 0; i<  NR_VMX_MSR; ++i) {
>> 		u32 index = vmx_msr_index[i];
>> 		u32 data_low, data_high;
>> 		u64 data;
>> 		int j = vmx->nmsrs;
>>
>> 		if (rdmsr_safe(index,&data_low,&data_high)<  0)
>> 			continue;
>> 		if (wrmsr_safe(index, data_low, data_high)<  0)
>> 			continue;
>> 		data = data_low | ((u64)data_high<<  32);
>> 		vmx->guest_msrs[j].index = i;
>> 		vmx->guest_msrs[j].data = 0;
>>      
>                                          ^^^^^
> Local 'data' drops on the floor. Is that correct (then it deserves a
> cleanup)? Previous version did a "guest = host".
>    

Arguably, it's more correct than what we used to have.  This code dates 
back to the day when kvm started in 64-bit mode and so we copied 
important MSRs from the host.

Shouldn't matter for our case.

>> static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
>> {
>> 	struct vcpu_vmx *vmx = to_vmx(vcpu);
>> 	struct shared_msr_entry *msr = find_msr_entry(vmx, MSR_EFER);
>>
>> 	if (!msr)
>> 		return;
>> 	vcpu->arch.shadow_efer = efer;
>> 	if (!msr)
>> 		return;
>>      
> One "if (!msr)" too much - really the second one?
>    

Yes.  (Either really - if there is no host EFER, the only legal value 
for efer is 0, so whether we update it or not doesn't matter).

Likely introduced by a bad merge.  How code rots.

-- 
error compiling committee.c: too many arguments to function