Re: [PATCH 3/3 v4] kvm-unit-tests: nSVM: Test that MBZ bits in CR3 and CR4 are not set on vmrun of nested guests

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/07/20 02:39, Krish Sadhukhan wrote:
> +	SVM_TEST_CR_RESERVED_BITS(0, 2, 1, 3, cr3_saved,
> +	    SVM_CR3_LEGACY_PAE_RESERVED_MASK);
> +
> +	cr4 = cr4_saved & ~X86_CR4_PAE;
> +	vmcb->save.cr4 = cr4;
> +	SVM_TEST_CR_RESERVED_BITS(0, 11, 2, 3, cr3_saved,
> +	    SVM_CR3_LEGACY_RESERVED_MASK);
> +
> +	cr4 |= X86_CR4_PAE;
> +	vmcb->save.cr4 = cr4;
> +	efer |= EFER_LMA;
> +	vmcb->save.efer = efer;
> +	SVM_TEST_CR_RESERVED_BITS(0, 63, 2, 3, cr3_saved,
> +	    SVM_CR3_LONG_RESERVED_MASK);

The test is not covering *non*-reserved bits, so it doesn't catch that 
in both cases KVM is checking against long-mode bits.  Doing this would 
require setting up the VMCB for immediate VMEXIT (for example, injecting 
an event while the IDT limit is zero), so it can be done later.

Instead, you need to set/clear EFER_LME.  Please be more careful to 
check that the test is covering what you expect.

Also, the tests show

PASS: Test CR3 2:0: 641001
PASS: Test CR3 2:0: 2
PASS: Test CR3 2:0: 4
PASS: Test CR3 11:0: 1
PASS: Test CR3 11:0: 4
PASS: Test CR3 11:0: 40
PASS: Test CR3 11:0: 100
PASS: Test CR3 11:0: 400
PASS: Test CR3 63:0: 1
PASS: Test CR3 63:0: 4
PASS: Test CR3 63:0: 40
PASS: Test CR3 63:0: 100
PASS: Test CR3 63:0: 400
PASS: Test CR3 63:0: 10000000000000
PASS: Test CR3 63:0: 40000000000000
PASS: Test CR3 63:0: 100000000000000
PASS: Test CR3 63:0: 400000000000000
PASS: Test CR3 63:0: 1000000000000000
PASS: Test CR3 63:0: 4000000000000000
PASS: Test CR4 31:12: 0
PASS: Test CR4 31:12: 0

and then exits.  There is an issue with compiler optimization for which 
I've sent a patch, but even after fixing it the premature exit is a 
problem: it is caused by a problem in __cr4_reserved_bits and a typo in 
the tests:

diff --git a/x86/svm.h b/x86/svm.h
index f6b9a31..58c9069 100644
--- a/x86/svm.h
+++ b/x86/svm.h
@@ -328,8 +328,8 @@ struct __attribute__ ((__packed__)) vmcb {
 #define	SVM_CR3_LEGACY_RESERVED_MASK		0xfe7U
 #define	SVM_CR3_LEGACY_PAE_RESERVED_MASK	0x7U
 #define	SVM_CR3_LONG_RESERVED_MASK		0xfff0000000000fe7U
-#define	SVM_CR4_LEGACY_RESERVED_MASK		0xffbaf000U
-#define	SVM_CR4_RESERVED_MASK			0xffffffffffbaf000U
+#define	SVM_CR4_LEGACY_RESERVED_MASK		0xffcaf000U
+#define	SVM_CR4_RESERVED_MASK			0xffffffffffcaf000U
 #define	SVM_DR6_RESERVED_MASK			0xffffffffffff1ff0U
 #define	SVM_DR7_RESERVED_MASK			0xffffffff0000cc00U
 #define	SVM_EFER_RESERVED_MASK			0xffffffffffff0200U

(Also, this kind of problem is made harder to notice by only testing
even bits, which may make sense for high order bits, but certainly not
for low-order ones).

All in all, fixing this series has taken me almost 2 hours.  Since I have
done the work I'm queuing but, but I wonder: the compiler optimization
issue could depend on register allocation, but did all of these issues
really happen only on my machine?

Paolo