Re: [PATCH] kvm-qemu: Proper vm_stop on debug events

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> When a vcpu exits after hitting a debug exception, we have to invoke
>> vm_stop(EXCP_DEBUG). But this has to take place over the io-thread.
>>
>> This patch introduces kvm_debug_stop_requested to signal this event, and
>> it takes care that the interrupted vcpu itself goes immediately into
>> stop state.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> ---
>>  qemu/qemu-kvm.c |    9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> Index: b/qemu/qemu-kvm.c
>> ===================================================================
>> --- a/qemu/qemu-kvm.c
>> +++ b/qemu/qemu-kvm.c
>> @@ -58,6 +58,8 @@ pthread_t io_thread;
>>  static int io_thread_fd = -1;
>>  static int io_thread_sigfd = -1;
>>  
>> +static int kvm_debug_stop_requested;
>> +
>>   
> 
> Why use this instead of just keying off of exception_index == EXCP_DEBUG?
> 
>>  static inline unsigned long kvm_get_thread_id(void)
>>  {
>>      return syscall(SYS_gettid);
>> @@ -517,6 +519,10 @@ int kvm_main_loop(void)
>>              qemu_system_powerdown();
>>          else if (qemu_reset_requested())
>>          qemu_kvm_system_reset();
>> +    else if (kvm_debug_stop_requested) {
>> +        kvm_debug_stop_requested = 0;
>> +        vm_stop(EXCP_DEBUG);
>> +    }
>>      }
>>  
>>      pause_all_threads();
>> @@ -529,7 +535,8 @@ static int kvm_debug(void *opaque, int v
>>  {
>>      CPUState *env = cpu_single_env;
>>  
>> -    env->exception_index = EXCP_DEBUG;
>> +    kvm_debug_stop_requested = 1;
>> +    vcpu_info[vcpu].stopped = 1;
>>   
> 
> This isn't quite right.  In the very least, you need to set stopping = 0
> and signal on the qemu_pause_cond.  Thinking it through more though, a
> breakpoint should stop all VCPUs, right?

It does already, give it a try.

> 
> vm_stop(EXCP_DEBUG) will actually do this.  It invokes the
> vm_state_notify callbacks and the io-thread registers one.  I think you
> should probably just issue vm_stop(EXCP_DEBUG) from kvm_debug.

As explained, it can't be called over the vcpu contexts (there is even
an assert() fence against it). This policy has been recently agreed on
while fixing other deadlock issues of qemu-kvm.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]