On 8/11/20 11:37 AM, Jim Mattson wrote:
On Mon, Aug 10, 2020 at 3:40 PM Krish Sadhukhan
<krish.sadhukhan@xxxxxxxxxx> 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@xxxxxxxxxx>
---
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
