* [Qemu-devel] [PATCH v3 0/2] Support CPUID signature for TCG @ 2017-05-09 11:20 Daniel P. Berrange 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated Daniel P. Berrange 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf Daniel P. Berrange 0 siblings, 2 replies; 6+ messages in thread From: Daniel P. Berrange @ 2017-05-09 11:20 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Daniel P. Berrange This enables report of a signature in CPUID for the TCG interpretor. Changed in v3: - Simplify CPU limit code still further (Eduardo) Changed in v2: - Rewrite the way we bounds check / cap the CPUID index to use a flat switch, instead of nested ifs (Eduardo) - Add a 'tcg-cpuid' property to allow it to be hidden (Eduardo) - Hide the TCG signature for old machine types - Force code to a no-op if tcg_enabled() is false (Eduardo) NB, I did not introduce a general 'hypervisor-cpuid' property to obsolete the existing 'kvm=off|on' -cpu property, since it appears impossible to get the back compat semantics right, as described in a previous reply. Daniel P. Berrange (2): i386: rewrite way CPUID index is validated i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf include/hw/i386/pc.h | 5 +++++ target/i386/cpu.c | 59 +++++++++++++++++++++++++++++++++------------------- target/i386/cpu.h | 1 + 3 files changed, 44 insertions(+), 21 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated 2017-05-09 11:20 [Qemu-devel] [PATCH v3 0/2] Support CPUID signature for TCG Daniel P. Berrange @ 2017-05-09 11:20 ` Daniel P. Berrange 2017-05-09 11:40 ` Eduardo Habkost 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf Daniel P. Berrange 1 sibling, 1 reply; 6+ messages in thread From: Daniel P. Berrange @ 2017-05-09 11:20 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Daniel P. Berrange Change the nested if statements into a flat format, to make it clearer what validation / capping is being performed on different CPUID index values. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- target/i386/cpu.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 13c0985..8cb4af4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2626,28 +2626,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, X86CPU *cpu = x86_env_get_cpu(env); CPUState *cs = CPU(cpu); uint32_t pkg_offset; + uint32_t limit; - /* test if maximum index reached */ - if (index & 0x80000000) { - if (index > env->cpuid_xlevel) { - if (env->cpuid_xlevel2 > 0) { - /* Handle the Centaur's CPUID instruction. */ - if (index > env->cpuid_xlevel2) { - index = env->cpuid_xlevel2; - } else if (index < 0xC0000000) { - index = env->cpuid_xlevel; - } - } else { - /* Intel documentation states that invalid EAX input will - * return the same information as EAX=cpuid_level - * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID) - */ - index = env->cpuid_level; - } - } + /* 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 > env->cpuid_level) - index = env->cpuid_level; + limit = env->cpuid_level; + } + + if (index > limit) { + /* Intel documentation states that invalid EAX input will + * return the same information as EAX=cpuid_level + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID) + */ + index = env->cpuid_level; } switch(index) { -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated Daniel P. Berrange @ 2017-05-09 11:40 ` Eduardo Habkost 0 siblings, 0 replies; 6+ messages in thread From: Eduardo Habkost @ 2017-05-09 11:40 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: qemu-devel, Paolo Bonzini, Richard Henderson On Tue, May 09, 2017 at 12:20:33PM +0100, Daniel P. Berrange wrote: > Change the nested if statements into a flat format, to make > it clearer what validation / capping is being performed on > different CPUID index values. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Probably worth noting in the commit message that the patch changes the results of cpu_x86_cpuid() when (index > env->cpuid_xlevel2), and that it won't have any guest-visible effect because no CPUID[0xC0000001] feature is supported by TCG, and KVM code will never call cpu_x86_cpuid() with (index > env->cpuid_xlevel2). Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target/i386/cpu.c | 35 +++++++++++++++-------------------- > 1 file changed, 15 insertions(+), 20 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 13c0985..8cb4af4 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -2626,28 +2626,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > X86CPU *cpu = x86_env_get_cpu(env); > CPUState *cs = CPU(cpu); > uint32_t pkg_offset; > + uint32_t limit; > > - /* test if maximum index reached */ > - if (index & 0x80000000) { > - if (index > env->cpuid_xlevel) { > - if (env->cpuid_xlevel2 > 0) { > - /* Handle the Centaur's CPUID instruction. */ > - if (index > env->cpuid_xlevel2) { > - index = env->cpuid_xlevel2; > - } else if (index < 0xC0000000) { > - index = env->cpuid_xlevel; > - } > - } else { > - /* Intel documentation states that invalid EAX input will > - * return the same information as EAX=cpuid_level > - * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID) > - */ > - index = env->cpuid_level; > - } > - } > + /* 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 > env->cpuid_level) > - index = env->cpuid_level; > + limit = env->cpuid_level; > + } > + > + if (index > limit) { > + /* Intel documentation states that invalid EAX input will > + * return the same information as EAX=cpuid_level > + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID) > + */ > + index = env->cpuid_level; > } > > switch(index) { > -- > 2.9.3 > -- Eduardo ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf 2017-05-09 11:20 [Qemu-devel] [PATCH v3 0/2] Support CPUID signature for TCG Daniel P. Berrange 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated Daniel P. Berrange @ 2017-05-09 11:20 ` Daniel P. Berrange 2017-05-09 11:52 ` Eduardo Habkost 1 sibling, 1 reply; 6+ messages in thread From: Daniel P. Berrange @ 2017-05-09 11:20 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Daniel P. Berrange 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 <berrange@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) { + limit = 0x40000000; } 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; + *ebx = signature[0]; + *ecx = signature[1]; + *edx = signature[2]; + } else { + *eax = 0; + *ebx = 0; + *ecx = 0; + *edx = 0; + } + break; case 0x80000000: *eax = env->cpuid_xlevel; *ebx = env->cpuid_vendor1; @@ -4008,6 +4029,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration, false), DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true), + DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true), DEFINE_PROP_END_OF_LIST() }; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index c4602ca..c25f0ce 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1216,6 +1216,7 @@ struct X86CPU { bool check_cpuid; bool enforce_cpuid; bool expose_kvm; + bool expose_tcg; bool migratable; bool max_features; /* Enable all supported features automatically */ uint32_t apic_id; -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf Daniel P. Berrange @ 2017-05-09 11:52 ` Eduardo Habkost 2017-05-09 12:39 ` Daniel P. Berrange 0 siblings, 1 reply; 6+ messages in thread From: Eduardo Habkost @ 2017-05-09 11:52 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: qemu-devel, Paolo Bonzini, Richard Henderson 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 <berrange@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. > + 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. > } 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). > + *ebx = signature[0]; > + *ecx = signature[1]; > + *edx = signature[2]; > + } else { > + *eax = 0; > + *ebx = 0; > + *ecx = 0; > + *edx = 0; > + } > + break; > case 0x80000000: > *eax = env->cpuid_xlevel; > *ebx = env->cpuid_vendor1; > @@ -4008,6 +4029,7 @@ static Property x86_cpu_properties[] = { > DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration, > false), > DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true), > + DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true), > DEFINE_PROP_END_OF_LIST() > }; > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index c4602ca..c25f0ce 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1216,6 +1216,7 @@ struct X86CPU { > bool check_cpuid; > bool enforce_cpuid; > bool expose_kvm; > + bool expose_tcg; > bool migratable; > bool max_features; /* Enable all supported features automatically */ > uint32_t apic_id; > -- > 2.9.3 > -- Eduardo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf 2017-05-09 11:52 ` Eduardo Habkost @ 2017-05-09 12:39 ` Daniel P. Berrange 0 siblings, 0 replies; 6+ messages in thread From: Daniel P. Berrange @ 2017-05-09 12:39 UTC (permalink / raw) To: Eduardo Habkost; +Cc: qemu-devel, Paolo Bonzini, Richard Henderson 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 <berrange@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 :| ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-09 12:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-09 11:20 [Qemu-devel] [PATCH v3 0/2] Support CPUID signature for TCG Daniel P. Berrange 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated Daniel P. Berrange 2017-05-09 11:40 ` Eduardo Habkost 2017-05-09 11:20 ` [Qemu-devel] [PATCH v3 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf Daniel P. Berrange 2017-05-09 11:52 ` Eduardo Habkost 2017-05-09 12:39 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).