| Message ID | 1447661048-31048-2-git-send-email-haozhong.zhang@intel.com |
|---|---|
| State | New |
| Headers | show |
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@intel.com> > --- > 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. > + 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. > + } 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. > + > ret = kvm_getput_regs(x86_cpu, 1); > if (ret < 0) { > return ret; > -- > 2.4.8 >
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@intel.com> > > --- > > 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
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; + if (r < 0) { + error_report("warning: KVM_GET_TSC_KHZ failed"); + } 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); + } + ret = kvm_getput_regs(x86_cpu, 1); if (ret < 0) { return ret;
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@intel.com> --- target-i386/kvm.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)