QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH 1/1] x86: add CPU flags supported inside libvirt
@ 2019-07-18 13:45 Denis V. Lunev
  2019-07-18 13:52 ` Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Denis V. Lunev @ 2019-07-18 13:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Krempa, Daniel P . Berrangé,
	Eduardo Habkost, Nikolay Shirokovskiy, Paolo Bonzini,
	Denis V. Lunev, Richard Henderson

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",
-- 
2.17.1



^ 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 13:00   ` Denis V. Lunev
  2019-07-19 20:53 ` Eduardo Habkost, To: "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, To: "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, To: "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, To: "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, To: "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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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, To: "Denis V. Lunev
2019-07-19 21:44   ` Paolo Bonzini
2019-07-19 22:05     ` Eduardo Habkost
2019-07-19 22:16       ` Paolo Bonzini
2019-08-14 11:36 ` Jiri Denemark

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox