1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] i386: Disable OSPKE on CPU model definitions

Eduardo Habkost posted 1 patch 5 years, 1 month ago
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190319200515.14999-1-ehabkost@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.c               |  6 +++---
tests/acceptance/cpu_queries.py | 33 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 tests/acceptance/cpu_queries.py
[Qemu-devel] [PATCH] i386: Disable OSPKE on CPU model definitions
Posted by Eduardo Habkost 5 years, 1 month ago 
Currently, the Cascadelake-Server, Icelake-Client, and
Icelake-Server are always generating the following warning:

  qemu-system-x86_64: warning: \
    host doesn't support requested feature: CPUID.07H:ECX [bit 4]

This happens because OSPKE was never returned by
GET_SUPPORTED_CPUID or x86_cpu_get_supported_feature_word().
OSPKE is a runtime flag automatically set by the KVM module or by
TCG code, was always cleared by x86_cpu_filter_features(), and
was not supposed to appear on the CPU model table.

Remove the OSPKE flag from the CPU model table entries, to avoid
the bogus warning and avoid returning invalid feature data on
query-cpu-* QMP commands.  As OSPKE was always cleared by
x86_cpu_filter_features(), this won't have any guest-visible
impact.

Include a test case that should detect the problem if we introduce
a similar bug again.

Fixes: c7a88b52f62b ("i386: Add new model of Cascadelake-Server")
Fixes: 8a11c62da914 ("i386: Add new CPU model Icelake-{Server,Client}")
Cc: Tao Xu <tao3.xu@intel.com>
Cc: Robert Hoo <robert.hu@linux.intel.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c               |  6 +++---
 tests/acceptance/cpu_queries.py | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 tests/acceptance/cpu_queries.py

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d90c01a059..38cfdec33b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2533,7 +2533,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
             CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
         .features[FEAT_7_0_ECX] =
-            CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE |
+            CPUID_7_0_ECX_PKU |
             CPUID_7_0_ECX_AVX512VNNI,
         .features[FEAT_7_0_EDX] =
             CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
@@ -2586,7 +2586,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_SMAP,
         .features[FEAT_7_0_ECX] =
             CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU |
-            CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
+            CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
             CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ |
             CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
             CPUID_7_0_ECX_AVX512_VPOPCNTDQ,
@@ -2644,7 +2644,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
         .features[FEAT_7_0_ECX] =
             CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU |
-            CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
+            CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
             CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ |
             CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
             CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57,
diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
new file mode 100644
index 0000000000..e71edec39f
--- /dev/null
+++ b/tests/acceptance/cpu_queries.py
@@ -0,0 +1,33 @@
+# Sanity check of query-cpu-* results
+#
+# Copyright (c) 2019 Red Hat, Inc.
+#
+# Author:
+#  Eduardo Habkost <ehabkost@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import logging
+
+from avocado_qemu import Test
+
+class QueryCPUModelExpansion(Test):
+    """
+    Run query-cpu-model-expansion for each CPU model, and validate results
+    """
+
+    def test(self):
+        self.vm.set_machine('none')
+        self.vm.add_args('-S')
+        self.vm.launch()
+
+        cpus = self.vm.command('query-cpu-definitions')
+        for c in cpus:
+            print(repr(c))
+            self.assertNotIn('', c['unavailable-features'], c['name'])
+
+        for c in cpus:
+            model = {'name': c['name']}
+            e = self.vm.command('query-cpu-model-expansion', model=model, type='full')
+            self.assertEquals(e['model']['name'], c['name'])
-- 
2.18.0.rc1.1.g3f1ff2140
Re: [Qemu-devel] [PATCH] i386: Disable OSPKE on CPU model definitions
Posted by Richard Henderson 5 years, 1 month ago 
On 3/19/19 1:05 PM, Eduardo Habkost wrote:
> Currently, the Cascadelake-Server, Icelake-Client, and
> Icelake-Server are always generating the following warning:
> 
>   qemu-system-x86_64: warning: \
>     host doesn't support requested feature: CPUID.07H:ECX [bit 4]
> 
> This happens because OSPKE was never returned by
> GET_SUPPORTED_CPUID or x86_cpu_get_supported_feature_word().
> OSPKE is a runtime flag automatically set by the KVM module or by
> TCG code, was always cleared by x86_cpu_filter_features(), and
> was not supposed to appear on the CPU model table.
> 
> Remove the OSPKE flag from the CPU model table entries, to avoid
> the bogus warning and avoid returning invalid feature data on
> query-cpu-* QMP commands.  As OSPKE was always cleared by
> x86_cpu_filter_features(), this won't have any guest-visible
> impact.
> 
> Include a test case that should detect the problem if we introduce
> a similar bug again.
> 
> Fixes: c7a88b52f62b ("i386: Add new model of Cascadelake-Server")
> Fixes: 8a11c62da914 ("i386: Add new CPU model Icelake-{Server,Client}")
> Cc: Tao Xu <tao3.xu@intel.com>
> Cc: Robert Hoo <robert.hu@linux.intel.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c               |  6 +++---
>  tests/acceptance/cpu_queries.py | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 tests/acceptance/cpu_queries.py

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Re: [Qemu-devel] [PATCH] i386: Disable OSPKE on CPU model definitions
Posted by Robert Hoo 5 years, 1 month ago 
On Tue, 2019-03-19 at 17:05 -0300, Eduardo Habkost wrote:
> Currently, the Cascadelake-Server, Icelake-Client, and
> Icelake-Server are always generating the following warning:
> 
>   qemu-system-x86_64: warning: \
>     host doesn't support requested feature: CPUID.07H:ECX [bit 4]
> 
> This happens because OSPKE was never returned by
> GET_SUPPORTED_CPUID or x86_cpu_get_supported_feature_word().
> OSPKE is a runtime flag automatically set by the KVM module or by
> TCG code, was always cleared by x86_cpu_filter_features(), and
> was not supposed to appear on the CPU model table.
> 
> Remove the OSPKE flag from the CPU model table entries, to avoid
> the bogus warning and avoid returning invalid feature data on
> query-cpu-* QMP commands.  As OSPKE was always cleared by
> x86_cpu_filter_features(), this won't have any guest-visible
> impact.
> 
> Include a test case that should detect the problem if we introduce
> a similar bug again.
> 
> Fixes: c7a88b52f62b ("i386: Add new model of Cascadelake-Server")
> Fixes: 8a11c62da914 ("i386: Add new CPU model Icelake-
> {Server,Client}")
> Cc: Tao Xu <tao3.xu@intel.com>
> Cc: Robert Hoo <robert.hu@linux.intel.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c               |  6 +++---
>  tests/acceptance/cpu_queries.py | 33
> +++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 tests/acceptance/cpu_queries.py
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d90c01a059..38cfdec33b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2533,7 +2533,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
>              CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
>          .features[FEAT_7_0_ECX] =
> -            CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE |
> +            CPUID_7_0_ECX_PKU |
>              CPUID_7_0_ECX_AVX512VNNI,
>          .features[FEAT_7_0_EDX] =
>              CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
> @@ -2586,7 +2586,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_SMAP,
>          .features[FEAT_7_0_ECX] =
>              CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP |
> CPUID_7_0_ECX_PKU |
> -            CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 |
> CPUID_7_0_ECX_GFNI |
> +            CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
>              CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ |
>              CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
>              CPUID_7_0_ECX_AVX512_VPOPCNTDQ,
> @@ -2644,7 +2644,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
>          .features[FEAT_7_0_ECX] =
>              CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP |
> CPUID_7_0_ECX_PKU |
> -            CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 |
> CPUID_7_0_ECX_GFNI |
> +            CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
>              CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ |
>              CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
>              CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57,
> diff --git a/tests/acceptance/cpu_queries.py
> b/tests/acceptance/cpu_queries.py
> new file mode 100644
> index 0000000000..e71edec39f
> --- /dev/null
> +++ b/tests/acceptance/cpu_queries.py
> @@ -0,0 +1,33 @@
> +# Sanity check of query-cpu-* results
> +#
> +# Copyright (c) 2019 Red Hat, Inc.
> +#
> +# Author:
> +#  Eduardo Habkost <ehabkost@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import logging
> +
> +from avocado_qemu import Test
> +
> +class QueryCPUModelExpansion(Test):
> +    """
> +    Run query-cpu-model-expansion for each CPU model, and validate
> results
> +    """
> +
> +    def test(self):
> +        self.vm.set_machine('none')
> +        self.vm.add_args('-S')
> +        self.vm.launch()
> +
> +        cpus = self.vm.command('query-cpu-definitions')
> +        for c in cpus:
> +            print(repr(c))
> +            self.assertNotIn('', c['unavailable-features'],
> c['name'])
> +
> +        for c in cpus:
> +            model = {'name': c['name']}
> +            e = self.vm.command('query-cpu-model-expansion',
> model=model, type='full')
> +            self.assertEquals(e['model']['name'], c['name'])

Thanks for fix.

Reviewed-by: Robert Hoo <robert.hu@linux.intel.com>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.