Re: [Qemu-devel] [PATCH RFC] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf

[Daniel P. Berrange]

On Thu, May 04, 2017 at 03:42:41PM -0300, Eduardo Habkost wrote:
> On Thu, May 04, 2017 at 03:56:58PM +0100, Daniel P. Berrange wrote:
> > Currently when running KVM, we expose "KVMKVMKVM\0\0\0" in
> > the 0x40000000 CPUID leaf. Other hypervisors (VMWare,
> > HyperV, Xen, BHyve) all do the same thing, which leaves
> > TCG as the odd one out.
> > 
> > The CPUID is used by software to detect when running in a
> > virtual environment and change behaviour in certain ways.
> > For example, systemd supports a ConditionVirtualization=
> > setting in unit files. Currently they have to resort to
> > custom hacks like looking for 'fw-cfg' entry in the
> > /proc/device-tree file. The virt-what command has the
> > same hacks & needs.
> > 
> > This change thus proposes a signature TCGTCGTCGTCG to be
> > reported when running under TCG.
> > 
> > NB1, for reasons I don't undersatnd 'cpu_x86_cpuid' function
> > clamps the requested CPUID leaf based on env->cpuid_level.

[snip]

> > NB2, for KVM, we added a flag for '-cpu kvm=off' to let you
> > hide the KVMKVMKVM signature from guests. Presumably we should
> > add a 'tcg=off' flag for the same reason ?
> 
> I don't like "kvm=off" because it sounds like it disables KVM
> completely. "tcg=off" would be misleading as well.
> 
> What about a generic "hypervisor-cpuid=off" property?

Assuming that is intended to obsolete the existing 'kvm' parameter too,
I'm not seeing a way to introduce that in a backwards compatible safe
manner.

eg we add

    DEFINE_PROP_BOOL("hypervisor-cpuid", X86CPU, expose_hypervisor, true),

and in legacy machine types we need to set

    {\
        .driver   = TYPE_X86_CPU,\
        .property = "hypervisor-cpuid",\
        .value    = "off",\
    },\

to prevent TCGTCGTCGTCG appearing.


Now in current KVM code we have

    if (cpu->expose_kvm) {
        memcpy(signature, "KVMKVMKVM\0\0\0", 12);
        ....
    }

if we add 'expose_hypervisor' as a generic variable and thus change
existing KVM code to

    if (cpu->expose_kvm || cpu->expose_hypervisor)

then to disable the KVMKVMKVMKVM signature, we now need to set *two*
flags - kvm=off, and hypervisor-cpuid=off and this will break existing
apps that are only setting kvm=off to disable it.

Conversely if we do

    if (cpu->expose_kvm && cpu->expose_hypervisor)

we're going to break KVMKVMKVM signature reporting by default, since
we need to set hypervisor-cpuid=off in back compat machine types
to prevent TCGTCGTCGTCG being exposed.

So it seems we're doomed either way, as we can't distinguish between
a boolean value that is at its default, vs a boolean value that has
been explicitly set to a value that matches the default.

Having a separate variable just for TCG seems the only viable option
to me - eg

    DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|