Re: [PATCH] i386: Mask SVM features if nested SVM is disabled

2020-06-24 Thread Paolo Bonzini
On 24/06/20 01:01, Eduardo Habkost wrote:
> QEMU incorrectly validates FEAT_SVM feature flags against
> GET_SUPPORTED_CPUID even if SVM features are being masked out by
> cpu_x86_cpuid().  This can make QEMU print warnings on most AMD
> CPU models, even when SVM nesting is disabled (which is the
> default).
> 
> This bug was never detected before because of a Linux KVM bug:
> until Linux v5.6, KVM was not filtering out SVM features in
> GET_SUPPORTED_CPUID when nested was disabled.  This KVM bug was
> fixed in Linux v5.7-rc1, on Linux commit a50718cc3f43 ("KVM:
> nSVM: Expose SVM features to L1 iff nested is enabled").
> 
> Fix the problem by adding a CPUID_EXT3_SVM dependency to all
> FEAT_SVM feature flags in the feature_dependencies table.
> 
> Reported-by: Yanan Fu 
> Signed-off-by: Eduardo Habkost 
> ---
>  target/i386/cpu.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b1b311baa2..a9edcaf531 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1404,6 +1404,10 @@ static FeatureDep feature_dependencies[] = {
>  .from = { FEAT_VMX_SECONDARY_CTLS,  VMX_SECONDARY_EXEC_ENABLE_VMFUNC 
> },
>  .to = { FEAT_VMX_VMFUNC,~0ull },
>  },
> +{
> +.from = { FEAT_8000_0001_ECX,   CPUID_EXT3_SVM },
> +.to = { FEAT_SVM,   ~0ull },
> +},
>  };
>  
>  typedef struct X86RegisterInfo32 {
> 

Queued with this fixup:

diff --git a/tests/qtest/test-x86-cpuid-compat.c 
b/tests/qtest/test-x86-cpuid-compat.c
index 772287bdb4..8709e7d9ce 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -256,7 +256,7 @@ int main(int argc, char **argv)
"-cpu 486,+invtsc", "xlevel", 0x8007);
 /* CPUID[8000_000A].EDX: */
 add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
-   "-cpu 486,+npt", "xlevel", 0x800A);
+   "-cpu 486,+svm,+npt", "xlevel", 0x800A);
 /* CPUID[C000_0001].EDX: */
 add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
"-cpu phenom,+xstore", "xlevel2", 0xC001);
@@ -348,7 +348,7 @@ int main(int argc, char **argv)
"-machine pc-i440fx-2.4 -cpu SandyBridge,",
"xlevel", 0x8008);
 add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
-   "-machine pc-i440fx-2.4 -cpu SandyBridge,+npt",
+   "-machine pc-i440fx-2.4 -cpu SandyBridge,+svm,+npt",
"xlevel", 0x8008);
 
 /* Test feature parsing */

Paolo




Re: [PATCH] i386: Mask SVM features if nested SVM is disabled

2020-06-23 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200623230116.277409-1-ehabk...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTiotest-qcow2: 184
  TESTcheck-qtest-x86_64: tests/qtest/test-x86-cpuid-compat
  TESTiotest-qcow2: 186
qemu-system-x86_64: warning: This feature depends on other features that were 
not requested: CPUID.800AH:EDX.npt [bit 0]
**
ERROR:/tmp/qemu-test/src/tests/qtest/test-x86-cpuid-compat.c:64:test_cpuid_prop:
 assertion failed (val == args->expected_value): (0 == 2147483658)
ERROR - Bail out! 
ERROR:/tmp/qemu-test/src/tests/qtest/test-x86-cpuid-compat.c:64:test_cpuid_prop:
 assertion failed (val == args->expected_value): (0 == 2147483658)
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 187
  TESTiotest-qcow2: 190
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=2a55c44dc54d40a4938f8487303f6dba', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-naaagll6/src/docker-src.2020-06-23-19.07.43.6102:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=2a55c44dc54d40a4938f8487303f6dba
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-naaagll6/src'
make: *** [docker-run-test-quick@centos7] Error 2

real15m25.121s
user0m8.515s


The full log is available at
http://patchew.org/logs/20200623230116.277409-1-ehabk...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH] i386: Mask SVM features if nested SVM is disabled

2020-06-23 Thread Eduardo Habkost
QEMU incorrectly validates FEAT_SVM feature flags against
GET_SUPPORTED_CPUID even if SVM features are being masked out by
cpu_x86_cpuid().  This can make QEMU print warnings on most AMD
CPU models, even when SVM nesting is disabled (which is the
default).

This bug was never detected before because of a Linux KVM bug:
until Linux v5.6, KVM was not filtering out SVM features in
GET_SUPPORTED_CPUID when nested was disabled.  This KVM bug was
fixed in Linux v5.7-rc1, on Linux commit a50718cc3f43 ("KVM:
nSVM: Expose SVM features to L1 iff nested is enabled").

Fix the problem by adding a CPUID_EXT3_SVM dependency to all
FEAT_SVM feature flags in the feature_dependencies table.

Reported-by: Yanan Fu 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b1b311baa2..a9edcaf531 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1404,6 +1404,10 @@ static FeatureDep feature_dependencies[] = {
 .from = { FEAT_VMX_SECONDARY_CTLS,  VMX_SECONDARY_EXEC_ENABLE_VMFUNC },
 .to = { FEAT_VMX_VMFUNC,~0ull },
 },
+{
+.from = { FEAT_8000_0001_ECX,   CPUID_EXT3_SVM },
+.to = { FEAT_SVM,   ~0ull },
+},
 };
 
 typedef struct X86RegisterInfo32 {
-- 
2.26.2