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

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

[Jan Kiszka]

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;
+
 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;
     return 1;
 }
 

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

[Anthony Liguori]

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?

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.

Regards,

Anthony Liguori

>      return 1;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

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

[Jan Kiszka]

[-- 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 --]