[PATCH for-4.2 0/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[PATCH for-4.2 0/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Eduardo Habkost]

Systems with TSX disabled due to TAA now make existing CPU models
not usable.  Add new versions with TSX disabled so people can
still use them.  See individual patch commit messages for
additional details.

I'm not 100% sure about the new -noTSX aliases, so I'm adding
them in a separate patch.

Eduardo Habkost (2):
  i386: Add new versions of Skylake/Cascadelake/Icelake without TSX
  i386: Add -noTSX aliases for hle=off,rtm=off CPU models

 target/i386/cpu.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

-- 
2.21.0

[PATCH for-4.2 1/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Eduardo Habkost]

One of the mitigation methods for TAA[1] is to disable TSX
support on the host system.  Linux added a mechanism to disable
TSX globally through the kernel command line, and many Linux
distributions now default to tsx=off.  This makes existing CPU
models that have HLE and RTM enabled not usable anymore.

Add new versions of all CPU models that have the HLE and RTM
features enabled, that can be used when TSX is disabled in the
host system.

References:

[1] TAA, TSX asynchronous Abort:
    https://software.intel.com/security-software-guidance/insights/deep-dive-intel-transactional-synchronization-extensions-intel-tsx-asynchronous-abort
    https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/tsx_async_abort.html

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 296b491607..0267e08612 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2474,6 +2474,14 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ }
                 }
             },
+            {
+                .version = 3,
+                .props = (PropValue[]) {
+                    { "hle", "off" },
+                    { "rtm", "off" },
+                    { /* end of list */ }
+                }
+            },
             { /* end of list */ }
         }
     },
@@ -2541,6 +2549,14 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ }
                 }
             },
+            {
+                .version = 3,
+                .props = (PropValue[]) {
+                    { "hle", "off" },
+                    { "rtm", "off" },
+                    { /* end of list */ }
+                }
+            },
             { /* end of list */ }
         }
     },
@@ -2608,6 +2624,13 @@ static X86CPUDefinition builtin_x86_defs[] = {
                   { /* end of list */ }
               },
             },
+            { .version = 3,
+              .props = (PropValue[]) {
+                  { "hle", "off" },
+                  { "rtm", "off" },
+                  { /* end of list */ }
+              },
+            },
             { /* end of list */ }
         }
     },
@@ -2665,6 +2688,18 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_6_EAX_ARAT,
         .xlevel = 0x80000008,
         .model_id = "Intel Core Processor (Icelake)",
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            {
+                .version = 2,
+                .props = (PropValue[]) {
+                    { "hle", "off" },
+                    { "rtm", "off" },
+                    { /* end of list */ }
+                },
+            },
+            { /* end of list */ }
+        }
     },
     {
         .name = "Icelake-Server",
@@ -2723,6 +2758,18 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_6_EAX_ARAT,
         .xlevel = 0x80000008,
         .model_id = "Intel Xeon Processor (Icelake)",
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            {
+                .version = 2,
+                .props = (PropValue[]) {
+                    { "hle", "off" },
+                    { "rtm", "off" },
+                    { /* end of list */ }
+                },
+            },
+            { /* end of list */ }
+        }
     },
     {
         .name = "Denverton",
-- 
2.21.0

[PATCH for-4.2 2/2] i386: Add -noTSX aliases for hle=off, rtm=off CPU models

[Eduardo Habkost]

We have been trying to avoid adding new aliases for CPU model
versions, but in the case of changes in defaults introduced by
the TAA mitigation patches, the aliases might help avoid user
confusion when applying host software updates.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0267e08612..56eb98ef35 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2476,6 +2476,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             },
             {
                 .version = 3,
+                .alias = "Skylake-Client-noTSX-IBRS",
                 .props = (PropValue[]) {
                     { "hle", "off" },
                     { "rtm", "off" },
@@ -2551,6 +2552,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             },
             {
                 .version = 3,
+                .alias = "Skylake-Server-noTSX-IBRS",
                 .props = (PropValue[]) {
                     { "hle", "off" },
                     { "rtm", "off" },
@@ -2625,6 +2627,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
               },
             },
             { .version = 3,
+              .alias = "Cascadelake-Server-noTSX",
               .props = (PropValue[]) {
                   { "hle", "off" },
                   { "rtm", "off" },
@@ -2692,6 +2695,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             { .version = 1 },
             {
                 .version = 2,
+                .alias = "Icelake-Client-noTSX",
                 .props = (PropValue[]) {
                     { "hle", "off" },
                     { "rtm", "off" },
@@ -2762,6 +2766,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             { .version = 1 },
             {
                 .version = 2,
+                .alias = "Icelake-Server-noTSX",
                 .props = (PropValue[]) {
                     { "hle", "off" },
                     { "rtm", "off" },
-- 
2.21.0

Re: [PATCH for-4.2 1/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Paolo Bonzini]

On 20/11/19 17:49, Eduardo Habkost wrote:
> One of the mitigation methods for TAA[1] is to disable TSX
> support on the host system.  Linux added a mechanism to disable
> TSX globally through the kernel command line, and many Linux
> distributions now default to tsx=off.  This makes existing CPU
> models that have HLE and RTM enabled not usable anymore.
> 
> Add new versions of all CPU models that have the HLE and RTM
> features enabled, that can be used when TSX is disabled in the
> host system.

What is the effect of this when using "-cpu CascadeLake-Server" and
upgrading QEMU?  Would it automatically switch to the new version?  If
so, would it be better to include a duplicate of the models (and if so,
that would conflict with my VMX features patch, which is also for 4.2).

Paolo

Re: [PATCH for-4.2 1/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Eduardo Habkost]

On Wed, Nov 20, 2019 at 06:40:06PM +0100, Paolo Bonzini wrote:
> On 20/11/19 17:49, Eduardo Habkost wrote:
> > One of the mitigation methods for TAA[1] is to disable TSX
> > support on the host system.  Linux added a mechanism to disable
> > TSX globally through the kernel command line, and many Linux
> > distributions now default to tsx=off.  This makes existing CPU
> > models that have HLE and RTM enabled not usable anymore.
> > 
> > Add new versions of all CPU models that have the HLE and RTM
> > features enabled, that can be used when TSX is disabled in the
> > host system.
> 
> What is the effect of this when using "-cpu CascadeLake-Server" and
> upgrading QEMU?  Would it automatically switch to the new version?  If
> so, would it be better to include a duplicate of the models (and if so,
> that would conflict with my VMX features patch, which is also for 4.2).

It won't, because PCMachineClass::default_cpu_version==1 for all
versioned PC machine-types, currently.

The plan is to set default_cpu_version=CPU_VERSION_LATEST on
pc-*-5.0 (or, more likely, 5.1).  But this will happen only after
libvirt starts resolving CPU model versions.  See the
"Runnability guarantee of CPU models" section at
qemu-deprecated.texi.

-- 
Eduardo

Re: [PATCH for-4.2 1/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Paolo Bonzini]

On 20/11/19 19:42, Eduardo Habkost wrote:
> The plan is to set default_cpu_version=CPU_VERSION_LATEST on
> pc-*-5.0 (or, more likely, 5.1).  But this will happen only after
> libvirt starts resolving CPU model versions.  See the
> "Runnability guarantee of CPU models" section at
> qemu-deprecated.texi.

Thanks!

Then the patches do not interact negatively with my VMX series.  I have
queued both of them so that we can get the pull request out with all the
TAA bits.

Paolo

Re: [PATCH for-4.2 1/2] i386: Add new versions of Skylake/Cascadelake/Icelake without TSX

[Kashyap Chamarthy]

On Wed, Nov 20, 2019 at 01:49:11PM -0300, Eduardo Habkost wrote:
> One of the mitigation methods for TAA[1] is to disable TSX
> support on the host system.  Linux added a mechanism to disable
> TSX globally through the kernel command line, and many Linux
> distributions now default to tsx=off.  This makes existing CPU
> models that have HLE and RTM enabled not usable anymore.
>
> Add new versions of all CPU models that have the HLE and RTM
> features enabled, that can be used when TSX is disabled in the
> host system.
>
> References:
>
> [1] TAA, TSX asynchronous Abort:
>     https://software.intel.com/security-software-guidance/insights/deep-dive-intel-transactional-synchronization-extensions-intel-tsx-asynchronous-abort
>     https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/tsx_async_abort.html
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---

FWIW:

    Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

Here are _all_ the Cascadelake/Icelake/Skylake variants I see with your
patches applied:

    $> ./qemu-system-x86_64 -cpu help | egrep  '(Cascadelake.*|Icelake.*|Skylake.*)'
    x86 Cascadelake-Server    (alias configured by machine type)
    x86 Cascadelake-Server-noTSX  (alias of Cascadelake-Server-v3)
    x86 Cascadelake-Server-v1  Intel Xeon Processor (Cascadelake)
    x86 Cascadelake-Server-v2  Intel Xeon Processor (Cascadelake)
    x86 Cascadelake-Server-v3  Intel Xeon Processor (Cascadelake)
    x86 Icelake-Client        (alias configured by machine type)
    x86 Icelake-Client-noTSX  (alias of Icelake-Client-v2)
    x86 Icelake-Client-v1     Intel Core Processor (Icelake)
    x86 Icelake-Client-v2     Intel Core Processor (Icelake)
    x86 Icelake-Server        (alias configured by machine type)
    x86 Icelake-Server-noTSX  (alias of Icelake-Server-v2)
    x86 Icelake-Server-v1     Intel Xeon Processor (Icelake)
    x86 Icelake-Server-v2     Intel Xeon Processor (Icelake)
    x86 Skylake-Client        (alias configured by machine type)
    x86 Skylake-Client-IBRS   (alias of Skylake-Client-v2)
    x86 Skylake-Client-noTSX-IBRS  (alias of Skylake-Client-v3)
    x86 Skylake-Client-v1     Intel Core Processor (Skylake)
    x86 Skylake-Client-v2     Intel Core Processor (Skylake, IBRS)
    x86 Skylake-Client-v3     Intel Core Processor (Skylake, IBRS)
    x86 Skylake-Server        (alias configured by machine type)
    x86 Skylake-Server-IBRS   (alias of Skylake-Server-v2)
    x86 Skylake-Server-noTSX-IBRS  (alias of Skylake-Server-v3)
    x86 Skylake-Server-v1     Intel Xeon Processor (Skylake)
    x86 Skylake-Server-v2     Intel Xeon Processor (Skylake, IBRS)
    x86 Skylake-Server-v3     Intel Xeon Processor (Skylake, IBRS)


Test with system QEMU
---------------------

Where `cat system-qemu.sh` is:

    #!/usr/bin/env bash
    args=(
     -display none
     -cpu Skylake-Client-IBRS
     -no-user-config
     -machine q35,accel=kvm
     -nodefaults
     -m 2048
     -serial stdio
     -drive file=/export/vm1.qcow2,format=qcow2,if=virtio
    )
    /usr/bin/qemu-system-x86_64 "${args[@]}"

Run it:

    $> ./system-qemu.sh
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.01H:ECX.aes [bit 25]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.hle [bit 4]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.rtm [bit 11]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.rdseed [bit 18]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.adx [bit 19]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.smap [bit 20]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.80000001H:ECX.3dnowprefetch [bit 8]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.0DH:EAX.xsavec [bit 1]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.0DH:EAX.xgetbv1 [bit 2]
    [...]

Notice that it is (correctly) complaining about about "hle" and "rtm".


Test with patched QEMU
----------------------

Now, with a QEMU built with your both patches, and using the -noTSX-IBRS
model:

Where `cat patched-qemu.sh` is:

    #!/usr/bin/env bash
    args=(
     -display none
     -cpu Skylake-Client-noTSX-IBRS
     -no-user-config
     -machine q35,accel=kvm
     -nodefaults
     -m 2048
     -serial stdio
     -drive file=/export/vm1.qcow2,format=qcow2,if=virtio
    )
    ~/build/qemu/x86_64-softmmu/qemu-system-x86_64 "${args[@]}"

Run it:

    $> ./patched-qemu.sh
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.01H:ECX.aes [bit 25]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.rdseed [bit 18]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.adx [bit 19]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.smap [bit 20]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.80000001H:ECX.3dnowprefetch [bit 8]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.0DH:EAX.xsavec [bit 1]
    qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.0DH:EAX.xgetbv1 [bit 2]
    [...]

Here it doesn't complain (also correctly so) about "hle" and "rtm",
because the -noTSX-IBRS model disabled them :-)


>  target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 296b491607..0267e08612 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2474,6 +2474,14 @@ static X86CPUDefinition builtin_x86_defs[] = {
>                      { /* end of list */ }
>                  }
>              },
> +            {
> +                .version = 3,
> +                .props = (PropValue[]) {
> +                    { "hle", "off" },
> +                    { "rtm", "off" },
> +                    { /* end of list */ }
> +                }
> +            },
>              { /* end of list */ }
>          }
>      },
> @@ -2541,6 +2549,14 @@ static X86CPUDefinition builtin_x86_defs[] = {
>                      { /* end of list */ }
>                  }
>              },
> +            {
> +                .version = 3,
> +                .props = (PropValue[]) {
> +                    { "hle", "off" },
> +                    { "rtm", "off" },
> +                    { /* end of list */ }
> +                }
> +            },
>              { /* end of list */ }
>          }
>      },
> @@ -2608,6 +2624,13 @@ static X86CPUDefinition builtin_x86_defs[] = {
>                    { /* end of list */ }
>                },
>              },
> +            { .version = 3,
> +              .props = (PropValue[]) {
> +                  { "hle", "off" },
> +                  { "rtm", "off" },
> +                  { /* end of list */ }
> +              },
> +            },
>              { /* end of list */ }
>          }
>      },
> @@ -2665,6 +2688,18 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_6_EAX_ARAT,
>          .xlevel = 0x80000008,
>          .model_id = "Intel Core Processor (Icelake)",
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            {
> +                .version = 2,
> +                .props = (PropValue[]) {
> +                    { "hle", "off" },
> +                    { "rtm", "off" },
> +                    { /* end of list */ }
> +                },
> +            },
> +            { /* end of list */ }
> +        }
>      },
>      {
>          .name = "Icelake-Server",
> @@ -2723,6 +2758,18 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_6_EAX_ARAT,
>          .xlevel = 0x80000008,
>          .model_id = "Intel Xeon Processor (Icelake)",
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            {
> +                .version = 2,
> +                .props = (PropValue[]) {
> +                    { "hle", "off" },
> +                    { "rtm", "off" },
> +                    { /* end of list */ }
> +                },
> +            },
> +            { /* end of list */ }
> +        }
>      },
>      {
>          .name = "Denverton",
> --
> 2.21.0
>

--
/kashyap

Re: [PATCH for-4.2 2/2] i386: Add -noTSX aliases for hle=off, rtm=off CPU models

[Kashyap Chamarthy]

On Wed, Nov 20, 2019 at 01:49:12PM -0300, Eduardo Habkost wrote:
> We have been trying to avoid adding new aliases for CPU model
> versions, but in the case of changes in defaults introduced by
> the TAA mitigation patches, the aliases might help avoid user
> confusion when applying host software updates.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

    https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg03501.html

            - - -

Should we (can do it, if you already don't have a patch WIP for it)
also update this file to reflect this?
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qemu-cpu-models.texi

While at it ... I wonder if it's worth making a separte doc
(versioned-cpu-models.rst) explaining the versioned CPU models, usage,
etc.

There was a very useful discussion between you and Dan Berrangé on this
list (Message-Id: <20190625050008.12789-5-ehabkost@redhat.com>, the
first version of the thread: "[PATCH 4/6] i386: Infrastructure for
versioned CPU models").  Could potentially incorporate some of that
content.

> ---
>  target/i386/cpu.c | 5 +++++
>  1 file changed, 5 insertions(+)

[...]

-- 
/kashyap

Re: [PATCH for-4.2 2/2] i386: Add -noTSX aliases for hle=off, rtm=off CPU models

[Eduardo Habkost]

On Thu, Nov 21, 2019 at 03:12:45PM +0100, Kashyap Chamarthy wrote:
> On Wed, Nov 20, 2019 at 01:49:12PM -0300, Eduardo Habkost wrote:
> > We have been trying to avoid adding new aliases for CPU model
> > versions, but in the case of changes in defaults introduced by
> > the TAA mitigation patches, the aliases might help avoid user
> > confusion when applying host software updates.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
> 
>     https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg03501.html

Thanks!

> 
>             - - -
> 
> Should we (can do it, if you already don't have a patch WIP for it)
> also update this file to reflect this?
> https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qemu-cpu-models.texi

Yes, we should.  Thanks for the reminder!

> 
> While at it ... I wonder if it's worth making a separte doc
> (versioned-cpu-models.rst) explaining the versioned CPU models, usage,
> etc.
> 
> There was a very useful discussion between you and Dan Berrangé on this
> list (Message-Id: <20190625050008.12789-5-ehabkost@redhat.com>, the
> first version of the thread: "[PATCH 4/6] i386: Infrastructure for
> versioned CPU models").  Could potentially incorporate some of that
> content.
> 

We should, but I don't think I can do it in time for QEMU 4.2.

> > ---
> >  target/i386/cpu.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> [...]
> 
> -- 
> /kashyap

-- 
Eduardo

Re: [PATCH for-4.2 2/2] i386: Add -noTSX aliases for hle=off, rtm=off CPU models

[Kashyap Chamarthy]

On Mon, Nov 25, 2019 at 11:21:10AM -0300, Eduardo Habkost wrote:
> On Thu, Nov 21, 2019 at 03:12:45PM +0100, Kashyap Chamarthy wrote:
> > On Wed, Nov 20, 2019 at 01:49:12PM -0300, Eduardo Habkost wrote:

[...]

> >             - - -
> > 
> > Should we (can do it, if you already don't have a patch WIP for it)
> > also update this file to reflect this?
> > https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qemu-cpu-models.texi
> 
> Yes, we should.  Thanks for the reminder!
> 
> > 
> > While at it ... I wonder if it's worth making a separte doc
> > (versioned-cpu-models.rst) explaining the versioned CPU models, usage,
> > etc.
> > 
> > There was a very useful discussion between you and Dan Berrangé on this
> > list (Message-Id: <20190625050008.12789-5-ehabkost@redhat.com>, the
> > first version of the thread: "[PATCH 4/6] i386: Infrastructure for
> > versioned CPU models").  Could potentially incorporate some of that
> > content.
> > 
> 
> We should, but I don't think I can do it in time for QEMU 4.2.

It's okay; that can wait for post-4.2.

[...]

-- 
/kashyap