Re: [PATCH]: pointer to vmcs getting lost

[Marcelo Tosatti <mtosatti@redhat.com>]

On Fri, Aug 01, 2008 at 04:36:03PM -0700, Jesse wrote:
> 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.

They will: kvm_sched_in -> kvm_arch_vcpu_load -> vmx_vcpu_load:

    struct vcpu_vmx *vmx = to_vmx(vcpu)
    
    if (per_cpu(current_vmcs, cpu) != vmx->vmcs) {
        ...

>>> 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.

Do you have preempt notifiers enabled in your host kernel?