| Message ID | 149ef70e3a2ebe96529b0956da4bd4009099e3ac.1297077507.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
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?
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
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.
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
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; }
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(-)