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

[Phil Dennis-Jordan]

Thanks for the comments Paulo and Eduardo,

On 18 January 2017 at 16:05, Paolo Bonzini <address@hidden> wrote:
>
> > +    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).
>

Sounds good, I've fixed this and the page/leaf terminology mixup for the
next patch iteration.

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.
>

That makes sense. Darwin can't handle changing TSC frequencies in any case,
regardless of cpuid leaf 0x40000010. Do I deduce correctly from the
following code (lines 967~977) that this bit inhibits migration
intrinsically, so other than depending on it, I don't need to specifically
disable migration for this option?

if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
…
        vmstate_x86_cpu.unmigratable = 1;

(Likewise, it would appear that the user_tsc_khz case Eduardo suggested
already has a migration sanity check in cpu_post_load() too.)

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.
>

Sorry, you've lost me here. Would you mind explaining in a little more
detail? What would I be enabling unconditionally? (I'm getting lost on what
the various 'this'/'that'/'it' are referring to.)

> +    if (cpu->vmware_clock_rates) {
>
> ^^ Here is where you should also check invtsc.
>
> > +        if (cpu->expose_kvm
>
> I think this should not depend on cpu->expose_kvm.  This is not a KVM
> leaf, it's a vmware leaf; if it were a KVM leaf, it would obey kvm_base.
>  Of course checking kvm_base is still a good idea, to avoid stomping on
> Hyper-V's CPUID space.
>

Hmm, my thinking here is that leaf 0x40000000 only is published if kvm or
Hyper-V is exposed. Without 0x40000000, Darwin won't find 0x40000010. Like
you say, this option is mutually exclusive with the Hyper-V leaves - so
either we rely on the KVM version of leaf 0x40000000 (my code so far), or
this option will have to explicitly expose 0x40000000. I'm not sure what
signature (ebx-edx) we should give it in the latter case - using VMWare's
seems off as it doesn't replicate any other of their leaves. Given that
exposing KVM is the default, anyone disabling it presumably has a reason
for not wanting page 0x40000000 exposed. Thoughts?