do not enter vcpu again if it was stopped during IO

PATCH Applied Gleb Natapov 4 participants 8 messages Jun 21, 2010 – Jun 23, 2010 KVM

Share & tools

Gleb Natapov

author

2010-06-21 09:01 UTC #0 lore raw

PATCH 1/1 do not enter vcpu again if it was stopped during IO

To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.

Signed-off-by Gleb Natapov <[email protected]>

qemu-kvm.c +4 -0

index be1dac2..4f7cf6d 100644
@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);
 
     if (r == -1 && errno != EINTR && errno != EAGAIN) {

vl.c +1 -0

index 9e9c176..dcfab13 100644
@@ -1817,6 +1817,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env) {
         cpu_single_env->stopped = 1;
+        cpu_exit(cpu_single_env);
     }
     qemu_notify_event();
 }

--
Gleb.

Marcelo Tosatti

2010-06-21 20:45 UTC #1 lore raw

Quoted 12 lines · Gleb Natapov

To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm.c +0 -0
index be1dac2..4f7cf6d 100644

qemu-kvm.c int kvm_run(CPUState *env) · line 610

Quoted 8 lines

qemu-kvm.c +4 -0

@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);

Can't you check for env->stopped instead?

Quoted 16 lines

qemu-kvm.c +0 -0

     if (r == -1 && errno != EINTR && errno != EAGAIN) {

vl.c +1 -1

index 9e9c176..dcfab13 100644
@@ -1817,6 +1817,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env) {
         cpu_single_env->stopped = 1;
+        cpu_exit(cpu_single_env);
     }
     qemu_notify_event();
 }
--

Gleb.

Gleb Natapov

author

2010-06-22 05:18 UTC #2 lore raw

Quoted 7 lines · Marcelo Tosatti

Quoted 6 lines · Gleb Natapov
To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.

Quoted 17 lines

Quoted 6 lines
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm.c +0 -0
index be1dac2..4f7cf6d 100644
qemu-kvm.c int kvm_run(CPUState *env) · line 610
Quoted 8 lines
qemu-kvm.c +4 -0
@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);
Can't you check for env->stopped instead?

Why it would be better? exit_request is used exactly for purpose to notify cpu loop that is should exit. Sometimes it may be useful to request cpu exit without stopping the cpu. I see gdbstub uses it, haven't check if it applicable to kvm though.

Quoted 16 lines

Quoted 15 lines

qemu-kvm.c +0 -0

     if (r == -1 && errno != EINTR && errno != EAGAIN) {

vl.c +1 -1

index 9e9c176..dcfab13 100644
@@ -1817,6 +1817,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env) {
         cpu_single_env->stopped = 1;
+        cpu_exit(cpu_single_env);
     }
     qemu_notify_event();
 }
--

Quoted 1 line

Gleb.

--
Gleb.

Marcelo Tosatti

2010-06-22 16:48 UTC #3 lore raw

Quoted 25 lines · Gleb Natapov

Quoted 7 lines · Marcelo Tosatti
Quoted 6 lines · Gleb Natapov
To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.
Quoted 17 lines
Quoted 6 lines
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm.c +0 -0
index be1dac2..4f7cf6d 100644
qemu-kvm.c int kvm_run(CPUState *env) · line 610
Quoted 8 lines
qemu-kvm.c +4 -0
@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);
Can't you check for env->stopped instead?

Quoted 4 lines

Why it would be better? exit_request is used exactly for purpose to notify cpu loop that is should exit. Sometimes it may be useful to request cpu exit without stopping the cpu. I see gdbstub uses it, haven't check if it applicable to kvm though.

Makes sense.

Avi Kivity

Applied

2010-06-23 08:22 UTC #4 lore raw

In reply to Gleb Natapov

Quoted 7 lines · Gleb Natapov

To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.

Applied, thanks.

Does uq/master need such a patch?

Signature · 1 line

error compiling committee.c: too many arguments to function

Gleb Natapov

author

2010-06-23 08:24 UTC #5 lore raw

Quoted 8 lines · Avi Kivity

Quoted 6 lines · Gleb Natapov
To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.

Quoted 4 lines

Applied, thanks.
Does uq/master need such a patch?

The code is very different there. I looks to me broken too, but the fix will be different.

--
Gleb.

Jan Kiszka

2010-06-23 10:17 UTC #6 lore raw

In reply to Gleb Natapov

Quoted 27 lines · Gleb Natapov

To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm.c +4 -0
index be1dac2..4f7cf6d 100644
@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);
 
     if (r == -1 && errno != EINTR && errno != EAGAIN) {
vl.c +0 -0
index 9e9c176..dcfab13 100644

vl.c void qemu_system_reset_request(void) · line 1823

Quoted 7 lines

vl.c +1 -0

@@ -1817,6 +1817,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env) {
         cpu_single_env->stopped = 1;
+        cpu_exit(cpu_single_env);
     }
     qemu_notify_event();
 }

What does this second hunk do, specifically in the context of I/O processing? The changelog does not mention it explicitly (or I'm missing the obvious).

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Gleb Natapov

author

2010-06-23 10:21 UTC #7 lore raw

Quoted 31 lines · Jan Kiszka

Quoted 30 lines · Gleb Natapov
To prevent reentering vcpu after IO completion it is not enough to set env->stopped since it is checked only in main loop but control will not get there until next non-IO exit since kvm_run() will reenter vcpu to complete IO instruction. Solve this by sending self-signal to request exit after IO instruction completion.
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm.c +4 -0
index be1dac2..4f7cf6d 100644
@@ -603,6 +603,10 @@ int kvm_run(CPUState *env)
     r = pre_kvm_run(kvm, env);
     if (r)
         return r;
+    if (env->exit_request) {
+        env->exit_request = 0;
+        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    }
     r = ioctl(fd, KVM_RUN, 0);
 
     if (r == -1 && errno != EINTR && errno != EAGAIN) {
vl.c +0 -0
index 9e9c176..dcfab13 100644
@@ -1817,6 +1817,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env) {
         cpu_single_env->stopped = 1;

Quoted 9 lines

vl.c void qemu_system_reset_request(void) · line 1823
Quoted 4 lines
vl.c +1 -0
+        cpu_exit(cpu_single_env);
     }
     qemu_notify_event();
 }
What does this second hunk do, specifically in the context of I/O processing? The changelog does not mention it explicitly (or I'm missing the obvious).

It sets env->exit_request. If qemu_system_reset_request() inside io handler (happens during S3) we should not return to vcpu till resume, or terrible things will happen. You are probably right that it should have been send as different patch.

--
Gleb.