Patchwork [07/15] kvm: Separate TCG from KVM cpu execution

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 7, 2011, 11:19 a.m.
Message ID <149ef70e3a2ebe96529b0956da4bd4009099e3ac.1297077507.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/82178/
State New
Headers show

Comments

Jan Kiszka - Feb. 7, 2011, 11:19 a.m.
Mixing up TCG bits with KVM already led to problems around eflags
emulation on x86. Moreover, quite some code that TCG requires on cpu
enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
kvm_cpu_exec as early as possible.

The core logic of cpu_halted from cpu_exec is added to
kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
exception_index meaningless for KVM, we can simply pass the exit reason
directly (only "EXCP_DEBUG vs. rest" is relevant).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c        |   19 ++++++-------------
 cpus.c            |   10 +++++-----
 kvm-all.c         |   19 +++++++++----------
 target-i386/kvm.c |    6 +++---
 4 files changed, 23 insertions(+), 31 deletions(-)
Marcelo Tosatti - Feb. 8, 2011, 11:39 p.m.
On Mon, Feb 07, 2011 at 12:19:18PM +0100, Jan Kiszka wrote:
> Mixing up TCG bits with KVM already led to problems around eflags
> emulation on x86. Moreover, quite some code that TCG requires on cpu
> enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
> kvm_cpu_exec as early as possible.
> 
> The core logic of cpu_halted from cpu_exec is added to
> kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
> exception_index meaningless for KVM, we can simply pass the exit reason
> directly (only "EXCP_DEBUG vs. rest" is relevant).
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  cpu-exec.c        |   19 ++++++-------------
>  cpus.c            |   10 +++++-----
>  kvm-all.c         |   19 +++++++++----------
>  target-i386/kvm.c |    6 +++---
>  4 files changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index ba183c4..377a0a3 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
>  
>  int kvm_arch_process_irqchip_events(CPUState *env)
>  {
> +    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
> +        env->halted = 0;
> +    }

Why is it necessary to clear env->halted here?
Jan Kiszka - Feb. 9, 2011, 7:59 a.m.
On 2011-02-09 00:39, Marcelo Tosatti wrote:
> On Mon, Feb 07, 2011 at 12:19:18PM +0100, Jan Kiszka wrote:
>> Mixing up TCG bits with KVM already led to problems around eflags
>> emulation on x86. Moreover, quite some code that TCG requires on cpu
>> enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
>> kvm_cpu_exec as early as possible.
>>
>> The core logic of cpu_halted from cpu_exec is added to
>> kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
>> exception_index meaningless for KVM, we can simply pass the exit reason
>> directly (only "EXCP_DEBUG vs. rest" is relevant).
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  cpu-exec.c        |   19 ++++++-------------
>>  cpus.c            |   10 +++++-----
>>  kvm-all.c         |   19 +++++++++----------
>>  target-i386/kvm.c |    6 +++---
>>  4 files changed, 23 insertions(+), 31 deletions(-)
>>
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index ba183c4..377a0a3 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
>>  
>>  int kvm_arch_process_irqchip_events(CPUState *env)
>>  {
>> +    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
>> +        env->halted = 0;
>> +    }
> 
> Why is it necessary to clear env->halted here?

Because we no longer come along cpu_halted() in cpu_exec(). This
corresponds to the tail of process_irqchip_events() in qemu-kvm

Jan
Marcelo Tosatti - Feb. 9, 2011, 2:44 p.m.
On Wed, Feb 09, 2011 at 08:59:23AM +0100, Jan Kiszka wrote:
> On 2011-02-09 00:39, Marcelo Tosatti wrote:
> > On Mon, Feb 07, 2011 at 12:19:18PM +0100, Jan Kiszka wrote:
> >> Mixing up TCG bits with KVM already led to problems around eflags
> >> emulation on x86. Moreover, quite some code that TCG requires on cpu
> >> enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
> >> kvm_cpu_exec as early as possible.
> >>
> >> The core logic of cpu_halted from cpu_exec is added to
> >> kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
> >> exception_index meaningless for KVM, we can simply pass the exit reason
> >> directly (only "EXCP_DEBUG vs. rest" is relevant).
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  cpu-exec.c        |   19 ++++++-------------
> >>  cpus.c            |   10 +++++-----
> >>  kvm-all.c         |   19 +++++++++----------
> >>  target-i386/kvm.c |    6 +++---
> >>  4 files changed, 23 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index ba183c4..377a0a3 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
> >>  
> >>  int kvm_arch_process_irqchip_events(CPUState *env)
> >>  {
> >> +    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
> >> +        env->halted = 0;
> >> +    }
> > 
> > Why is it necessary to clear env->halted here?
> 
> Because we no longer come along cpu_halted() in cpu_exec(). This
> corresponds to the tail of process_irqchip_events() in qemu-kvm

This is not yet well integrated, we probably don't need env->halted
anymore (see cpu_has_work). Can be improved later though.
Jan Kiszka - Feb. 9, 2011, 2:53 p.m.
On 2011-02-09 15:44, Marcelo Tosatti wrote:
> On Wed, Feb 09, 2011 at 08:59:23AM +0100, Jan Kiszka wrote:
>> On 2011-02-09 00:39, Marcelo Tosatti wrote:
>>> On Mon, Feb 07, 2011 at 12:19:18PM +0100, Jan Kiszka wrote:
>>>> Mixing up TCG bits with KVM already led to problems around eflags
>>>> emulation on x86. Moreover, quite some code that TCG requires on cpu
>>>> enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
>>>> kvm_cpu_exec as early as possible.
>>>>
>>>> The core logic of cpu_halted from cpu_exec is added to
>>>> kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
>>>> exception_index meaningless for KVM, we can simply pass the exit reason
>>>> directly (only "EXCP_DEBUG vs. rest" is relevant).
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  cpu-exec.c        |   19 ++++++-------------
>>>>  cpus.c            |   10 +++++-----
>>>>  kvm-all.c         |   19 +++++++++----------
>>>>  target-i386/kvm.c |    6 +++---
>>>>  4 files changed, 23 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>>> index ba183c4..377a0a3 100644
>>>> --- a/target-i386/kvm.c
>>>> +++ b/target-i386/kvm.c
>>>> @@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
>>>>  
>>>>  int kvm_arch_process_irqchip_events(CPUState *env)
>>>>  {
>>>> +    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
>>>> +        env->halted = 0;
>>>> +    }
>>>
>>> Why is it necessary to clear env->halted here?
>>
>> Because we no longer come along cpu_halted() in cpu_exec(). This
>> corresponds to the tail of process_irqchip_events() in qemu-kvm
> 
> This is not yet well integrated, we probably don't need env->halted
> anymore (see cpu_has_work). Can be improved later though.

So far we check for it, at least in cpu_[thread_]is_idle. And that's a
generic service.

Jan

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 9c0b10d..b03b3a7 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -226,13 +226,11 @@  int cpu_exec(CPUState *env1)
     }
 
 #if defined(TARGET_I386)
-    if (!kvm_enabled()) {
-        /* put eflags in CPU temporary format */
-        CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-        DF = 1 - (2 * ((env->eflags >> 10) & 1));
-        CC_OP = CC_OP_EFLAGS;
-        env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-    }
+    /* put eflags in CPU temporary format */
+    CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    DF = 1 - (2 * ((env->eflags >> 10) & 1));
+    CC_OP = CC_OP_EFLAGS;
+    env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
 #elif defined(TARGET_SPARC)
 #elif defined(TARGET_M68K)
     env->cc_op = CC_OP_FLAGS;
@@ -257,7 +255,7 @@  int cpu_exec(CPUState *env1)
         if (setjmp(env->jmp_env) == 0) {
 #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
 #undef env
-                    env = cpu_single_env;
+            env = cpu_single_env;
 #define env cpu_single_env
 #endif
             /* if an exception is pending, we execute it here */
@@ -316,11 +314,6 @@  int cpu_exec(CPUState *env1)
                 }
             }
 
-            if (kvm_enabled()) {
-                kvm_cpu_exec(env);
-                longjmp(env->jmp_env, 1);
-            }
-
             next_tb = 0; /* force lookup of first TB */
             for(;;) {
                 interrupt_request = env->interrupt_request;
diff --git a/cpus.c b/cpus.c
index d4b5e80..2414316 100644
--- a/cpus.c
+++ b/cpus.c
@@ -800,8 +800,6 @@  static void qemu_kvm_wait_io_event(CPUState *env)
     qemu_wait_io_event_common(env);
 }
 
-static int qemu_cpu_exec(CPUState *env);
-
 static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
@@ -829,7 +827,7 @@  static void *qemu_kvm_cpu_thread_fn(void *arg)
 
     while (1) {
         if (cpu_can_run(env)) {
-            r = qemu_cpu_exec(env);
+            r = kvm_cpu_exec(env);
             if (r == EXCP_DEBUG) {
                 cpu_handle_debug_exception(env);
             }
@@ -1040,7 +1038,7 @@  void vm_stop(int reason)
 
 #endif
 
-static int qemu_cpu_exec(CPUState *env)
+static int tcg_cpu_exec(CPUState *env)
 {
     int ret;
 #ifdef CONFIG_PROFILER
@@ -1095,9 +1093,11 @@  bool cpu_exec_all(void)
             break;
         }
         if (cpu_can_run(env)) {
-            r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
+                r = kvm_cpu_exec(env);
                 qemu_kvm_eat_signals(env);
+            } else {
+                r = tcg_cpu_exec(env);
             }
             if (r == EXCP_DEBUG) {
                 cpu_handle_debug_exception(env);
diff --git a/kvm-all.c b/kvm-all.c
index 19cf188..802c6b8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -895,10 +895,11 @@  int kvm_cpu_exec(CPUState *env)
 
     if (kvm_arch_process_irqchip_events(env)) {
         env->exit_request = 0;
-        env->exception_index = EXCP_HLT;
-        return 0;
+        return EXCP_HLT;
     }
 
+    cpu_single_env = env;
+
     do {
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
@@ -927,7 +928,6 @@  int kvm_cpu_exec(CPUState *env)
         kvm_flush_coalesced_mmio_buffer();
 
         if (ret == -EINTR || ret == -EAGAIN) {
-            cpu_exit(env);
             DPRINTF("io window exit\n");
             ret = 0;
             break;
@@ -978,8 +978,8 @@  int kvm_cpu_exec(CPUState *env)
             DPRINTF("kvm_exit_debug\n");
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
-                env->exception_index = EXCP_DEBUG;
-                return 0;
+                ret = EXCP_DEBUG;
+                goto out;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
@@ -995,13 +995,12 @@  int kvm_cpu_exec(CPUState *env)
     if (ret < 0) {
         cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
         vm_stop(VMSTOP_PANIC);
-        env->exit_request = 1;
-    }
-    if (env->exit_request) {
-        env->exit_request = 0;
-        env->exception_index = EXCP_INTERRUPT;
     }
+    ret = EXCP_INTERRUPT;
 
+out:
+    env->exit_request = 0;
+    cpu_single_env = NULL;
     return ret;
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ba183c4..377a0a3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1502,12 +1502,13 @@  int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 
 int kvm_arch_process_irqchip_events(CPUState *env)
 {
+    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+        env->halted = 0;
+    }
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
         kvm_cpu_synchronize_state(env);
         do_cpu_init(env);
-        env->exception_index = EXCP_HALTED;
     }
-
     if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
         kvm_cpu_synchronize_state(env);
         do_cpu_sipi(env);
@@ -1522,7 +1523,6 @@  static int kvm_handle_halt(CPUState *env)
           (env->eflags & IF_MASK)) &&
         !(env->interrupt_request & CPU_INTERRUPT_NMI)) {
         env->halted = 1;
-        env->exception_index = EXCP_HLT;
         return 0;
     }