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
