|
From: | Paolo Bonzini |
Subject: | Re: [Qemu-devel] [PATCH v4 01/10] hw/mips/cputimer: Don't start periodic timer in KVM mode |
Date: | Wed, 19 Mar 2014 17:29:40 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
Il 14/03/2014 13:47, James Hogan ha scritto:
From: Sanjay Lal <address@hidden> Compare/Count timer interrupts are handled in-kernel for KVM, so don't bother starting it in QEMU. Signed-off-by: Sanjay Lal <address@hidden> Signed-off-by: James Hogan <address@hidden> Reviewed-by: Aurelien Jarno <address@hidden> --- Changes in v2: - Expand commit message - Rebase on v1.7.0 - Wrap comment --- hw/mips/cputimer.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c index c8b4b00..52570fd 100644 --- a/hw/mips/cputimer.c +++ b/hw/mips/cputimer.c @@ -23,6 +23,7 @@ #include "hw/hw.h" #include "hw/mips/cpudevs.h" #include "qemu/timer.h" +#include "sysemu/kvm.h" #define TIMER_FREQ 100 * 1000 * 1000 @@ -141,7 +142,13 @@ static void mips_timer_cb (void *opaque) void cpu_mips_clock_init (CPUMIPSState *env) { - env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env); - env->CP0_Compare = 0; - cpu_mips_store_count(env, 1); + /* + * If we're in KVM mode, don't start the periodic timer, that is handled in + * kernel. + */ + if (!kvm_enabled()) { + env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env); + env->CP0_Compare = 0; + cpu_mips_store_count(env, 1); + } }
I hate to make you do unrelated changes, but... initializing CP0_Compare is unnecessary, it should already be 0; and for CP0_Count it should not be done here. but in cpu_state_reset function. Then here you can call qemu_register_reset to register another reset callback, and call cpu_mips_timer_update in that callback.
I'm asking because while if (!kvm_enabled()) { env->timer = ... qemu_register_reset(...); } is fine, changing values of registers conditionally is not.Also, I noticed two things in the implementation of the CPU timer that should be fixed:
1) right now the hypervisor's frequency is hardcoded to 1/4th of the host, while QEMU's is 100 MHz. It would be nice to make them either consistent, or customizable (you can use another ONE_REG interface to set CPU parameters).
2) in KVM, CP0_Count does not start at the same value on guest reset. There is a comment that "Linux doesn't seem to write into COUNT", but QEMU does. So KVM should implement CP0_Count writes and adjust the "bias" of the guest CP0_Count.
In fact, right now kvm_mips_te_put_cp0_registers should always return -EINVAL because KVM_REG_MIPS_CP0_COUNT is not handled in kvm_mips_get/set_reg. Am I missing something?
Paolo
[Prev in Thread] | Current Thread | [Next in Thread] |