Re: [Qemu-devel] [PATCH] x86-KVM: Supply TSC and APIC clock rates to guest like VMWare

[Eduardo Habkost]

On Wed, Jan 18, 2017 at 04:05:59PM +0100, Paolo Bonzini wrote:
> 
> 
> On 18/01/2017 15:24, Phil Dennis-Jordan wrote:
> > ---
> >  target/i386/cpu.c |  1 +
> >  target/i386/cpu.h |  4 ++++
> >  target/i386/kvm.c | 40 ++++++++++++++++++++++++++++++++--------
> >  3 files changed, 37 insertions(+), 8 deletions(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index aba11ae..e5523d4 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3677,6 +3677,7 @@ static Property x86_cpu_properties[] = {
> >      DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
> >      DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
> >      DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
> > +    DEFINE_PROP_BOOL("vmware-tsc-apic-clocks", X86CPU, vmware_clock_rates, 
> > false),
> 
> Maybe just vmware-cpuid-freq instead?  Whatever the choice, please make
> the bool field in struct X86CPU consistent with the property name (e.g.
> enable_vmware_cpuid_freq).
> 
> One issue is that the TSC frequency can change, for example on
> migration.  Telling the guest about the TSC frequency makes little sense
> if it can change.
> 
> So the leaf should be conditional on the INVTSC feature
> (CPUID[0x80000007].EDX bit 8).  You can enable this unconditionally for
> new machine types (i.e. making it true here, and turning it off in
> include/hw/i386/pc.h's PC_COMPAT_2_8 macro), but only expose it if that
> bit is also set.

It can be made conditional on (invtsc || cpu->user_tsc_khz). If
the TSC frequency is configured explicitly, we know it won't
change.

-- 
Eduardo