* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-18 13:45 [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt Denis V. Lunev
@ 2019-07-18 13:52 ` Paolo Bonzini
2019-07-19 13:00 ` Denis V. Lunev
2019-07-19 20:53 ` Eduardo Habkost, Denis V. Lunev
2019-08-14 11:36 ` Jiri Denemark
2 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2019-07-18 13:52 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel
Cc: Nikolay Shirokovskiy, Peter Krempa, Daniel P . Berrangé,
Eduardo Habkost, Richard Henderson
On 18/07/19 15:45, Denis V. Lunev wrote:
> There are the following flags available in libvirt inside cpu_map.xm
> <feature name='cvt16'>
> <cpuid function='0x80000001' ecx='0x00040000'/>
> </feature>
> <feature name='cmt'> <!-- cqm -->
> <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
> </feature>
> We have faced the problem that QEMU does not start once these flags are
> present in the domain.xml.
>
> This patch just adds proper names into the map.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> CC: Peter Krempa <pkrempa@redhat.com>
> CC: Daniel P. Berrangé <berrange@redhat.com>
> ---
> target/i386/cpu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 805ce95247..88ba4dad47 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -870,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "lahf-lm", "cmp-legacy", "svm", "extapic",
> "cr8legacy", "abm", "sse4a", "misalignsse",
> "3dnowprefetch", "osvw", "ibs", "xop",
> - "skinit", "wdt", NULL, "lwp",
> + "skinit", "wdt", "cvt16", "lwp",
> "fma4", "tce", NULL, "nodeid-msr",
> NULL, "tbm", "topoext", "perfctr-core",
> "perfctr-nb", NULL, NULL, NULL,
> @@ -1044,7 +1044,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "fsgsbase", "tsc-adjust", NULL, "bmi1",
> "hle", "avx2", NULL, "smep",
> "bmi2", "erms", "invpcid", "rtm",
> - NULL, NULL, "mpx", NULL,
> + "cmt", NULL, "mpx", NULL,
> "avx512f", "avx512dq", "rdseed", "adx",
> "smap", "avx512ifma", "pcommit", "clflushopt",
> "clwb", "intel-pt", "avx512pf", "avx512er",
>
Oops, nice catch! I've queued the patch for 4.1.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-18 13:52 ` Paolo Bonzini
@ 2019-07-19 13:00 ` Denis V. Lunev
0 siblings, 0 replies; 8+ messages in thread
From: Denis V. Lunev @ 2019-07-19 13:00 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: Nikolay Shirokovskiy, Peter Krempa, Daniel P . Berrangé,
Eduardo Habkost, Richard Henderson
On 7/18/19 4:52 PM, Paolo Bonzini wrote:
> On 18/07/19 15:45, Denis V. Lunev wrote:
>> There are the following flags available in libvirt inside cpu_map.xm
>> <feature name='cvt16'>
>> <cpuid function='0x80000001' ecx='0x00040000'/>
>> </feature>
>> <feature name='cmt'> <!-- cqm -->
>> <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
>> </feature>
>> We have faced the problem that QEMU does not start once these flags are
>> present in the domain.xml.
>>
>> This patch just adds proper names into the map.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Richard Henderson <rth@twiddle.net>
>> CC: Eduardo Habkost <ehabkost@redhat.com>
>> CC: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
>> CC: Peter Krempa <pkrempa@redhat.com>
>> CC: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>> target/i386/cpu.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 805ce95247..88ba4dad47 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -870,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>> "lahf-lm", "cmp-legacy", "svm", "extapic",
>> "cr8legacy", "abm", "sse4a", "misalignsse",
>> "3dnowprefetch", "osvw", "ibs", "xop",
>> - "skinit", "wdt", NULL, "lwp",
>> + "skinit", "wdt", "cvt16", "lwp",
>> "fma4", "tce", NULL, "nodeid-msr",
>> NULL, "tbm", "topoext", "perfctr-core",
>> "perfctr-nb", NULL, NULL, NULL,
>> @@ -1044,7 +1044,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>> "fsgsbase", "tsc-adjust", NULL, "bmi1",
>> "hle", "avx2", NULL, "smep",
>> "bmi2", "erms", "invpcid", "rtm",
>> - NULL, NULL, "mpx", NULL,
>> + "cmt", NULL, "mpx", NULL,
>> "avx512f", "avx512dq", "rdseed", "adx",
>> "smap", "avx512ifma", "pcommit", "clflushopt",
>> "clwb", "intel-pt", "avx512pf", "avx512er",
>>
> Oops, nice catch! I've queued the patch for 4.1.
>
> Paolo
I have written small script to find differences between
CPU features in QEMU and libvirt.
#!/bin/bash
LIST=`awk "/name/{split(\\\$2, arr, \"'\"); print arr[2]}" x86_features.xml`
for feat in $LIST; do
var=`grep \"$feat\" target/i386/cpu.c`
if [ -z "$var" ]; then
echo $feat
fi
done
There are the following list of features present in libvirt
and missed in QEMU:
osxsave - removed in qemu in f1a23522b03
ospke - removed in qemu in 9ccb9784b57
pconfig - removed in qemu in 712f807e196.
mbm_total
mbm_local
Two last features are described as follows:
<!-- cpuid level 0x0000000f:1 (edx) -->
<feature name='mbm_total'>
<cpuid eax_in='0x0f' ecx_in='0x01' edx='0x00000002'/>
</feature>
<feature name='mbm_local'>
<cpuid eax_in='0x0f' ecx_in='0x01' edx='0x00000004'/>
</feature>
This leaf is not supported in QEMU at all.
According to Intel 64 and IA-32 Architecture Developer manual
vol. 2a these bits are used for
EDX Bit 00: Supports L3 occupancy monitoring if 1.
Bit 01: Supports L3 Total Bandwidth monitoring if 1.
Bit 02: Supports L3 Local Bandwidth monitoring if 1.
Bits 31 - 03: Reserved.
Thus technically these 5 bits are able to produce problems
for QEMU if they will be found in domain.xml
Den
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-18 13:45 [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt Denis V. Lunev
2019-07-18 13:52 ` Paolo Bonzini
@ 2019-07-19 20:53 ` Eduardo Habkost, Denis V. Lunev
2019-07-19 21:44 ` Paolo Bonzini
2019-08-14 11:36 ` Jiri Denemark
2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost, Denis V. Lunev @ 2019-07-19 20:53 UTC (permalink / raw)
Cc: Peter Krempa, Daniel P . Berrangé,
qemu-devel, Nikolay Shirokovskiy, Paolo Bonzini, Jiri Denemark,
Richard Henderson
On Thu, Jul 18, 2019 at 04:45:37PM +0300, Denis V. Lunev wrote:
> There are the following flags available in libvirt inside cpu_map.xm
> <feature name='cvt16'>
> <cpuid function='0x80000001' ecx='0x00040000'/>
This is bit 18...
> </feature>
> <feature name='cmt'> <!-- cqm -->
> <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
> </feature>
> We have faced the problem that QEMU does not start once these flags are
> present in the domain.xml.
>
> This patch just adds proper names into the map.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> CC: Peter Krempa <pkrempa@redhat.com>
> CC: Daniel P. Berrangé <berrange@redhat.com>
> ---
> target/i386/cpu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 805ce95247..88ba4dad47 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -870,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "lahf-lm", "cmp-legacy", "svm", "extapic",
> "cr8legacy", "abm", "sse4a", "misalignsse",
> "3dnowprefetch", "osvw", "ibs", "xop",
> - "skinit", "wdt", NULL, "lwp",
> + "skinit", "wdt", "cvt16", "lwp",
...this is bit 14.
Anyway, the cvt16 bit was removed on purpose, and was never
supported. See:
http://mid.mail-archive.com/508091FB.1030705@amd.com
> "fma4", "tce", NULL, "nodeid-msr",
> NULL, "tbm", "topoext", "perfctr-core",
> "perfctr-nb", NULL, NULL, NULL,
> @@ -1044,7 +1044,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "fsgsbase", "tsc-adjust", NULL, "bmi1",
> "hle", "avx2", NULL, "smep",
> "bmi2", "erms", "invpcid", "rtm",
> - NULL, NULL, "mpx", NULL,
> + "cmt", NULL, "mpx", NULL,
This is one is named "cqm" on Linux (X86_FEATURE_CQM). I prefer
to keep consistency with the name already in use by Linux than
the one in libvirt that was never used.
You can still add a "cmt" alias property if you think it would be
useful.
Also, I see no code implementing migration of MSR_IA32_QM_EVTSEL,
MSR_IA32_QM_CTR, or other RDT-M state. If the feature is not
safe for migration yet, you need to explicitly add the feature to
.unmigratable_flags.
> "avx512f", "avx512dq", "rdseed", "adx",
> "smap", "avx512ifma", "pcommit", "clflushopt",
> "clwb", "intel-pt", "avx512pf", "avx512er",
> --
> 2.17.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-19 20:53 ` Eduardo Habkost, Denis V. Lunev
@ 2019-07-19 21:44 ` Paolo Bonzini
2019-07-19 22:05 ` Eduardo Habkost
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2019-07-19 21:44 UTC (permalink / raw)
To: Eduardo Habkost, Denis V. Lunev
Cc: Peter Krempa, Daniel P. Berrangé,
qemu-devel, Nikolay Shirokovskiy, Jiri Denemark,
Richard Henderson
On 19/07/19 22:53, Eduardo Habkost wrote:
> This is one is named "cqm" on Linux (X86_FEATURE_CQM). I prefer
> to keep consistency with the name already in use by Linux than
> the one in libvirt that was never used.
>
> You can still add a "cmt" alias property if you think it would be
> useful.
Actually KVM does not mark it as supported:
const u32 kvm_cpuid_7_0_ebx_x86_features =
F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-19 21:44 ` Paolo Bonzini
@ 2019-07-19 22:05 ` Eduardo Habkost
2019-07-19 22:16 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2019-07-19 22:05 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Krempa, Daniel P. Berrangé,
qemu-devel, Nikolay Shirokovskiy, Denis V. Lunev, Jiri Denemark,
Richard Henderson
On Fri, Jul 19, 2019 at 11:44:57PM +0200, Paolo Bonzini wrote:
> On 19/07/19 22:53, Eduardo Habkost wrote:
> > This is one is named "cqm" on Linux (X86_FEATURE_CQM). I prefer
> > to keep consistency with the name already in use by Linux than
> > the one in libvirt that was never used.
> >
> > You can still add a "cmt" alias property if you think it would be
> > useful.
>
> Actually KVM does not mark it as supported:
>
> const u32 kvm_cpuid_7_0_ebx_x86_features =
> F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
> F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
> F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
> F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
> F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
We can still add the feature name if we also set it on
.unmigratable_features, but I don't see why it would be useful.
Is anybody working to support exposing RDT-M to guests?
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-19 22:05 ` Eduardo Habkost
@ 2019-07-19 22:16 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2019-07-19 22:16 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Krempa, Daniel P. Berrangé,
qemu-devel, Nikolay Shirokovskiy, Denis V. Lunev, Jiri Denemark,
Richard Henderson
On 20/07/19 00:05, Eduardo Habkost wrote:
>> Actually KVM does not mark it as supported:
>>
>> const u32 kvm_cpuid_7_0_ebx_x86_features =
>> F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
>> F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
>> F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
>> F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
>> F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
> We can still add the feature name if we also set it on
> .unmigratable_features, but I don't see why it would be useful.
> Is anybody working to support exposing RDT-M to guests?
I don't think so.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
2019-07-18 13:45 [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt Denis V. Lunev
2019-07-18 13:52 ` Paolo Bonzini
2019-07-19 20:53 ` Eduardo Habkost, Denis V. Lunev
@ 2019-08-14 11:36 ` Jiri Denemark
2 siblings, 0 replies; 8+ messages in thread
From: Jiri Denemark @ 2019-08-14 11:36 UTC (permalink / raw)
To: Denis V. Lunev
Cc: Peter Krempa, Daniel P . Berrangé,
Eduardo Habkost, qemu-devel, Nikolay Shirokovskiy, Paolo Bonzini,
Richard Henderson
On Thu, Jul 18, 2019 at 16:45:37 +0300, Denis V. Lunev wrote:
> There are the following flags available in libvirt inside cpu_map.xm
> <feature name='cvt16'>
> <cpuid function='0x80000001' ecx='0x00040000'/>
> </feature>
> <feature name='cmt'> <!-- cqm -->
> <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/>
> </feature>
> We have faced the problem that QEMU does not start once these flags are
> present in the domain.xml.
Libvirt should not add this to the XML by itself (when using host-model
CPU, for example) so the user must have asked for the feature
explicitly. Thus I don't see any problem with QEMU refusing to start
with such configuration. And the workaround is easy, just don't do it.
I'm not sure about cvt16, but IIRC cmt and mbm_* features were added as
a way to detect whether the host CPU supports perf monitoring counters.
I think tt was not the brightest idea, but there's no reason why QEMU
should support enabling these features. Unless it actually makes sense
for QEMU.
If there are any issues with libvirt passing these features to QEMU
without explicit request from the user, we should address them in
libvirt.
Jirka
^ permalink raw reply [flat|nested] 8+ messages in thread