[libvirt PATCH] cpu_map: Remove intel-pt from x86 CPU models

Jiri Denemark posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/dc0291b2950c725727a3ea3b3c70fc1d453c9b76.1611556143.git.jdenemar@redhat.com
src/cpu_map/x86_Icelake-Client-noTSX.xml                        | 2 +-
src/cpu_map/x86_Icelake-Client.xml                              | 2 +-
src/cpu_map/x86_Icelake-Server-noTSX.xml                        | 2 +-
src/cpu_map/x86_Icelake-Server.xml                              | 2 +-
tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml        | 1 +
tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml         | 1 +
.../cpu-Icelake-Server-pconfig.x86_64-3.1.0.args                | 2 +-
.../cpu-Icelake-Server-pconfig.x86_64-latest.args               | 2 +-
8 files changed, 8 insertions(+), 6 deletions(-)
[libvirt PATCH] cpu_map: Remove intel-pt from x86 CPU models
Posted by Jiri Denemark 3 years, 2 months ago
As explained in QEMU commit 4c257911dcc7c4189768e9651755c849ce9db4e8
intel-pt features should never be included in the CPU models as it was
not supported by KVM back then and even once it started to be supported,
users have to enable it by passing pt_mode=1 parameter to kvm_intel
module. The Icelake-* CPU models with intel-pt included were added to
QEMU 3.1.0 and removed right in the following 4.0.0 release (and even in
3.1.1 maintenance release).

In libvirt 6.10.0 I introduced 'removed' attribute for features included
in our CPU model definitions which we can use to drop intel-pt from
Icelake-* CPU models. Back then I explained we can safely do so only for
features which could never be enabled, which is not the case of intel-pt.

Theoretically, it could be possible to create an environment in which
QEMU would enable intel-pt without asking for it explicitly: it would
need to use a new enough kernel (not available at the time of QEMU
3.1.0) and pt_mode KVM parameter in combination with QEMU 3.1.0 running
a domain with q35 machine type and all that on a CPU which didn't really
exist at that time.

Migrating such domain to a host with newer SW stack including libvirt
with this patch applied would result in incompatible guest ABI (the
virtual CPU would lose intel-pt). However, QEMU changed its CPU models
unconditionally and thus migration would not work even without this
patch. That said, it is safe to follow QEMU and remove the feature from
Icelake-* CPU models in our cpu_map.

https://bugzilla.redhat.com/show_bug.cgi?id=1853972

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu_map/x86_Icelake-Client-noTSX.xml                        | 2 +-
 src/cpu_map/x86_Icelake-Client.xml                              | 2 +-
 src/cpu_map/x86_Icelake-Server-noTSX.xml                        | 2 +-
 src/cpu_map/x86_Icelake-Server.xml                              | 2 +-
 tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml        | 1 +
 tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml         | 1 +
 .../cpu-Icelake-Server-pconfig.x86_64-3.1.0.args                | 2 +-
 .../cpu-Icelake-Server-pconfig.x86_64-latest.args               | 2 +-
 8 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/cpu_map/x86_Icelake-Client-noTSX.xml b/src/cpu_map/x86_Icelake-Client-noTSX.xml
index 65e648ae21..217adba4f2 100644
--- a/src/cpu_map/x86_Icelake-Client-noTSX.xml
+++ b/src/cpu_map/x86_Icelake-Client-noTSX.xml
@@ -30,7 +30,7 @@
     <feature name='fsgsbase'/>
     <feature name='fxsr'/>
     <feature name='gfni'/>
-    <feature name='intel-pt'/>
+    <feature name='intel-pt' removed='yes'/>
     <feature name='invpcid'/>
     <feature name='lahf_lm'/>
     <feature name='lm'/>
diff --git a/src/cpu_map/x86_Icelake-Client.xml b/src/cpu_map/x86_Icelake-Client.xml
index 5cf32e91fa..b725227f46 100644
--- a/src/cpu_map/x86_Icelake-Client.xml
+++ b/src/cpu_map/x86_Icelake-Client.xml
@@ -31,7 +31,7 @@
     <feature name='fxsr'/>
     <feature name='gfni'/>
     <feature name='hle'/>
-    <feature name='intel-pt'/>
+    <feature name='intel-pt' removed='yes'/>
     <feature name='invpcid'/>
     <feature name='lahf_lm'/>
     <feature name='lm'/>
diff --git a/src/cpu_map/x86_Icelake-Server-noTSX.xml b/src/cpu_map/x86_Icelake-Server-noTSX.xml
index 34a0f7c18c..7c9c32c977 100644
--- a/src/cpu_map/x86_Icelake-Server-noTSX.xml
+++ b/src/cpu_map/x86_Icelake-Server-noTSX.xml
@@ -37,7 +37,7 @@
     <feature name='fsgsbase'/>
     <feature name='fxsr'/>
     <feature name='gfni'/>
-    <feature name='intel-pt'/>
+    <feature name='intel-pt' removed='yes'/>
     <feature name='invpcid'/>
     <feature name='la57'/>
     <feature name='lahf_lm'/>
diff --git a/src/cpu_map/x86_Icelake-Server.xml b/src/cpu_map/x86_Icelake-Server.xml
index 1ee4ea9cd4..b4685bead0 100644
--- a/src/cpu_map/x86_Icelake-Server.xml
+++ b/src/cpu_map/x86_Icelake-Server.xml
@@ -38,7 +38,7 @@
     <feature name='fxsr'/>
     <feature name='gfni'/>
     <feature name='hle'/>
-    <feature name='intel-pt'/>
+    <feature name='intel-pt' removed='yes'/>
     <feature name='invpcid'/>
     <feature name='la57'/>
     <feature name='lahf_lm'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
index 3a71b28cfb..7fcd20d26d 100644
--- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
@@ -21,6 +21,7 @@
   <feature policy='require' name='tsc_adjust'/>
   <feature policy='require' name='cmt'/>
   <feature policy='require' name='avx512ifma'/>
+  <feature policy='require' name='intel-pt'/>
   <feature policy='require' name='sha-ni'/>
   <feature policy='require' name='ospke'/>
   <feature policy='require' name='rdpid'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
index 1582de0422..07e8f8bc24 100644
--- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
@@ -22,6 +22,7 @@
   <feature name='tsc_adjust'/>
   <feature name='cmt'/>
   <feature name='avx512ifma'/>
+  <feature name='intel-pt'/>
   <feature name='sha-ni'/>
   <feature name='ospke'/>
   <feature name='rdpid'/>
diff --git a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-3.1.0.args b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-3.1.0.args
index 96d4306238..b69bfe0f0f 100644
--- a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-3.1.0.args
+++ b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-3.1.0.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-test/master-key.aes \
 -machine pc-i440fx-3.1,accel=kvm,usb=off,dump-guest-core=off \
--cpu Icelake-Server,pconfig=off \
+-cpu Icelake-Server,pconfig=off,intel-pt=off \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-latest.args
index a512623af6..64f223bee3 100644
--- a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-test/master-key.aes \
 -machine pc,accel=kvm,usb=off,dump-guest-core=off,memory-backend=pc.ram \
--cpu Icelake-Server \
+-cpu Icelake-Server,intel-pt=off \
 -m 214 \
 -object memory-backend-ram,id=pc.ram,size=224395264 \
 -overcommit mem-lock=off \
-- 
2.30.0

Re: [libvirt PATCH] cpu_map: Remove intel-pt from x86 CPU models
Posted by Tim Wiederhake 3 years, 2 months ago
On Mon, 2021-01-25 at 07:29 +0100, Jiri Denemark wrote:
> As explained in QEMU commit 4c257911dcc7c4189768e9651755c849ce9db4e8
> intel-pt features should never be included in the CPU models as it
> was
> not supported by KVM back then and even once it started to be
> supported,
> users have to enable it by passing pt_mode=1 parameter to kvm_intel
> module. The Icelake-* CPU models with intel-pt included were added to
> QEMU 3.1.0 and removed right in the following 4.0.0 release (and even
> in
> 3.1.1 maintenance release).
> 
> In libvirt 6.10.0 I introduced 'removed' attribute for features
> included
> in our CPU model definitions which we can use to drop intel-pt from
> Icelake-* CPU models. Back then I explained we can safely do so only
> for
> features which could never be enabled, which is not the case of
> intel-pt.
> 
> Theoretically, it could be possible to create an environment in which
> QEMU would enable intel-pt without asking for it explicitly: it would
> need to use a new enough kernel (not available at the time of QEMU
> 3.1.0) and pt_mode KVM parameter in combination with QEMU 3.1.0
> running
> a domain with q35 machine type and all that on a CPU which didn't
> really
> exist at that time.
> 
> Migrating such domain to a host with newer SW stack including libvirt
> with this patch applied would result in incompatible guest ABI (the
> virtual CPU would lose intel-pt). However, QEMU changed its CPU
> models
> unconditionally and thus migration would not work even without this
> patch. That said, it is safe to follow QEMU and remove the feature
> from
> Icelake-* CPU models in our cpu_map.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1853972
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/cpu_map/x86_Icelake-Client-noTSX.xml                        | 2
> +-
>  src/cpu_map/x86_Icelake-Client.xml                              | 2
> +-
>  src/cpu_map/x86_Icelake-Server-noTSX.xml                        | 2
> +-
>  src/cpu_map/x86_Icelake-Server.xml                              | 2
> +-
>  tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml        | 1
> +
>  tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml         | 1
> +
>  .../cpu-Icelake-Server-pconfig.x86_64-3.1.0.args                | 2
> +-
>  .../cpu-Icelake-Server-pconfig.x86_64-latest.args               | 2
> +-
>  8 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/cpu_map/x86_Icelake-Client-noTSX.xml
> b/src/cpu_map/x86_Icelake-Client-noTSX.xml
> index 65e648ae21..217adba4f2 100644
> --- a/src/cpu_map/x86_Icelake-Client-noTSX.xml
> +++ b/src/cpu_map/x86_Icelake-Client-noTSX.xml
> @@ -30,7 +30,7 @@
>      <feature name='fsgsbase'/>
>      <feature name='fxsr'/>
>      <feature name='gfni'/>
> -    <feature name='intel-pt'/>
> +    <feature name='intel-pt' removed='yes'/>
>      <feature name='invpcid'/>
>      <feature name='lahf_lm'/>
>      <feature name='lm'/>
> diff --git a/src/cpu_map/x86_Icelake-Client.xml
> b/src/cpu_map/x86_Icelake-Client.xml
> index 5cf32e91fa..b725227f46 100644
> --- a/src/cpu_map/x86_Icelake-Client.xml
> +++ b/src/cpu_map/x86_Icelake-Client.xml
> @@ -31,7 +31,7 @@
>      <feature name='fxsr'/>
>      <feature name='gfni'/>
>      <feature name='hle'/>
> -    <feature name='intel-pt'/>
> +    <feature name='intel-pt' removed='yes'/>
>      <feature name='invpcid'/>
>      <feature name='lahf_lm'/>
>      <feature name='lm'/>
> diff --git a/src/cpu_map/x86_Icelake-Server-noTSX.xml
> b/src/cpu_map/x86_Icelake-Server-noTSX.xml
> index 34a0f7c18c..7c9c32c977 100644
> --- a/src/cpu_map/x86_Icelake-Server-noTSX.xml
> +++ b/src/cpu_map/x86_Icelake-Server-noTSX.xml
> @@ -37,7 +37,7 @@
>      <feature name='fsgsbase'/>
>      <feature name='fxsr'/>
>      <feature name='gfni'/>
> -    <feature name='intel-pt'/>
> +    <feature name='intel-pt' removed='yes'/>
>      <feature name='invpcid'/>
>      <feature name='la57'/>
>      <feature name='lahf_lm'/>
> diff --git a/src/cpu_map/x86_Icelake-Server.xml
> b/src/cpu_map/x86_Icelake-Server.xml
> index 1ee4ea9cd4..b4685bead0 100644
> --- a/src/cpu_map/x86_Icelake-Server.xml
> +++ b/src/cpu_map/x86_Icelake-Server.xml
> @@ -38,7 +38,7 @@
>      <feature name='fxsr'/>
>      <feature name='gfni'/>
>      <feature name='hle'/>
> -    <feature name='intel-pt'/>
> +    <feature name='intel-pt' removed='yes'/>
>      <feature name='invpcid'/>
>      <feature name='la57'/>
>      <feature name='lahf_lm'/>
> diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml 
> b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
> index 3a71b28cfb..7fcd20d26d 100644
> --- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
> +++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-guest.xml
> @@ -21,6 +21,7 @@
>    <feature policy='require' name='tsc_adjust'/>
>    <feature policy='require' name='cmt'/>
>    <feature policy='require' name='avx512ifma'/>
> +  <feature policy='require' name='intel-pt'/>
>    <feature policy='require' name='sha-ni'/>
>    <feature policy='require' name='ospke'/>
>    <feature policy='require' name='rdpid'/>
> diff --git a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
> b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
> index 1582de0422..07e8f8bc24 100644
> --- a/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
> +++ b/tests/cputestdata/x86_64-cpuid-Ice-Lake-Server-host.xml
> @@ -22,6 +22,7 @@
>    <feature name='tsc_adjust'/>
>    <feature name='cmt'/>
>    <feature name='avx512ifma'/>
> +  <feature name='intel-pt'/>
>    <feature name='sha-ni'/>
>    <feature name='ospke'/>
>    <feature name='rdpid'/>
> diff --git a/tests/qemuxml2argvdata/cpu-Icelake-Server-
> pconfig.x86_64-3.1.0.args b/tests/qemuxml2argvdata/cpu-Icelake-
> Server-pconfig.x86_64-3.1.0.args
> index 96d4306238..b69bfe0f0f 100644
> --- a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-
> 3.1.0.args
> +++ b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-
> 3.1.0.args
> @@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
>  -object secret,id=masterKey0,format=raw,\
>  file=/tmp/lib/domain--1-test/master-key.aes \
>  -machine pc-i440fx-3.1,accel=kvm,usb=off,dump-guest-core=off \
> --cpu Icelake-Server,pconfig=off \
> +-cpu Icelake-Server,pconfig=off,intel-pt=off \
>  -m 214 \
>  -overcommit mem-lock=off \
>  -smp 1,sockets=1,cores=1,threads=1 \
> diff --git a/tests/qemuxml2argvdata/cpu-Icelake-Server-
> pconfig.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-Icelake-
> Server-pconfig.x86_64-latest.args
> index a512623af6..64f223bee3 100644
> --- a/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-
> latest.args
> +++ b/tests/qemuxml2argvdata/cpu-Icelake-Server-pconfig.x86_64-
> latest.args
> @@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
>  -object secret,id=masterKey0,format=raw,\
>  file=/tmp/lib/domain--1-test/master-key.aes \
>  -machine pc,accel=kvm,usb=off,dump-guest-core=off,memory-
> backend=pc.ram \
> --cpu Icelake-Server \
> +-cpu Icelake-Server,intel-pt=off \
>  -m 214 \
>  -object memory-backend-ram,id=pc.ram,size=224395264 \
>  -overcommit mem-lock=off \

Reviewed-by: Tim Wiederhake <twiederh@redhat.com>