From: Krish Sadhukhan <krish.sadhukhan@oracle.com>
To: Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org
Subject: 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
Date: Wed, 8 Jul 2020 17:01:35 -0700 [thread overview]
Message-ID: <8e065692-e073-1ef6-9c0f-9190eb46d359@oracle.com> (raw)
In-Reply-To: <80ff1de6-f8db-5a09-b67f-ee81937d0dc6@redhat.com>
On 7/8/20 4:07 AM, Paolo Bonzini wrote:
> 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.
Sorry, I wasn't aware that setting/unsetting EFER.LMA wouldn't work !
Another test case that I missed here is testing the reserved bits in
CR3[11:0] in long-mode based on the setting of CR4.PCIDE.
>
> 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
Sorry to hear it caused so many issues ! Thanks for looking into it !
> 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?
Just as a reference, I compiled it using gcc 4.8.5 and ran it on an AMD
EPYC that was running kernel 5.8.0-rc4+. Surprisingly, all tests passed:
PASS: Test CR3 2:0: 641001
PASS: Test CR3 2:0: 641002
PASS: Test CR3 2:0: 641004
PASS: Test CR3 11:0: 641001
PASS: Test CR3 11:0: 641004
PASS: Test CR3 11:0: 641040
PASS: Test CR3 11:0: 641100
PASS: Test CR3 11:0: 641400
PASS: Test CR3 63:0: 641001
PASS: Test CR3 63:0: 641004
PASS: Test CR3 63:0: 641040
PASS: Test CR3 63:0: 641100
PASS: Test CR3 63:0: 641400
PASS: Test CR3 63:0: 10000000641000
PASS: Test CR3 63:0: 40000000641000
PASS: Test CR3 63:0: 100000000641000
PASS: Test CR3 63:0: 400000000641000
PASS: Test CR3 63:0: 1000000000641000
PASS: Test CR3 63:0: 4000000000641000
PASS: Test CR4 31:12: 1000
PASS: Test CR4 31:12: 4000
PASS: Test CR4 31:12: 100000
PASS: Test CR4 31:12: 1000000
PASS: Test CR4 31:12: 4000000
PASS: Test CR4 31:12: 10000000
PASS: Test CR4 31:12: 40000000
PASS: Test CR4 31:12: 1000
PASS: Test CR4 31:12: 4000
PASS: Test CR4 31:12: 100000
PASS: Test CR4 31:12: 1000000
PASS: Test CR4 31:12: 4000000
PASS: Test CR4 31:12: 10000000
PASS: Test CR4 31:12: 40000000
PASS: Test CR4 63:32: 100000000
PASS: Test CR4 63:32: 1000000000
PASS: Test CR4 63:32: 10000000000
PASS: Test CR4 63:32: 100000000000
PASS: Test CR4 63:32: 1000000000000
PASS: Test CR4 63:32: 10000000000000
PASS: Test CR4 63:32: 100000000000000
PASS: Test CR4 63:32: 1000000000000000
>
> Paolo
>
next prev parent reply other threads:[~2020-07-09 0:01 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-08 0:39 [PATCH 0/3 v4] KVM: nSVM: Check MBZ bits in CR3 and CR4 on vmrun of nested guests Krish Sadhukhan
2020-07-08 0:39 ` [PATCH 1/3 v4] KVM: x86: Create mask for guest CR4 reserved bits in kvm_update_cpuid() Krish Sadhukhan
2020-07-08 9:48 ` Paolo Bonzini
2020-07-08 0:39 ` [PATCH 2/3 v4] KVM: nSVM: Check that MBZ bits in CR3 and CR4 are not set on vmrun of nested guests Krish Sadhukhan
2020-07-08 10:03 ` Paolo Bonzini
2020-07-08 21:36 ` Krish Sadhukhan
2020-07-08 22:51 ` Krish Sadhukhan
2020-07-08 23:07 ` Jim Mattson
2020-07-09 9:36 ` Paolo Bonzini
2020-07-08 0:39 ` [PATCH 3/3 v4] kvm-unit-tests: nSVM: Test " Krish Sadhukhan
2020-07-08 11:07 ` Paolo Bonzini
2020-07-09 0:01 ` Krish Sadhukhan [this message]
2020-07-11 16:12 ` Nadav Amit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8e065692-e073-1ef6-9c0f-9190eb46d359@oracle.com \
--to=krish.sadhukhan@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.