Re: BUG with Win7 and user-return-notifier

Avi Kivity
2009-10-29T16:05:23Z

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