| Message ID | 481368ec2de108b87df4cd11c2bd870b215e49b5.1299233998.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 04, 2011 at 11:20:08AM +0100, Jan Kiszka wrote: > Let kvm_cpu_exec return EXCP_* values consistently and generate those > codes already inside its inner loop. This means we will now re-enter the > kernel while ret == 0. > > Update kvm_handle_internal_error accordingly, but keep > kvm_arch_handle_exit untouched, it will be converted in a separate step. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > kvm-all.c | 26 ++++++++++++++------------ > 1 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 2952499..cc652cf 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run) > fprintf(stderr, "emulation failure\n"); > if (!kvm_arch_stop_on_emulation_error(env)) { > cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); > - return 0; > + return EXCP_INTERRUPT; > } > } > /* FIXME: Should trigger a qmp message to let management know > @@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env) > > if (ret == -EINTR || ret == -EAGAIN) { > DPRINTF("io window exit\n"); > - ret = 0; > + ret = EXCP_INTERRUPT; > break; > } > > @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env) > abort(); > } > > - ret = 0; /* exit loop */ > switch (run->exit_reason) { Better keep ret assignment here so default behaviour is to exit loop? EXCP_INTERRUPT.
On 2011-03-05 17:05, Marcelo Tosatti wrote: > On Fri, Mar 04, 2011 at 11:20:08AM +0100, Jan Kiszka wrote: >> Let kvm_cpu_exec return EXCP_* values consistently and generate those >> codes already inside its inner loop. This means we will now re-enter the >> kernel while ret == 0. >> >> Update kvm_handle_internal_error accordingly, but keep >> kvm_arch_handle_exit untouched, it will be converted in a separate step. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> kvm-all.c | 26 ++++++++++++++------------ >> 1 files changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 2952499..cc652cf 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run) >> fprintf(stderr, "emulation failure\n"); >> if (!kvm_arch_stop_on_emulation_error(env)) { >> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); >> - return 0; >> + return EXCP_INTERRUPT; >> } >> } >> /* FIXME: Should trigger a qmp message to let management know >> @@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env) >> >> if (ret == -EINTR || ret == -EAGAIN) { >> DPRINTF("io window exit\n"); >> - ret = 0; >> + ret = EXCP_INTERRUPT; >> break; >> } >> >> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env) >> abort(); >> } >> >> - ret = 0; /* exit loop */ >> switch (run->exit_reason) { > > Better keep ret assignment here so default behaviour is to > exit loop? EXCP_INTERRUPT. There is no real default behavior: in two cases we stay in the loop, in two others we leave, and the rest obtains ret from a return value. Moreover, if a new case misses to set ret, the compiler will complain. Jan
On Sat, Mar 05, 2011 at 07:12:50PM +0100, Jan Kiszka wrote: > >> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env) > >> abort(); > >> } > >> > >> - ret = 0; /* exit loop */ > >> switch (run->exit_reason) { > > > > Better keep ret assignment here so default behaviour is to > > exit loop? EXCP_INTERRUPT. > > There is no real default behavior: in two cases we stay in the loop, in > two others we leave, and the rest obtains ret from a return value. > Moreover, if a new case misses to set ret, the compiler will complain. > > Jan It will not complain because "ret" is used to store return value of KVM_RUN.
On 2011-03-11 22:34, Marcelo Tosatti wrote: > On Sat, Mar 05, 2011 at 07:12:50PM +0100, Jan Kiszka wrote: >>>> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env) >>>> abort(); >>>> } >>>> >>>> - ret = 0; /* exit loop */ >>>> switch (run->exit_reason) { >>> >>> Better keep ret assignment here so default behaviour is to >>> exit loop? EXCP_INTERRUPT. >> >> There is no real default behavior: in two cases we stay in the loop, in >> two others we leave, and the rest obtains ret from a return value. >> Moreover, if a new case misses to set ret, the compiler will complain. >> >> Jan > > It will not complain because "ret" is used to store return value > of KVM_RUN. > Right, I'll disentangle this duplicate use of 'ret' (writing patches is likely much better than watching more news this morning...). Jan
diff --git a/kvm-all.c b/kvm-all.c index 2952499..cc652cf 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run) fprintf(stderr, "emulation failure\n"); if (!kvm_arch_stop_on_emulation_error(env)) { cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); - return 0; + return EXCP_INTERRUPT; } } /* FIXME: Should trigger a qmp message to let management know @@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env) if (ret == -EINTR || ret == -EAGAIN) { DPRINTF("io window exit\n"); - ret = 0; + ret = EXCP_INTERRUPT; break; } @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env) abort(); } - ret = 0; /* exit loop */ switch (run->exit_reason) { case KVM_EXIT_IO: DPRINTF("handle_io\n"); @@ -965,7 +964,7 @@ int kvm_cpu_exec(CPUState *env) run->io.direction, run->io.size, run->io.count); - ret = 1; + ret = 0; break; case KVM_EXIT_MMIO: DPRINTF("handle_mmio\n"); @@ -973,14 +972,16 @@ int kvm_cpu_exec(CPUState *env) run->mmio.data, run->mmio.len, run->mmio.is_write); - ret = 1; + ret = 0; break; case KVM_EXIT_IRQ_WINDOW_OPEN: DPRINTF("irq_window_open\n"); + ret = EXCP_INTERRUPT; break; case KVM_EXIT_SHUTDOWN: DPRINTF("shutdown\n"); qemu_system_reset_request(); + ret = EXCP_INTERRUPT; break; case KVM_EXIT_UNKNOWN: fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n", @@ -997,28 +998,29 @@ int kvm_cpu_exec(CPUState *env) DPRINTF("kvm_exit_debug\n"); if (kvm_arch_debug(&run->debug.arch)) { ret = EXCP_DEBUG; - goto out; + break; } /* re-enter, this exception was guest-internal */ - ret = 1; + ret = 0; break; #endif /* KVM_CAP_SET_GUEST_DEBUG */ default: DPRINTF("kvm_arch_handle_exit\n"); ret = kvm_arch_handle_exit(env, run); + if (ret == 0) { + ret = EXCP_INTERRUPT; + } else if (ret > 0) { + ret = 0; + } break; } - } while (ret > 0); + } while (ret == 0); if (ret < 0) { cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); vm_stop(VMSTOP_PANIC); } - ret = EXCP_INTERRUPT; -#ifdef KVM_CAP_SET_GUEST_DEBUG -out: -#endif env->exit_request = 0; cpu_single_env = NULL; return ret;
Let kvm_cpu_exec return EXCP_* values consistently and generate those codes already inside its inner loop. This means we will now re-enter the kernel while ret == 0. Update kvm_handle_internal_error accordingly, but keep kvm_arch_handle_exit untouched, it will be converted in a separate step. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- kvm-all.c | 26 ++++++++++++++------------ 1 files changed, 14 insertions(+), 12 deletions(-)