On 11/16/15 11:39, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 04:04:07PM +0800, Haozhong Zhang wrote:
> > If no user-specified TSC rate is present, we will try to set
> > env->tsc_khz to the value returned by KVM_GET_TSC_KHZ.
> >
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> > ---
> > target-i386/kvm.c | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 2a9953b..9084b29 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
> > return 0;
> > }
> >
> > +static void kvm_arch_set_tsc_khz(CPUState *cs)
>
> > +{
> > + X86CPU *cpu = X86_CPU(cs);
> > + CPUX86State *env = &cpu->env;
> > + int r;
> > +
> > + /*
> > + * If no user-specified TSC frequency is present, we will try to
> > + * set env->tsc_khz to the value used by KVM.
> > + */
> > + if (!env->tsc_khz) {
> > + r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> > + kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) : -ENOTSUP;
>
> Can't we do this on kvm_arch_init_vcpu()? kvm_arch_put_registers()'s purpose is
> to just copy data from QEMU to KVM, not the other way around.
>
I'll move this to kvm_arch_init_vcpu().
>
> > + if (r < 0) {
> > + error_report("warning: KVM_GET_TSC_KHZ failed");
>
> Having a kernel that doesn't support KVM_CAP_GET_TSC_KHZ shouldn't trigger a
> warning every time we run QEMU, unless the user is explicitly asking for a
> feature that requires KVM_GET_TSC_KHZ.
>
I'll remove the warning.
> > + } else {
> > + env->tsc_khz = r;
> > + }
> > + }
> > +}
> > +
> > int kvm_arch_put_registers(CPUState *cpu, int level)
> > {
> > X86CPU *x86_cpu = X86_CPU(cpu);
> > @@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> > }
> > }
> >
> > + if (level == KVM_PUT_FULL_STATE) {
> > + kvm_arch_set_tsc_khz(cpu);
> > + }
>
> I see that kvm_arch_set_tsc_khz() will be extended to call
> KVM_SET_TSC_KHZ in the next patch, so the kvm_arch_set_tsc_khz()
> seems to belong here. But the KVM_GET_TSC_KHZ call doesn't seem
> to belong in kvm_arch_set_tsc_khz()
>
> kvm_arch_put_registers() callers don't expect any QEMU-side data
> to change, but just that KVM data is updated according to the
> QEMU-side data.
>
Good to know this. As above, I'll move the KVM_GET_TSC_KHZ call to
kvm_arch_init_vcpu().
Haozhong
> > +
> > ret = kvm_getput_regs(x86_cpu, 1);
> > if (ret < 0) {
> > return ret;
> > --
> > 2.4.8
> >
>
> --
> Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
