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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to