Re: [PATCH]: pointer to vmcs getting lost

[Jesse <jdutton@neuraliq.com>]

Thanks for the feedback. Comments inline.

Marcelo Tosatti wrote:
> Hi Jesse,
>
> On Fri, Aug 01, 2008 at 03:18:52PM -0700, Jesse wrote:
>   
>> Greetings,
>>
>> I noticed a race condition when running two guests simultaneously and  
>> debugging both guests (on 64-bit intel cpus). Periodically I would get  
>> errors from the vmread, vmwrite, or vmresume instructions. Some research  
>> revealed that these errors were being caused by having an invalid vmcs  
>> loaded. Further, I found that the vmcs is a per_cpu variable, which I  
>> believe means that any reference to it is invalid after a context  
>> switch. (Corrections appreciated). This means that the vmcs must be  
>> reloaded each time the process is switched to. 
>>     
>
> The preempt notifiers will do that for you.
>   
Right, but they won't call VMPTRLD. For some reason this matters (for 
intel chips), even if the variable ends up back in the same place, as 
far as I can tell.
>   
>> The patch below fixed the  
>> problem for me.
>>
>> This patch does three things.
>> 1. Extends the critical section in __vcpu_run to include the handling of  
>> vmexits, where many of the vmread/writes occur.
>> 2. Perform a vcpu_load after we enter the critical section, and after we  
>> return from kvm_resched.
>> 3. Move the call to kvm_guest_debug_pre into the critical section  
>> (because it calls vmread/write).
>>     
>
> Wouldnt it suffice to move ->guest_debug_pre into the non preemptable
> section? http://article.gmane.org/gmane.comp.emulators.kvm.devel/20244 
>   
Excellent. I hadn't seen that patch yet. However, many of the 
vmreads/vmwrites that failed in my testing were in the exit handlers. 
And a calling VMPTRLD (in vcpu_load) explicitly on entering the critical 
section secures any other vmcs concurrency problems.
> I haven't tested that patch though.
>
>