diff mbox series

[v2,9/9] i386: Add Cascadelake-Server-v2 CPU model

Message ID 20190628002844.24894-10-ehabkost@redhat.com
State New
Headers show
Series x86 CPU model versioning | expand

Commit Message

Eduardo Habkost June 28, 2019, 12:28 a.m. UTC
Add new version of Cascadelake-Server CPU model, setting
stepping=5 and enabling the IA32_ARCH_CAPABILITIES MSR
with some flags.

The new feature will introduce a new host software requirement,
breaking our CPU model runnability promises.  This means we can't
enable the new CPU model version by default in QEMU 4.1, because
management software isn't ready yet to resolve CPU model aliases.
This is why "pc-*-4.1" will keep returning Cascadelake-Server-v1
if "-cpu Cascadelake-Server" is specified.

Includes a test case to ensure the right combinations of
machine-type + CPU model + command-line feature flags will work
as expected.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Enable rdctl-no, ibrs-all, skip-l1dfl-vmentry
  (Suggested by Tao Xu)

Cc: "Hu, Robert" <robert.hu@intel.com>
Cc: Tao Xu <tao3.xu@intel.com>
Cc: jingqi.liu@intel.com,
Cc: "Lai, Paul C" <paul.c.lai@intel.com>
---
 target/i386/cpu.c                          | 14 +++++
 tests/acceptance/x86_cpu_model_versions.py | 73 ++++++++++++++++++++++
 2 files changed, 87 insertions(+)

Comments

Xiaoyao Li July 1, 2019, 7:23 a.m. UTC | #1
On 6/28/2019 8:28 AM, Eduardo Habkost wrote:
> Add new version of Cascadelake-Server CPU model, setting
> stepping=5 and enabling the IA32_ARCH_CAPABILITIES MSR
> with some flags.
> 
> The new feature will introduce a new host software requirement,
> breaking our CPU model runnability promises.  This means we can't
> enable the new CPU model version by default in QEMU 4.1, because
> management software isn't ready yet to resolve CPU model aliases.
> This is why "pc-*-4.1" will keep returning Cascadelake-Server-v1
> if "-cpu Cascadelake-Server" is specified.
> 
> Includes a test case to ensure the right combinations of
> machine-type + CPU model + command-line feature flags will work
> as expected.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Enable rdctl-no, ibrs-all, skip-l1dfl-vmentry
>    (Suggested by Tao Xu)
> 
> Cc: "Hu, Robert" <robert.hu@intel.com>
> Cc: Tao Xu <tao3.xu@intel.com>
> Cc: jingqi.liu@intel.com,
> Cc: "Lai, Paul C" <paul.c.lai@intel.com>
> ---
>   target/i386/cpu.c                          | 14 +++++
>   tests/acceptance/x86_cpu_model_versions.py | 73 ++++++++++++++++++++++
>   2 files changed, 87 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 10a09c971a..570890a7dd 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2343,6 +2343,20 @@ static X86CPUDefinition builtin_x86_defs[] = {
>               CPUID_6_EAX_ARAT,
>           .xlevel = 0x80000008,
>           .model_id = "Intel Xeon Processor (Cascadelake)",
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            { .version = 2,
> +              .props = (PropValue[]) {
> +                  { "stepping", "5" },

Stepping should be 6?

> +                  { "arch-capabilities", "on" },
> +                  { "rdctl-no", "on" },
> +                  { "ibrs-all", "on" },
> +                  { "skip-l1dfl-vmentry", "on" },

rdmsr 0x10AH returns 0x2b from host Cascadelake, we'd better add feature 
"mds-no" too. Otherwise we will need another version for that feature.

> +                  { /* end of list */ }
> +              },
> +            },
> +            { /* end of list */ }
> +        }
>       },
>       {
>           .name = "Icelake-Client",
> diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
> index 165c0c7601..1c9fd6a56e 100644
> --- a/tests/acceptance/x86_cpu_model_versions.py
> +++ b/tests/acceptance/x86_cpu_model_versions.py
> @@ -25,6 +25,10 @@
>   import avocado_qemu
>   import re
>   
> +def get_cpu_prop(vm, prop):
> +    cpu_path = vm.command('query-cpus')[0].get('qom_path')
> +    return vm.command('qom-get', path=cpu_path, property=prop)
> +
>   class X86CPUModelAliases(avocado_qemu.Test):
>       """
>       Validation of PC CPU model versions and CPU model aliases
> @@ -229,3 +233,72 @@ class X86CPUModelAliases(avocado_qemu.Test):
>                            'qemu64-v1 must not be an alias')
>   
>           self.validate_aliases(cpus)
> +
> +    def test_Cascadelake_arch_capabilities_result(self):
> +        # machine-type only:
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.1')
> +        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> +        vm.launch()
> +        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
> +
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.0')
> +        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> +        vm.launch()
> +        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
> +
> +        # command line must override machine-type if CPU model is not versioned:
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.0')
> +        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> +        vm.launch()
> +        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> +                        'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
> +
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.1')
> +        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> +        vm.launch()
> +        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
> +
> +        # versioned CPU model overrides machine-type:
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.0')
> +        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
> +        vm.launch()
> +        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.1 + Cascadelake-Server-v1 should not have arch-capabilities')
> +
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.0')
> +        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
> +        vm.launch()
> +        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.1 + Cascadelake-Server-v1 should have arch-capabilities')
> +
> +        # command line must override machine-type and versioned CPU model:
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.0')
> +        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> +        vm.launch()
> +        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
> +
> +        vm = self.get_vm()
> +        vm.add_args('-S')
> +        vm.set_machine('pc-i440fx-4.1')
> +        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> +        vm.launch()
> +        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +                         'pc-i440fx-4.1 + Cascadelake-Server-v2,-arch-capabilities should not have arch-capabilities')
>
Eduardo Habkost July 1, 2019, 8:38 p.m. UTC | #2
On Mon, Jul 01, 2019 at 03:23:31PM +0800, Xiaoyao Li wrote:
> On 6/28/2019 8:28 AM, Eduardo Habkost wrote:
> > Add new version of Cascadelake-Server CPU model, setting
> > stepping=5 and enabling the IA32_ARCH_CAPABILITIES MSR
> > with some flags.
> > 
> > The new feature will introduce a new host software requirement,
> > breaking our CPU model runnability promises.  This means we can't
> > enable the new CPU model version by default in QEMU 4.1, because
> > management software isn't ready yet to resolve CPU model aliases.
> > This is why "pc-*-4.1" will keep returning Cascadelake-Server-v1
> > if "-cpu Cascadelake-Server" is specified.
> > 
> > Includes a test case to ensure the right combinations of
> > machine-type + CPU model + command-line feature flags will work
> > as expected.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Enable rdctl-no, ibrs-all, skip-l1dfl-vmentry
> >    (Suggested by Tao Xu)
> > 
> > Cc: "Hu, Robert" <robert.hu@intel.com>
> > Cc: Tao Xu <tao3.xu@intel.com>
> > Cc: jingqi.liu@intel.com,
> > Cc: "Lai, Paul C" <paul.c.lai@intel.com>
> > ---
> >   target/i386/cpu.c                          | 14 +++++
> >   tests/acceptance/x86_cpu_model_versions.py | 73 ++++++++++++++++++++++
> >   2 files changed, 87 insertions(+)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 10a09c971a..570890a7dd 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -2343,6 +2343,20 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >               CPUID_6_EAX_ARAT,
> >           .xlevel = 0x80000008,
> >           .model_id = "Intel Xeon Processor (Cascadelake)",
> > +        .versions = (X86CPUVersionDefinition[]) {
> > +            { .version = 1 },
> > +            { .version = 2,
> > +              .props = (PropValue[]) {
> > +                  { "stepping", "5" },
> 
> Stepping should be 6?

Yes, my mistake.

> 
> > +                  { "arch-capabilities", "on" },
> > +                  { "rdctl-no", "on" },
> > +                  { "ibrs-all", "on" },
> > +                  { "skip-l1dfl-vmentry", "on" },
> 
> rdmsr 0x10AH returns 0x2b from host Cascadelake, we'd better add feature
> "mds-no" too. Otherwise we will need another version for that feature.

Thanks for the info, I will update it in the next version.

> [...]
Daniel P. Berrangé July 2, 2019, 9:47 a.m. UTC | #3
On Thu, Jun 27, 2019 at 09:28:44PM -0300, Eduardo Habkost wrote:
> Add new version of Cascadelake-Server CPU model, setting
> stepping=5 and enabling the IA32_ARCH_CAPABILITIES MSR
> with some flags.
> 
> The new feature will introduce a new host software requirement,
> breaking our CPU model runnability promises.  This means we can't
> enable the new CPU model version by default in QEMU 4.1, because
> management software isn't ready yet to resolve CPU model aliases.
> This is why "pc-*-4.1" will keep returning Cascadelake-Server-v1
> if "-cpu Cascadelake-Server" is specified.

Hmm, yes, we don't actually need to answer the question I
raised in the earlier patch for another 2 releases yet,
as we'll be sticking with v1 for all machines for a while.

> 
> Includes a test case to ensure the right combinations of
> machine-type + CPU model + command-line feature flags will work
> as expected.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Enable rdctl-no, ibrs-all, skip-l1dfl-vmentry
>   (Suggested by Tao Xu)
> 
> Cc: "Hu, Robert" <robert.hu@intel.com>
> Cc: Tao Xu <tao3.xu@intel.com>
> Cc: jingqi.liu@intel.com,
> Cc: "Lai, Paul C" <paul.c.lai@intel.com>
> ---
>  target/i386/cpu.c                          | 14 +++++
>  tests/acceptance/x86_cpu_model_versions.py | 73 ++++++++++++++++++++++
>  2 files changed, 87 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
Eduardo Habkost July 2, 2019, 1:55 p.m. UTC | #4
On Tue, Jul 02, 2019 at 10:47:17AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 27, 2019 at 09:28:44PM -0300, Eduardo Habkost wrote:
> > Add new version of Cascadelake-Server CPU model, setting
> > stepping=5 and enabling the IA32_ARCH_CAPABILITIES MSR
> > with some flags.
> > 
> > The new feature will introduce a new host software requirement,
> > breaking our CPU model runnability promises.  This means we can't
> > enable the new CPU model version by default in QEMU 4.1, because
> > management software isn't ready yet to resolve CPU model aliases.
> > This is why "pc-*-4.1" will keep returning Cascadelake-Server-v1
> > if "-cpu Cascadelake-Server" is specified.
> 
> Hmm, yes, we don't actually need to answer the question I
> raised in the earlier patch for another 2 releases yet,
> as we'll be sticking with v1 for all machines for a while.

After these 2 releases have passed, we'll set default version to
CPU_VERSION_LATEST.  I don't see the need to set per-model
defaults.
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 10a09c971a..570890a7dd 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2343,6 +2343,20 @@  static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_6_EAX_ARAT,
         .xlevel = 0x80000008,
         .model_id = "Intel Xeon Processor (Cascadelake)",
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            { .version = 2,
+              .props = (PropValue[]) {
+                  { "stepping", "5" },
+                  { "arch-capabilities", "on" },
+                  { "rdctl-no", "on" },
+                  { "ibrs-all", "on" },
+                  { "skip-l1dfl-vmentry", "on" },
+                  { /* end of list */ }
+              },
+            },
+            { /* end of list */ }
+        }
     },
     {
         .name = "Icelake-Client",
diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
index 165c0c7601..1c9fd6a56e 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -25,6 +25,10 @@ 
 import avocado_qemu
 import re
 
+def get_cpu_prop(vm, prop):
+    cpu_path = vm.command('query-cpus')[0].get('qom_path')
+    return vm.command('qom-get', path=cpu_path, property=prop)
+
 class X86CPUModelAliases(avocado_qemu.Test):
     """
     Validation of PC CPU model versions and CPU model aliases
@@ -229,3 +233,72 @@  class X86CPUModelAliases(avocado_qemu.Test):
                          'qemu64-v1 must not be an alias')
 
         self.validate_aliases(cpus)
+
+    def test_Cascadelake_arch_capabilities_result(self):
+        # machine-type only:
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.1')
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        vm.launch()
+        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
+
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.0')
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        vm.launch()
+        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
+
+        # command line must override machine-type if CPU model is not versioned:
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.0')
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        vm.launch()
+        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+                        'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
+
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.1')
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        vm.launch()
+        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
+
+        # versioned CPU model overrides machine-type:
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.0')
+        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
+        vm.launch()
+        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.1 + Cascadelake-Server-v1 should not have arch-capabilities')
+
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.0')
+        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
+        vm.launch()
+        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.1 + Cascadelake-Server-v1 should have arch-capabilities')
+
+        # command line must override machine-type and versioned CPU model:
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.0')
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        vm.launch()
+        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
+
+        vm = self.get_vm()
+        vm.add_args('-S')
+        vm.set_machine('pc-i440fx-4.1')
+        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        vm.launch()
+        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+                         'pc-i440fx-4.1 + Cascadelake-Server-v2,-arch-capabilities should not have arch-capabilities')