* [PATCH kvm-unit-tests 0/2] vmx: fixes to v2 tests @ 2017-05-11 11:23 Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 1/2] vmx: always perform access with modified page table for PADDR tests Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests Paolo Bonzini 0 siblings, 2 replies; 9+ messages in thread From: Paolo Bonzini @ 2017-05-11 11:23 UTC (permalink / raw) To: kvm; +Cc: Peter Feiner, David Matlack These patches fix the new EPT access tests by Peter Feiner and David Matlack, so that (with a couple KVM bugs fixed) everything passes with both eptad=0 and eptad=1. I've pushed these fixes into the "next" branch to kvm-unit-tests.git; I've also squashed these into the original patches and pushed the result to branch "next-fixed", which I intend to merge into master. Reviews of course are always welcome! Paolo Paolo Bonzini (2): vmx: always perform access with modified page table for PADDR tests vmx: fix expected results of new EPT tests x86/vmx_tests.c | 71 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 33 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH kvm-unit-tests 1/2] vmx: always perform access with modified page table for PADDR tests 2017-05-11 11:23 [PATCH kvm-unit-tests 0/2] vmx: fixes to v2 tests Paolo Bonzini @ 2017-05-11 11:23 ` Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests Paolo Bonzini 1 sibling, 0 replies; 9+ messages in thread From: Paolo Bonzini @ 2017-05-11 11:23 UTC (permalink / raw) To: kvm; +Cc: Peter Feiner, David Matlack Even if we do not expect a violation, we should try and perform the access while the EPT page tables have been modified. Without this change, ept_access_paddr is basically a no-op with expect_violation equal to false. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- x86/vmx_tests.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index 734d785..e1f92d4 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -2340,12 +2340,15 @@ static void ept_access_paddr(unsigned long ept_access, unsigned long pte_ad, orig_epte = ept_twiddle(gpa, /*mkhuge=*/0, /*level=*/1, /*clear=*/EPT_PRESENT, /*set=*/ept_access); - if (expect_violation) + if (expect_violation) { do_ept_violation(/*leaf=*/true, op, expected_qual | EPT_VLT_LADDR_VLD, gpa); - - ept_untwiddle(gpa, /*level=*/1, orig_epte); - do_ept_access_op(op); + ept_untwiddle(gpa, /*level=*/1, orig_epte); + do_ept_access_op(op); + } else { + do_ept_access_op(op); + ept_untwiddle(gpa, /*level=*/1, orig_epte); + } TEST_ASSERT(*ptep & PT_ACCESSED_MASK); if ((pte_ad & PT_DIRTY_MASK) || op == OP_WRITE) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-11 11:23 [PATCH kvm-unit-tests 0/2] vmx: fixes to v2 tests Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 1/2] vmx: always perform access with modified page table for PADDR tests Paolo Bonzini @ 2017-05-11 11:23 ` Paolo Bonzini 2017-05-11 15:58 ` Peter Feiner 1 sibling, 1 reply; 9+ messages in thread From: Paolo Bonzini @ 2017-05-11 11:23 UTC (permalink / raw) To: kvm; +Cc: Peter Feiner, David Matlack Remove RD/EX exchange hack which we can fix in KVM; mark page table accesses as read/write when EPT A/D is enabled, and expect them to be handled as read/write even with disabled EPT A/D bits (even though the exit qualification says otherwise). With these changes, and the corresponding KVM patch I'm going to send out, all v2 tests pass with both eptad=0 and eptad=1. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- x86/vmx_tests.c | 60 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index e1f92d4..03e4ad4 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -2206,12 +2206,6 @@ static void do_ept_violation(bool leaf, enum ept_access_op op, qual = vmcs_read(EXI_QUALIFICATION); - /* Hack now so important tests can pass. */ - if (!leaf && (expected_qual & EPT_VLT_PERM_RD) - && !(expected_qual & EPT_VLT_PERM_EX)) - expected_qual = (expected_qual & ~EPT_VLT_PERM_RD) | - EPT_VLT_PERM_EX; - diagnose_ept_violation_qual(expected_qual, qual); TEST_EXPECT_EQ(expected_qual, qual); @@ -2777,33 +2771,38 @@ static void ept_access_test_paddr_not_present_ad_disabled(void) static void ept_access_test_paddr_not_present_ad_enabled(void) { + u64 qual = EPT_VLT_RD | EPT_VLT_WR; + ept_access_test_setup(); ept_enable_ad_bits_or_skip_test(); - ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, EPT_VLT_WR); - ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_WR); - ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_WR); + ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, qual); + ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, qual); + ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, qual); } static void ept_access_test_paddr_read_only_ad_disabled(void) { - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD; + /* + * When EPT AD bits are disabled, all accesses to guest paging + * structures are reported as reads as far as EPT translation + * is concerned, but any write of A/D bits still fails (with an + * EPT violation and exit qualification 010'001'001). + */ + u64 qual = EPT_VLT_RD | EPT_VLT_PERM_RD; ept_access_test_setup(); ept_disable_ad_bits(); - /* Can't update A bit, so all accesses fail. */ ept_access_violation_paddr(EPT_RA, 0, OP_READ, qual); ept_access_violation_paddr(EPT_RA, 0, OP_WRITE, qual); ept_access_violation_paddr(EPT_RA, 0, OP_EXEC, qual); - /* AD bits disabled, so only writes try to update the D bit. */ - ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ); + ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ, qual); ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_WRITE, qual); - ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC); - /* Both A and D already set, so read-only is OK. */ - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_READ); - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_WRITE); - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_EXEC); + ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC, qual); + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_READ, qual); + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_WRITE, qual); + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_EXEC, qual); } static void ept_access_test_paddr_read_only_ad_enabled(void) @@ -2813,7 +2812,7 @@ static void ept_access_test_paddr_read_only_ad_enabled(void) * structures are considered writes as far as EPT translation * is concerned. */ - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD; + u64 qual = EPT_VLT_WR | EPT_VLT_RD | EPT_VLT_PERM_RD; ept_access_test_setup(); ept_enable_ad_bits_or_skip_test(); @@ -2849,23 +2848,26 @@ static void ept_access_test_paddr_read_write_execute(void) static void ept_access_test_paddr_read_execute_ad_disabled(void) { - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; + /* + * When EPT AD bits are disabled, all accesses to guest paging + * structures are reported as reads as far as EPT translation + * is concerned, but any write of A/D bits still fails (with an + * EPT violation and exit qualification 010'101'001). + */ + u64 qual = EPT_VLT_RD | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; ept_access_test_setup(); ept_disable_ad_bits(); - /* Can't update A bit, so all accesses fail. */ ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_READ, qual); ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_WRITE, qual); ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_EXEC, qual); - /* AD bits disabled, so only writes try to update the D bit. */ - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ); + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ, qual); ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_WRITE, qual); - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC); - /* Both A and D already set, so read-only is OK. */ - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ); - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE); - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC); + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC, qual); + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ, qual); + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE, qual); + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC, qual); } static void ept_access_test_paddr_read_execute_ad_enabled(void) @@ -2875,7 +2877,7 @@ static void ept_access_test_paddr_read_execute_ad_enabled(void) * structures are considered writes as far as EPT translation * is concerned. */ - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; + u64 qual = EPT_VLT_WR | EPT_VLT_RD | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; ept_access_test_setup(); ept_enable_ad_bits_or_skip_test(); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-11 11:23 ` [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests Paolo Bonzini @ 2017-05-11 15:58 ` Peter Feiner 2017-05-11 17:30 ` Paolo Bonzini 0 siblings, 1 reply; 9+ messages in thread From: Peter Feiner @ 2017-05-11 15:58 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm, David Matlack On Thu, May 11, 2017 at 4:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Remove RD/EX exchange hack which we can fix in KVM; mark page table > accesses as read/write when EPT A/D is enabled, and expect them to > be handled as read/write even with disabled EPT A/D bits (even though > the exit qualification says otherwise). I assume this is a stopgap change. I mean, you're asserting for the wrong behavior just so the tests pass. Correct? To fix this properly, you've got to disable EPT A/D in VMCS02 when it's disabled in VMCS12. We've been running with such a patch for the last year but haven't sent it upstream yet :-( (Shame on us for upstreaming the test without the fix!) > > With these changes, and the corresponding KVM patch I'm going to send > out, all v2 tests pass with both eptad=0 and eptad=1. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > x86/vmx_tests.c | 60 +++++++++++++++++++++++++++++---------------------------- > 1 file changed, 31 insertions(+), 29 deletions(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index e1f92d4..03e4ad4 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -2206,12 +2206,6 @@ static void do_ept_violation(bool leaf, enum ept_access_op op, > > qual = vmcs_read(EXI_QUALIFICATION); > > - /* Hack now so important tests can pass. */ > - if (!leaf && (expected_qual & EPT_VLT_PERM_RD) > - && !(expected_qual & EPT_VLT_PERM_EX)) > - expected_qual = (expected_qual & ~EPT_VLT_PERM_RD) | > - EPT_VLT_PERM_EX; > - > diagnose_ept_violation_qual(expected_qual, qual); > TEST_EXPECT_EQ(expected_qual, qual); > > @@ -2777,33 +2771,38 @@ static void ept_access_test_paddr_not_present_ad_disabled(void) > > static void ept_access_test_paddr_not_present_ad_enabled(void) > { > + u64 qual = EPT_VLT_RD | EPT_VLT_WR; > + > ept_access_test_setup(); > ept_enable_ad_bits_or_skip_test(); > > - ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, EPT_VLT_WR); > - ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_WR); > - ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_WR); > + ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, qual); > + ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, qual); > + ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, qual); > } > > static void ept_access_test_paddr_read_only_ad_disabled(void) > { > - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD; > + /* > + * When EPT AD bits are disabled, all accesses to guest paging > + * structures are reported as reads as far as EPT translation > + * is concerned, but any write of A/D bits still fails (with an > + * EPT violation and exit qualification 010'001'001). > + */ > + u64 qual = EPT_VLT_RD | EPT_VLT_PERM_RD; > > ept_access_test_setup(); > ept_disable_ad_bits(); > > - /* Can't update A bit, so all accesses fail. */ > ept_access_violation_paddr(EPT_RA, 0, OP_READ, qual); > ept_access_violation_paddr(EPT_RA, 0, OP_WRITE, qual); > ept_access_violation_paddr(EPT_RA, 0, OP_EXEC, qual); > - /* AD bits disabled, so only writes try to update the D bit. */ > - ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ); > + ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ, qual); > ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_WRITE, qual); > - ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC); > - /* Both A and D already set, so read-only is OK. */ > - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_READ); > - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_WRITE); > - ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_EXEC); > + ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC, qual); > + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_READ, qual); > + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_WRITE, qual); > + ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_EXEC, qual); > } > > static void ept_access_test_paddr_read_only_ad_enabled(void) > @@ -2813,7 +2812,7 @@ static void ept_access_test_paddr_read_only_ad_enabled(void) > * structures are considered writes as far as EPT translation > * is concerned. > */ > - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD; > + u64 qual = EPT_VLT_WR | EPT_VLT_RD | EPT_VLT_PERM_RD; > > ept_access_test_setup(); > ept_enable_ad_bits_or_skip_test(); > @@ -2849,23 +2848,26 @@ static void ept_access_test_paddr_read_write_execute(void) > > static void ept_access_test_paddr_read_execute_ad_disabled(void) > { > - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; > + /* > + * When EPT AD bits are disabled, all accesses to guest paging > + * structures are reported as reads as far as EPT translation > + * is concerned, but any write of A/D bits still fails (with an > + * EPT violation and exit qualification 010'101'001). > + */ > + u64 qual = EPT_VLT_RD | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; > > ept_access_test_setup(); > ept_disable_ad_bits(); > > - /* Can't update A bit, so all accesses fail. */ > ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_READ, qual); > ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_WRITE, qual); > ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_EXEC, qual); > - /* AD bits disabled, so only writes try to update the D bit. */ > - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ); > + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ, qual); > ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_WRITE, qual); > - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC); > - /* Both A and D already set, so read-only is OK. */ > - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ); > - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE); > - ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC); > + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC, qual); > + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ, qual); > + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE, qual); > + ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC, qual); > } > > static void ept_access_test_paddr_read_execute_ad_enabled(void) > @@ -2875,7 +2877,7 @@ static void ept_access_test_paddr_read_execute_ad_enabled(void) > * structures are considered writes as far as EPT translation > * is concerned. > */ > - u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; > + u64 qual = EPT_VLT_WR | EPT_VLT_RD | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX; > > ept_access_test_setup(); > ept_enable_ad_bits_or_skip_test(); > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-11 15:58 ` Peter Feiner @ 2017-05-11 17:30 ` Paolo Bonzini 2017-05-11 19:51 ` David Matlack 0 siblings, 1 reply; 9+ messages in thread From: Paolo Bonzini @ 2017-05-11 17:30 UTC (permalink / raw) To: Peter Feiner; +Cc: kvm, David Matlack ----- Original Message ----- > From: "Peter Feiner" <pfeiner@google.com> > To: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: kvm@vger.kernel.org, "David Matlack" <dmatlack@google.com> > Sent: Thursday, May 11, 2017 5:58:49 PM > Subject: Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests > > On Thu, May 11, 2017 at 4:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Remove RD/EX exchange hack which we can fix in KVM; mark page table > > accesses as read/write when EPT A/D is enabled, and expect them to > > be handled as read/write even with disabled EPT A/D bits (even though > > the exit qualification says otherwise). > > I assume this is a stopgap change. I mean, you're asserting for the > wrong behavior just so the tests pass. Correct? No, I've tried the tests on upstream Linux with eptad=0 (so that EPT A/D is not used by KVM on the host) and they also hang with an infinite stream of EPT violations. See the KVM patch I sent which also explains the hang in the comments ("[PATCH 2/2] KVM: nVMX: fix nEPT handling of guest page table accesses"). So it seems to me that this is the expected behavior of the processor even when A/D bits are disabled. I haven't tested on a processor with EPT but without A/D bits though. Paolo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-11 17:30 ` Paolo Bonzini @ 2017-05-11 19:51 ` David Matlack 2017-05-12 8:45 ` Paolo Bonzini 0 siblings, 1 reply; 9+ messages in thread From: David Matlack @ 2017-05-11 19:51 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Feiner, kvm list On Thu, May 11, 2017 at 10:30 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > ----- Original Message ----- >> From: "Peter Feiner" <pfeiner@google.com> >> To: "Paolo Bonzini" <pbonzini@redhat.com> >> Cc: kvm@vger.kernel.org, "David Matlack" <dmatlack@google.com> >> Sent: Thursday, May 11, 2017 5:58:49 PM >> Subject: Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests >> >> On Thu, May 11, 2017 at 4:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> > Remove RD/EX exchange hack which we can fix in KVM; mark page table >> > accesses as read/write when EPT A/D is enabled, and expect them to >> > be handled as read/write even with disabled EPT A/D bits (even though >> > the exit qualification says otherwise). >> >> I assume this is a stopgap change. I mean, you're asserting for the >> wrong behavior just so the tests pass. Correct? > > No, I've tried the tests on upstream Linux with eptad=0 (so that EPT A/D > is not used by KVM on the host) and they also hang with an infinite stream > of EPT violations. I think the failures are caused by this code in handle_ept_violation, which clears the ACC_WRITE bit of the exit qualification before handling the fault, when EPT A/D is disabled: if (is_guest_mode(vcpu) && !(exit_qualification & EPT_VIOLATION_GVA_TRANSLATED)) { /* * Fix up exit_qualification according to whether guest * page table accesses are reads or writes. */ u64 eptp = nested_ept_get_cr3(vcpu); if (!(eptp & VMX_EPT_AD_ENABLE_BIT)) exit_qualification &= ~EPT_VIOLATION_ACC_WRITE; } Per 28.2.3.2 EPT Violations: "Writes by the logical processor to guest paging structures to update accessed and dirty flags are considered to be data writes." In other words, it's valid for EPT_VIOLATION_GVA_TRANSLATED and EPT_VIOLATION_ACC_WRITE to both be set in the exit qual when EPT A/D is disabled. > See the KVM patch I sent which also explains the hang > in the comments ("[PATCH 2/2] KVM: nVMX: fix nEPT handling of guest page > table accesses"). > > So it seems to me that this is the expected behavior of the processor > even when A/D bits are disabled. I haven't tested on a processor with > EPT but without A/D bits though. > > Paolo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-11 19:51 ` David Matlack @ 2017-05-12 8:45 ` Paolo Bonzini 2017-05-12 23:45 ` David Matlack 0 siblings, 1 reply; 9+ messages in thread From: Paolo Bonzini @ 2017-05-12 8:45 UTC (permalink / raw) To: David Matlack Cc: Peter Feiner, kvm list, Xiao Guangrong, Radim Krčmář On 11/05/2017 21:51, David Matlack wrote: >> No, I've tried the tests on upstream Linux with eptad=0 (so that EPT A/D >> is not used by KVM on the host) and they also hang with an infinite stream >> of EPT violations. > > I think the failures are caused by this code in handle_ept_violation, > which clears the ACC_WRITE bit of the exit qualification before > handling the fault, when EPT A/D is disabled: > > if (is_guest_mode(vcpu) > && !(exit_qualification & EPT_VIOLATION_GVA_TRANSLATED)) { > /* > * Fix up exit_qualification according to whether guest > * page table accesses are reads or writes. > */ > u64 eptp = nested_ept_get_cr3(vcpu); > if (!(eptp & VMX_EPT_AD_ENABLE_BIT)) > exit_qualification &= ~EPT_VIOLATION_ACC_WRITE; > } I thought so as well; that's why my hypervisor patch moved trace_kvm_page_fault before munging exit_qualification. Then I searched for 0x82 or 0x8a in the traces, didn't find it, and got terribly confused. What actually happens doesn't match what the SDM says. You correctly cite this: > Per 28.2.3.2 EPT Violations: "Writes by the logical processor to guest > paging structures to update accessed and dirty flags are considered to > be data writes." though here you mean EPT_VIOLATION_GVA_TRANSLATED=0, EPT_VIOLATION_ACC_WRITE=1: > In other words, it's valid for > EPT_VIOLATION_GVA_TRANSLATED and EPT_VIOLATION_ACC_WRITE to both be > set in the exit qual when EPT A/D is disabled. What really happens is that the processor first reads the page tables, then translates the resulting GPA, and then finally does _another_ EPT translation for the page tables, this time with EPT_VIOLATION_ACC_READ=1 and EPT_VIOLATION_ACC_WRITE=1: qemu-kvm-3683 [042] 2364.196479: kvm_entry: vcpu 0 qemu-kvm-3683 [042] 2364.196479: kvm_exit: reason EPT_VIOLATION rip 0x40519e info 8b 0 qemu-kvm-3683 [042] 2364.196479: kvm_nested_vmexit: rip: 0x000000000040519e reason: EPT_VIOLATION ext_inf1: 0x000000000000008b ext_inf2: 0x0000000000000000 ext_int: 0x00000000 ext_int_err: 0x00000000 qemu-kvm-3683 [042] 2364.196480: kvm_page_fault: address 479000 error_code 8b qemu-kvm-3683 [042] 2364.196480: kvm_mmu_pagetable_walk: addr 479000 pferr 7 P|W|U qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 472007 level 4 qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 473007 level 3 qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 67c007 level 2 qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 479001 level 1 (This trace comes from vanilla Linux 4.11.0). This matches your kvm-unit-tests patch ("x86: fix ept_access_test_paddr exit qualifications"). Aargh. So Peter is right; the only way to get this behavior is to run with EPT A/D bits disabled when L1 disables them. I'll wait for you guys to send a patch for this---we have plenty of time to include it in 4.12.0. In the meanwhile Radim shall drop patch 2 from the KVM series. Thanks, Paolo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-12 8:45 ` Paolo Bonzini @ 2017-05-12 23:45 ` David Matlack 2017-05-13 12:44 ` Paolo Bonzini 0 siblings, 1 reply; 9+ messages in thread From: David Matlack @ 2017-05-12 23:45 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Feiner, kvm list, Xiao Guangrong, Radim Krčmář On Fri, May 12, 2017 at 1:45 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 11/05/2017 21:51, David Matlack wrote: >>> No, I've tried the tests on upstream Linux with eptad=0 (so that EPT A/D >>> is not used by KVM on the host) and they also hang with an infinite stream >>> of EPT violations. >> >> I think the failures are caused by this code in handle_ept_violation, >> which clears the ACC_WRITE bit of the exit qualification before >> handling the fault, when EPT A/D is disabled: >> >> if (is_guest_mode(vcpu) >> && !(exit_qualification & EPT_VIOLATION_GVA_TRANSLATED)) { >> /* >> * Fix up exit_qualification according to whether guest >> * page table accesses are reads or writes. >> */ >> u64 eptp = nested_ept_get_cr3(vcpu); >> if (!(eptp & VMX_EPT_AD_ENABLE_BIT)) >> exit_qualification &= ~EPT_VIOLATION_ACC_WRITE; >> } > > I thought so as well; that's why my hypervisor patch moved > trace_kvm_page_fault before munging exit_qualification. Then I searched > for 0x82 or 0x8a in the traces, didn't find it, and got terribly confused. > > What actually happens doesn't match what the SDM says. You correctly cite this: > >> Per 28.2.3.2 EPT Violations: "Writes by the logical processor to guest >> paging structures to update accessed and dirty flags are considered to >> be data writes." > > though here you mean EPT_VIOLATION_GVA_TRANSLATED=0, EPT_VIOLATION_ACC_WRITE=1: > >> In other words, it's valid for >> EPT_VIOLATION_GVA_TRANSLATED and EPT_VIOLATION_ACC_WRITE to both be >> set in the exit qual when EPT A/D is disabled. > > What really happens is that the processor first reads the page tables, > then translates the resulting GPA, and then finally does _another_ EPT > translation for the page tables, this time with EPT_VIOLATION_ACC_READ=1 > and EPT_VIOLATION_ACC_WRITE=1: > > qemu-kvm-3683 [042] 2364.196479: kvm_entry: vcpu 0 > qemu-kvm-3683 [042] 2364.196479: kvm_exit: reason EPT_VIOLATION rip 0x40519e info 8b 0 > qemu-kvm-3683 [042] 2364.196479: kvm_nested_vmexit: rip: 0x000000000040519e reason: EPT_VIOLATION ext_inf1: 0x000000000000008b ext_inf2: 0x0000000000000000 ext_int: 0x00000000 ext_int_err: 0x00000000 > qemu-kvm-3683 [042] 2364.196480: kvm_page_fault: address 479000 error_code 8b > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_pagetable_walk: addr 479000 pferr 7 P|W|U > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 472007 level 4 > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 473007 level 3 > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 67c007 level 2 > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte 479001 level 1 > > (This trace comes from vanilla Linux 4.11.0). This matches your kvm-unit-tests > patch ("x86: fix ept_access_test_paddr exit qualifications"). Aargh. > > So Peter is right; the only way to get this behavior is to run with > EPT A/D bits disabled when L1 disables them. I'll wait for you guys to send > a patch for this---we have plenty of time to include it in 4.12.0. In the > meanwhile Radim shall drop patch 2 from the KVM series. Sounds good, we'll get you that patch. When does 4.12.0 close? > > Thanks, > > Paolo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests 2017-05-12 23:45 ` David Matlack @ 2017-05-13 12:44 ` Paolo Bonzini 0 siblings, 0 replies; 9+ messages in thread From: Paolo Bonzini @ 2017-05-13 12:44 UTC (permalink / raw) To: David Matlack Cc: Peter Feiner, kvm list, Xiao Guangrong, Radim Krčmář > On Fri, May 12, 2017 at 1:45 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 11/05/2017 21:51, David Matlack wrote: > >>> No, I've tried the tests on upstream Linux with eptad=0 (so that EPT A/D > >>> is not used by KVM on the host) and they also hang with an infinite > >>> stream > >>> of EPT violations. > >> > >> I think the failures are caused by this code in handle_ept_violation, > >> which clears the ACC_WRITE bit of the exit qualification before > >> handling the fault, when EPT A/D is disabled: > >> > >> if (is_guest_mode(vcpu) > >> && !(exit_qualification & EPT_VIOLATION_GVA_TRANSLATED)) { > >> /* > >> * Fix up exit_qualification according to whether guest > >> * page table accesses are reads or writes. > >> */ > >> u64 eptp = nested_ept_get_cr3(vcpu); > >> if (!(eptp & VMX_EPT_AD_ENABLE_BIT)) > >> exit_qualification &= ~EPT_VIOLATION_ACC_WRITE; > >> } > > > > I thought so as well; that's why my hypervisor patch moved > > trace_kvm_page_fault before munging exit_qualification. Then I searched > > for 0x82 or 0x8a in the traces, didn't find it, and got terribly confused. > > > > What actually happens doesn't match what the SDM says. You correctly cite > > this: > > > >> Per 28.2.3.2 EPT Violations: "Writes by the logical processor to guest > >> paging structures to update accessed and dirty flags are considered to > >> be data writes." > > > > though here you mean EPT_VIOLATION_GVA_TRANSLATED=0, > > EPT_VIOLATION_ACC_WRITE=1: > > > >> In other words, it's valid for > >> EPT_VIOLATION_GVA_TRANSLATED and EPT_VIOLATION_ACC_WRITE to both be > >> set in the exit qual when EPT A/D is disabled. > > > > What really happens is that the processor first reads the page tables, > > then translates the resulting GPA, and then finally does _another_ EPT > > translation for the page tables, this time with EPT_VIOLATION_ACC_READ=1 > > and EPT_VIOLATION_ACC_WRITE=1: > > > > qemu-kvm-3683 [042] 2364.196479: kvm_entry: vcpu 0 > > qemu-kvm-3683 [042] 2364.196479: kvm_exit: reason > > EPT_VIOLATION rip 0x40519e info 8b 0 > > qemu-kvm-3683 [042] 2364.196479: kvm_nested_vmexit: rip: > > 0x000000000040519e reason: EPT_VIOLATION ext_inf1: > > 0x000000000000008b ext_inf2: 0x0000000000000000 ext_int: > > 0x00000000 ext_int_err: 0x00000000 > > qemu-kvm-3683 [042] 2364.196480: kvm_page_fault: address > > 479000 error_code 8b > > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_pagetable_walk: addr > > 479000 pferr 7 P|W|U > > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte > > 472007 level 4 > > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte > > 473007 level 3 > > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte > > 67c007 level 2 > > qemu-kvm-3683 [042] 2364.196480: kvm_mmu_paging_element: pte > > 479001 level 1 > > > > (This trace comes from vanilla Linux 4.11.0). This matches your > > kvm-unit-tests > > patch ("x86: fix ept_access_test_paddr exit qualifications"). Aargh. > > > > So Peter is right; the only way to get this behavior is to run with > > EPT A/D bits disabled when L1 disables them. I'll wait for you guys to > > send > > a patch for this---we have plenty of time to include it in 4.12.0. In the > > meanwhile Radim shall drop patch 2 from the KVM series. > > Sounds good, we'll get you that patch. When does 4.12.0 close? Sending it within the next 3-4 weeks should be fine, even if it needs a couple iterations to get everything in shape. Paolo ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-05-13 12:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-11 11:23 [PATCH kvm-unit-tests 0/2] vmx: fixes to v2 tests Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 1/2] vmx: always perform access with modified page table for PADDR tests Paolo Bonzini 2017-05-11 11:23 ` [PATCH kvm-unit-tests 2/2] vmx: fix expected results of new EPT tests Paolo Bonzini 2017-05-11 15:58 ` Peter Feiner 2017-05-11 17:30 ` Paolo Bonzini 2017-05-11 19:51 ` David Matlack 2017-05-12 8:45 ` Paolo Bonzini 2017-05-12 23:45 ` David Matlack 2017-05-13 12:44 ` Paolo Bonzini
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.