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

[Eduardo Habkost]

On Fri, May 05, 2017 at 12:46:21PM +0100, Daniel P. Berrange wrote:
> 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.

That's true, even if we add a hypervisor-cpuid property, we need
a way to represent the behavior of QEMU 2.9 (disabling hypervisor
CPUID only for TCG).

> 
> Having a separate variable just for TCG seems the only viable option
> to me - eg
> 
>     DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),

Sounds good to me.

-- 
Eduardo