On Tue, May 09, 2017 at 08:52:28AM -0300, Eduardo Habkost wrote:
> On Tue, May 09, 2017 at 12:20:34PM +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 signature is used by software to detect which
> > virtual environment they are running in and (potentially)
> > change behaviour in certain ways. For example, systemd
> > supports a ConditionVirtualization= setting in unit files.
> > The virt-what command can also report the virt type it is
> > running on
> >
> > Currently both these apps have to resort to custom hacks
> > like looking for 'fw-cfg' entry in the /proc/device-tree
> > file to identify TCG.
> >
> > This change thus proposes a signature "TCGTCGTCGTCG" to be
> > reported when running under TCG.
> >
> > To hide this, the -cpu option tcg-cpuid=off can be used.
> >
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> > include/hw/i386/pc.h | 5 +++++
> > target/i386/cpu.c | 22 ++++++++++++++++++++++
> > target/i386/cpu.h | 1 +
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index f278b3a..3aec60f 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -376,6 +376,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> > uint64_t *);
> > #define PC_COMPAT_2_8 \
> > HW_COMPAT_2_8 \
> > {\
> > + .driver = TYPE_X86_CPU,\
> > + .property = "tcg-cpuid",\
> > + .value = "off",\
> > + },\
> > + {\
> > .driver = "kvmclock",\
> > .property = "x-mach-use-reliable-get-clock",\
> > .value = "off",\
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 8cb4af4..ee4f515 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -2627,12 +2627,15 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> > index, uint32_t count,
> > CPUState *cs = CPU(cpu);
> > uint32_t pkg_offset;
> > uint32_t limit;
> > + uint32_t signature[3];
> >
> > /* Calculate & apply limits for different index ranges */
> > if (index >= 0xC0000000) {
> > limit = env->cpuid_xlevel2;
> > } else if (index >= 0x80000000) {
> > limit = env->cpuid_xlevel;
> > + } else if (index & 0x40000000) {
>
> Why did you decide to use (index & 0x40000000) instead of
> (index >= 0x40000000)? The latter would be more consistent with
> the rest of the code.
It was just a mistake.
> > + limit = 0x40000000;
>
> This is not strictly wrong, but it will make CPUID[0x40000001]
> return arbitrary bits (CPUID[env->cpuid_level]). Guests aren't
> supposed to look at CPUID[0x40000001] without checking
> CPUID[0x40000000] first, but probably it's safer to set
> limit = 0x40000001 and return all-zeroes on CPUID[0x40000001],
> just in case.
Ok
> > } else {
> > limit = env->cpuid_level;
> > }
> > @@ -2867,6 +2870,24 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> > uint32_t count,
> > }
> > break;
> > }
> > + case 0x40000000:
> > + /*
> > + * CPUID code in kvm_arch_init_vcpu() ignores stuff
> > + * set here, but we restrict to TCG none the less.
> > + */
> > + if (tcg_enabled() && cpu->expose_tcg) {
> > + memcpy(signature, "TCGTCGTCGTCG", 12);
> > + *eax = 0;
>
> On both KVM and Hyper-V, CPUID[0x40000000].EAX is the maximum
> CPUID function. KVM has the additional exception that eax=0 on
> the output is interpreted as 0x40000001.
>
> Setting this to 0x40000000 or 0x40000001 would make things more
> consistent for guests. Setting it to 0x40000001 would make it
> safer (see my other comment above).
Yep, makes sense.
>
>
> > + *ebx = signature[0];
> > + *ecx = signature[1];
> > + *edx = signature[2];
> > + } else {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + }
> > + break;
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 :|
