* [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests
@ 2020-08-10 22:39 Krish Sadhukhan
2020-08-11 18:37 ` Jim Mattson
0 siblings, 1 reply; 4+ messages in thread
From: Krish Sadhukhan @ 2020-08-10 22:39 UTC (permalink / raw)
To: kvm; +Cc: pbonzini
According to section "Canonicalization and Consistency Checks" in APM vol. 2
the following guest state combinations are illegal:
* EFER.LME and CR0.PG are both set and CR4.PAE is zero.
* EFER.LME and CR0.PG are both non-zero and CR0.PE is zero.
* EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
x86/svm_tests.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 1908c7c..43208fd 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -1962,7 +1962,51 @@ static void test_efer(void)
SVM_TEST_REG_RESERVED_BITS(16, 63, 4, "EFER", vmcb->save.efer,
efer_saved, SVM_EFER_RESERVED_MASK);
+ /*
+ * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
+ */
+ u64 cr0_saved = vmcb->save.cr0;
+ u64 cr0;
+ u64 cr4_saved = vmcb->save.cr4;
+ u64 cr4;
+
+ efer = efer_saved | EFER_LME;
+ vmcb->save.efer = efer;
+ cr0 = cr0_saved | X86_CR0_PG;
+ vmcb->save.cr0 = cr0;
+ cr4 = cr4_saved & ~X86_CR4_PAE;
+ vmcb->save.cr4 = cr4;
+ report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
+ "CR0.PG=1 (%lx) and CR4.PAE=0 (%lx)", efer, cr0, cr4);
+
+ /*
+ * EFER.LME and CR0.PG are both set and CR0.PE is zero.
+ */
+ vmcb->save.cr4 = cr4_saved;
+ cr0 &= ~X86_CR0_PE;
+ vmcb->save.cr0 = cr0;
+ report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
+ "CR0.PG=1 and CR0.PE=0 (%lx)", efer, cr0);
+
+ /*
+ * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero.
+ */
+ u32 cs_attrib_saved = vmcb->save.cs.attrib;
+ u32 cs_attrib;
+
+ cr4 = cr4_saved | X86_CR4_PAE;
+ vmcb->save.cr4 = cr4;
+ cs_attrib = cs_attrib_saved | SVM_SELECTOR_L_MASK |
+ SVM_SELECTOR_DB_MASK;
+ vmcb->save.cs.attrib = cs_attrib;
+ report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
+ "CR0.PG=1 (%lx), CR4.PAE=1 (%lx), CS.L=1 and CS.D=1 (%x)",
+ efer, cr0, cr4, cs_attrib);
+
+ vmcb->save.cr4 = cr4_saved;
+ vmcb->save.cs.attrib = cs_attrib_saved;
vmcb->save.efer = efer_saved;
+ vmcb->save.cr0 = cr0_saved;
}
static void test_cr0(void)
--
2.18.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests
2020-08-10 22:39 [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests Krish Sadhukhan
@ 2020-08-11 18:37 ` Jim Mattson
2020-08-11 19:48 ` Krish Sadhukhan
0 siblings, 1 reply; 4+ messages in thread
From: Jim Mattson @ 2020-08-11 18:37 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm list, Paolo Bonzini
On Mon, Aug 10, 2020 at 3:40 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
> According to section "Canonicalization and Consistency Checks" in APM vol. 2
> the following guest state combinations are illegal:
>
> * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
> * EFER.LME and CR0.PG are both non-zero and CR0.PE is zero.
> * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
> x86/svm_tests.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 1908c7c..43208fd 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -1962,7 +1962,51 @@ static void test_efer(void)
> SVM_TEST_REG_RESERVED_BITS(16, 63, 4, "EFER", vmcb->save.efer,
> efer_saved, SVM_EFER_RESERVED_MASK);
>
> + /*
> + * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
> + */
> + u64 cr0_saved = vmcb->save.cr0;
> + u64 cr0;
> + u64 cr4_saved = vmcb->save.cr4;
> + u64 cr4;
> +
> + efer = efer_saved | EFER_LME;
> + vmcb->save.efer = efer;
> + cr0 = cr0_saved | X86_CR0_PG;
> + vmcb->save.cr0 = cr0;
> + cr4 = cr4_saved & ~X86_CR4_PAE;
> + vmcb->save.cr4 = cr4;
> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
> + "CR0.PG=1 (%lx) and CR4.PAE=0 (%lx)", efer, cr0, cr4);
This seems adequate, but you have to assume that CR0.PE is set, or you
could just be testing the second rule (below).
> + /*
> + * EFER.LME and CR0.PG are both set and CR0.PE is zero.
> + */
> + vmcb->save.cr4 = cr4_saved;
> + cr0 &= ~X86_CR0_PE;
> + vmcb->save.cr0 = cr0;
> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
> + "CR0.PG=1 and CR0.PE=0 (%lx)", efer, cr0);
This too, seems adequate, but you have to assume that CR4.PAE is set,
or you could just be testing the first rule (above).
> + /*
> + * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero.
> + */
> + u32 cs_attrib_saved = vmcb->save.cs.attrib;
> + u32 cs_attrib;
> +
> + cr4 = cr4_saved | X86_CR4_PAE;
Or'ing in X86_CR4_PAE seems superfluous, since you have to assume that
cr4_saved already has the bit set for the above test to be worthwhile.
> + vmcb->save.cr4 = cr4;
> + cs_attrib = cs_attrib_saved | SVM_SELECTOR_L_MASK |
> + SVM_SELECTOR_DB_MASK;
> + vmcb->save.cs.attrib = cs_attrib;
At this point, the second rule still applies (EFER.LME and CR0.PG are
both set and CR0.PE is zero), so this doesn't necessarily verify the
third rule at all.
> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
> + "CR0.PG=1 (%lx), CR4.PAE=1 (%lx), CS.L=1 and CS.D=1 (%x)",
> + efer, cr0, cr4, cs_attrib);
> +
> + vmcb->save.cr4 = cr4_saved;
> + vmcb->save.cs.attrib = cs_attrib_saved;
> vmcb->save.efer = efer_saved;
> + vmcb->save.cr0 = cr0_saved;
> }
>
> static void test_cr0(void)
> --
> 2.18.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests
2020-08-11 18:37 ` Jim Mattson
@ 2020-08-11 19:48 ` Krish Sadhukhan
2020-08-11 20:16 ` Jim Mattson
0 siblings, 1 reply; 4+ messages in thread
From: Krish Sadhukhan @ 2020-08-11 19:48 UTC (permalink / raw)
To: Jim Mattson; +Cc: kvm list, Paolo Bonzini
On 8/11/20 11:37 AM, Jim Mattson wrote:
> On Mon, Aug 10, 2020 at 3:40 PM Krish Sadhukhan
> <krish.sadhukhan@oracle.com> wrote:
>> According to section "Canonicalization and Consistency Checks" in APM vol. 2
>> the following guest state combinations are illegal:
>>
>> * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
>> * EFER.LME and CR0.PG are both non-zero and CR0.PE is zero.
>> * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> ---
>> x86/svm_tests.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
>> index 1908c7c..43208fd 100644
>> --- a/x86/svm_tests.c
>> +++ b/x86/svm_tests.c
>> @@ -1962,7 +1962,51 @@ static void test_efer(void)
>> SVM_TEST_REG_RESERVED_BITS(16, 63, 4, "EFER", vmcb->save.efer,
>> efer_saved, SVM_EFER_RESERVED_MASK);
>>
>> + /*
>> + * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
>> + */
>> + u64 cr0_saved = vmcb->save.cr0;
>> + u64 cr0;
>> + u64 cr4_saved = vmcb->save.cr4;
>> + u64 cr4;
>> +
>> + efer = efer_saved | EFER_LME;
>> + vmcb->save.efer = efer;
>> + cr0 = cr0_saved | X86_CR0_PG;
>> + vmcb->save.cr0 = cr0;
>> + cr4 = cr4_saved & ~X86_CR4_PAE;
>> + vmcb->save.cr4 = cr4;
>> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
>> + "CR0.PG=1 (%lx) and CR4.PAE=0 (%lx)", efer, cr0, cr4);
> This seems adequate, but you have to assume that CR0.PE is set, or you
> could just be testing the second rule (below).
>
>> + /*
>> + * EFER.LME and CR0.PG are both set and CR0.PE is zero.
>> + */
>> + vmcb->save.cr4 = cr4_saved;
>> + cr0 &= ~X86_CR0_PE;
>> + vmcb->save.cr0 = cr0;
>> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
>> + "CR0.PG=1 and CR0.PE=0 (%lx)", efer, cr0);
> This too, seems adequate, but you have to assume that CR4.PAE is set,
> or you could just be testing the first rule (above).
I see what you mean. I am just trying to understand how extensive our
explicit assumptions should be when testing a given APM rule on valid
guest state. For example, should we also explicitly unset CS.L and CS.D
bits (third rule below) ? Or should we also explicitly unset CR3 MBZ
bits because CR3 is relevant when we are setting CR0.PG ?
>
>> + /*
>> + * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero.
>> + */
>> + u32 cs_attrib_saved = vmcb->save.cs.attrib;
>> + u32 cs_attrib;
>> +
>> + cr4 = cr4_saved | X86_CR4_PAE;
> Or'ing in X86_CR4_PAE seems superfluous, since you have to assume that
> cr4_saved already has the bit set for the above test to be worthwhile.
>
>> + vmcb->save.cr4 = cr4;
>> + cs_attrib = cs_attrib_saved | SVM_SELECTOR_L_MASK |
>> + SVM_SELECTOR_DB_MASK;
>> + vmcb->save.cs.attrib = cs_attrib;
> At this point, the second rule still applies (EFER.LME and CR0.PG are
> both set and CR0.PE is zero), so this doesn't necessarily verify the
> third rule at all.
Sorry, I missed it somehow. Will fix it.
>
>> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
>> + "CR0.PG=1 (%lx), CR4.PAE=1 (%lx), CS.L=1 and CS.D=1 (%x)",
>> + efer, cr0, cr4, cs_attrib);
>> +
>> + vmcb->save.cr4 = cr4_saved;
>> + vmcb->save.cs.attrib = cs_attrib_saved;
>> vmcb->save.efer = efer_saved;
>> + vmcb->save.cr0 = cr0_saved;
>> }
>>
>> static void test_cr0(void)
>> --
>> 2.18.4
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests
2020-08-11 19:48 ` Krish Sadhukhan
@ 2020-08-11 20:16 ` Jim Mattson
0 siblings, 0 replies; 4+ messages in thread
From: Jim Mattson @ 2020-08-11 20:16 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm list, Paolo Bonzini
On Tue, Aug 11, 2020 at 12:48 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
>
> On 8/11/20 11:37 AM, Jim Mattson wrote:
> > On Mon, Aug 10, 2020 at 3:40 PM Krish Sadhukhan
> > <krish.sadhukhan@oracle.com> wrote:
> >> According to section "Canonicalization and Consistency Checks" in APM vol. 2
> >> the following guest state combinations are illegal:
> >>
> >> * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
> >> * EFER.LME and CR0.PG are both non-zero and CR0.PE is zero.
> >> * EFER.LME, CR0.PG, CR4.PAE, CS.L, and CS.D are all non-zero
> >>
> >> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> >> ---
> >> x86/svm_tests.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 44 insertions(+)
> >>
> >> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> >> index 1908c7c..43208fd 100644
> >> --- a/x86/svm_tests.c
> >> +++ b/x86/svm_tests.c
> >> @@ -1962,7 +1962,51 @@ static void test_efer(void)
> >> SVM_TEST_REG_RESERVED_BITS(16, 63, 4, "EFER", vmcb->save.efer,
> >> efer_saved, SVM_EFER_RESERVED_MASK);
> >>
> >> + /*
> >> + * EFER.LME and CR0.PG are both set and CR4.PAE is zero.
> >> + */
> >> + u64 cr0_saved = vmcb->save.cr0;
> >> + u64 cr0;
> >> + u64 cr4_saved = vmcb->save.cr4;
> >> + u64 cr4;
> >> +
> >> + efer = efer_saved | EFER_LME;
> >> + vmcb->save.efer = efer;
> >> + cr0 = cr0_saved | X86_CR0_PG;
> >> + vmcb->save.cr0 = cr0;
> >> + cr4 = cr4_saved & ~X86_CR4_PAE;
> >> + vmcb->save.cr4 = cr4;
> >> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
> >> + "CR0.PG=1 (%lx) and CR4.PAE=0 (%lx)", efer, cr0, cr4);
> > This seems adequate, but you have to assume that CR0.PE is set, or you
> > could just be testing the second rule (below).
> >
> >> + /*
> >> + * EFER.LME and CR0.PG are both set and CR0.PE is zero.
> >> + */
> >> + vmcb->save.cr4 = cr4_saved;
> >> + cr0 &= ~X86_CR0_PE;
> >> + vmcb->save.cr0 = cr0;
> >> + report(svm_vmrun() == SVM_EXIT_ERR, "EFER.LME=1 (%lx), "
> >> + "CR0.PG=1 and CR0.PE=0 (%lx)", efer, cr0);
> > This too, seems adequate, but you have to assume that CR4.PAE is set,
> > or you could just be testing the first rule (above).
>
>
> I see what you mean. I am just trying to understand how extensive our
> explicit assumptions should be when testing a given APM rule on valid
> guest state. For example, should we also explicitly unset CS.L and CS.D
> bits (third rule below) ? Or should we also explicitly unset CR3 MBZ
> bits because CR3 is relevant when we are setting CR0.PG ?
I think it's enough to begin with any 'known good' state. However, if
you have <N> illegal guest states with intersecting sets of
specifications, you need to be careful to make sure that each test
case only satisfies the specifications of the one illegal guest state
that the test is attempting to verify. If the test meets the
specifications of multiple illegal guest states, then you can't be
sure which of the matching illegal guest states has triggered the
failed VM-entry.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-11 20:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-10 22:39 [PATCH] KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests Krish Sadhukhan
2020-08-11 18:37 ` Jim Mattson
2020-08-11 19:48 ` Krish Sadhukhan
2020-08-11 20:16 ` Jim Mattson
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.