Re: [Qemu-devel] [PATCH v5 3/4] Plumb the HAXM-based hardware acceleration support

[Yu Ning <yu.ning@linux.intel.com>]

On 1/6/2017 5:38, Paolo Bonzini wrote:
>
> On 05/01/2017 15:01, Paolo Bonzini wrote:
>>
>> In fact there is a race anyway:
>>
>>          if (cpu->exit_request) {
>>              ret = 1;
>>              break;
>>          }
>> 					cpu->exit_request
>> 					SuspendThread
>> 					ResumeThread
>>          hax_vcpu_interrupt(env);
>>          qemu_mutex_unlock_iothread();
>>          hax_ret = hax_vcpu_run(vcpu);
>>
>> and the same race is true for QueueUserAPC.  It's rare enough that I
>> guess we can accept the patches with just a FIXME comment, but...  Yu
>> Ning, can you tell us what user_event_pending is for? :)  My hunch is
>> that we should call hax_raise_event after setting cpu->exit_request, like
>>
>> 	hax_raise_event();
>> 	/* write user_event_pending before exit_request */
>> 	smp_wmb();
>> 	cpu->exit_request = 1;
>> 	SuspendThread/ResumeThread
>> 		(or QueueUserAPC)
>>
>> and in the hax thread:
>>
>>          if (cpu->exit_request) {
>> 	    cpu->hax_vcpu->tunnel->user_event_pending = 0;
>>              ret = 1;
>>              break;
>>          }
>>
>>          hax_vcpu_interrupt(env);
>>          qemu_mutex_unlock_iothread();
>>
>> 	/* read exit_request before user_event_pending */
>> 	smp_rmb();
>>          hax_ret = hax_vcpu_run(vcpu);
>>
>> but I would like some more official documentation than my own reverse
>> engineering of the brain of whoever wrote the interface (I have not
>> looked at the HAXM driver binary).

Unfortunately, user_event_pending was introduced in 2011 without proper 
documentation, so I guess even the original author might not have an 
answer now (even if I could find him).  I need more time to analyze the 
HAXM driver code to understand what it's about.  Please feel free to add 
a FIXME for now.

But one thing I've noticed is that the Darwin driver does not test or 
reset this flag properly - it will remain 1 after the first 
hax_raise_event() call.  So removing user_event_pending from the QEMU 
side should not affect Mac.