On Tue, May 24, 2011, Tian, Kevin wrote about "RE: [PATCH 18/31] nVMX: 
Implement VMLAUNCH and VMRESUME":
> > +   /*
> > +    * Switch from L1's VMCS (vmcs01), to L2's VMCS (vmcs02). Remember
> > +    * vmcs01, on which CPU it was last loaded, and whether it was launched
> > +    * (we need all these values next time we will use L1). Then recall
> > +    * these values from the last time vmcs02 was used.
> > +    */
> > +   saved_vmcs02 = nested_get_current_vmcs02(vmx);
> > +   if (!saved_vmcs02)
> > +           return -ENOMEM;
> > +
> > +   cpu = get_cpu();
> > +   vmx->nested.saved_vmcs01.vmcs = vmx->vmcs;
> > +   vmx->nested.saved_vmcs01.cpu = vcpu->cpu;
> > +   vmx->nested.saved_vmcs01.launched = vmx->launched;
> > +   vmx->vmcs = saved_vmcs02->vmcs;
> > +   vcpu->cpu = saved_vmcs02->cpu;
> 
> this may be another valid reason for your check on cpu_online in your
> latest [08/31] local_vcpus_link fix, since cpu may be offlined after
> this assignment. :-)

I believe that wrapping this part of the code with get_cpu()/put_cpu()
protected me from these kinds of race conditions.

By the way, please note that this part of the code was changed after my
latest loaded_vmcs overhaul. It now looks like this:

        vmcs02 = nested_get_current_vmcs02(vmx);
        if (!vmcs02)
                return -ENOMEM;

        cpu = get_cpu();
        vmx->loaded_vmcs = vmcs02;
        vmx_vcpu_put(vcpu);
        vmx_vcpu_load(vcpu, cpu);
        vcpu->cpu = cpu;
        put_cpu();

(if Avi gives me the green light, I'll send the entire, up-to-date, patch set
again).

> > +   vmcs12->launch_state = 1;
> > +
> > +   prepare_vmcs02(vcpu, vmcs12);
> 
> Since prepare_vmcs may fail, add a check here and move launch_state
> assignment after its success?

prepare_vmcs02() cannot fail. All the checks that need to be done on vmcs12
are done before calling it, in nested_vmx_run().

Currently, there's a single case where prepare_vmcs02 "fails" when it fails
to access apic_access_addr memory. This is wrong - the check should have been
done earlier. I'll fix that, and make prepare_vmcs02() void.


-- 
Nadav Har'El                        |      Tuesday, May 24 2011, 20 Iyyar 5771
[email protected]             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |It's no use crying over spilt milk -- it
http://nadav.harel.org.il           |only makes it salty for the cat.
--
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