Skip to content

Commit

Permalink
target-i386: yield to another VCPU on PAUSE
Browse files Browse the repository at this point in the history
After commit b1bbfe7 (aio / timers: On timer modification, qemu_notify
or aio_notify, 2013-08-21) FreeBSD guests report a huge slowdown.

The problem shows up as soon as FreeBSD turns out its periodic (~1 ms)
tick, but the timers are only the trigger for a pre-existing problem.

Before the offending patch, setting a timer did a timer_settime system call.

After, setting the timer exits the event loop (which uses poll) and
reenters it with a new deadline.  This does not cause any slowdown; the
difference is between one system call (timer_settime and a signal
delivery (SIGALRM) before the patch, and two system calls afterwards
(write to a pipe or eventfd + calling poll again when re-entering the
event loop).

Unfortunately, the exit/enter causes the main loop to grab the iothread
lock, which in turns kicks the VCPU thread out of execution.  This
causes TCG to execute the next VCPU in its round-robin scheduling of
VCPUS.  When the second VCPU is mostly unused, FreeBSD runs a "pause"
instruction in its idle loop which only burns cycles without any
progress.  As soon as the timer tick expires, the first VCPU runs
the interrupt handler but very soon it sets it again---and QEMU
then goes back doing nothing in the second VCPU.

The fix is to make the pause instruction do "cpu_loop_exit".

Cc: Richard Henderson <rth@twiddle.net>
Reported-by: Luigi Rizzo <rizzo@iet.unipi.it>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Message-id: 1384948442-24217-1-git-send-email-pbonzini@redhat.com
Signed-off-by: Anthony Liguori <aliguori@amazon.com>
  • Loading branch information
bonzini authored and Anthony Liguori committed Nov 21, 2013
1 parent 1eb1bd9 commit 81f3053
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
1 change: 1 addition & 0 deletions target-i386/helper.h
Expand Up @@ -58,6 +58,7 @@ DEF_HELPER_2(sysret, void, env, int)
DEF_HELPER_2(hlt, void, env, int)
DEF_HELPER_2(monitor, void, env, tl)
DEF_HELPER_2(mwait, void, env, int)
DEF_HELPER_2(pause, void, env, int)
DEF_HELPER_1(debug, void, env)
DEF_HELPER_1(reset_rf, void, env)
DEF_HELPER_3(raise_interrupt, void, env, int, int)
Expand Down
22 changes: 20 additions & 2 deletions target-i386/misc_helper.c
Expand Up @@ -566,6 +566,15 @@ void helper_rdmsr(CPUX86State *env)
}
#endif

static void do_pause(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;

/* Just let another CPU run. */
env->exception_index = EXCP_INTERRUPT;
cpu_loop_exit(env);
}

static void do_hlt(X86CPU *cpu)
{
CPUState *cs = CPU(cpu);
Expand Down Expand Up @@ -611,13 +620,22 @@ void helper_mwait(CPUX86State *env, int next_eip_addend)
cs = CPU(cpu);
/* XXX: not complete but not completely erroneous */
if (cs->cpu_index != 0 || CPU_NEXT(cs) != NULL) {
/* more than one CPU: do not sleep because another CPU may
wake this one */
do_pause(cpu);
} else {
do_hlt(cpu);
}
}

void helper_pause(CPUX86State *env, int next_eip_addend)
{
X86CPU *cpu = x86_env_get_cpu(env);

cpu_svm_check_intercept_param(env, SVM_EXIT_PAUSE, 0);
env->eip += next_eip_addend;

do_pause(cpu);
}

void helper_debug(CPUX86State *env)
{
env->exception_index = EXCP_DEBUG;
Expand Down
5 changes: 4 additions & 1 deletion target-i386/translate.c
Expand Up @@ -7224,7 +7224,10 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
goto do_xchg_reg_eax;
}
if (prefixes & PREFIX_REPZ) {
gen_svm_check_intercept(s, pc_start, SVM_EXIT_PAUSE);
gen_update_cc_op(s);
gen_jmp_im(pc_start - s->cs_base);
gen_helper_pause(cpu_env, tcg_const_i32(s->pc - pc_start));
s->is_jmp = DISAS_TB_JUMP;
}
break;
case 0x9b: /* fwait */
Expand Down

0 comments on commit 81f3053

Please sign in to comment.