| Message ID | 20200810223927.82895-1-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nSVM: Test combinations of EFER.LME, CR0.PG, CR4.PAE, CR0.PE and CS register on VMRUN of nested guests | expand |
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 >
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 >>
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.
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)
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(+)