| Message ID | 20210622210047.3691840-6-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nSVM: NPT improvements and cleanups | expand |
On 22/06/21 23:00, Sean Christopherson wrote: > Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will > provide a superset of their functionality, e.g. the current tests are > limited in the sense that they test a single entry and a single bit, > e.g. don't test conditionally-reserved bits. > > The npt_rsvd test in particular is quite nasty as it subtly relies on > EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness > (it's forced for_all_ tests, presumably to get the desired PFEC.FETCH=1 > for this one test). > > Signed-off-by: Sean Christopherson<seanjc@google.com> > --- > x86/svm_tests.c | 45 --------------------------------------------- > 1 file changed, 45 deletions(-) This exposes a KVM bug, reproducible with ./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \ -append 'npt_rw npt_rw_pfwalk' While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2 (address for the NPF location; on my machine it gets 0xbfede6f0 instead of 0xbfede000). The same tests work with QEMU from git. I didn't quite finish analyzing it, but my current theory is that KVM receives a pagewalk NPF for a *different* page walk that is caused by read-only page tables; then it finds that the page walk to 0xbfede6f0 *does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn) and therefore injects it anyway. This theory is because the 0x6f0 offset in the page table corresponds to the 0xde000 part of the faulting address. Maxim will look into it while I'm away. Paolo
On Thu, Jun 24, 2021, Paolo Bonzini wrote: > On 22/06/21 23:00, Sean Christopherson wrote: > > Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will > > provide a superset of their functionality, e.g. the current tests are > > limited in the sense that they test a single entry and a single bit, > > e.g. don't test conditionally-reserved bits. > > > > The npt_rsvd test in particular is quite nasty as it subtly relies on > > EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness > > (it's forced for_all_ tests, presumably to get the desired PFEC.FETCH=1 > > for this one test). > > > > Signed-off-by: Sean Christopherson<seanjc@google.com> > > --- > > x86/svm_tests.c | 45 --------------------------------------------- > > 1 file changed, 45 deletions(-) > > This exposes a KVM bug, reproducible with > > ./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \ > -append 'npt_rw npt_rw_pfwalk' Any chance you're running against an older KVM version? The test passes if I run against a build with my MMU pile on top of kvm/queue, but fails on a random older KVM. Side topic, these tests all fail to invalidate TLB entries after modifying PTEs. I suspect they work in part because KVM flushes and syncs on all nested SVM transitions... > While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2 > (address for the NPF location; on my machine it gets 0xbfede6f0 instead of > 0xbfede000). The same tests work with QEMU from git. > > I didn't quite finish analyzing it, but my current theory is > that KVM receives a pagewalk NPF for a *different* page walk that is caused > by read-only page tables; then it finds that the page walk to 0xbfede6f0 > *does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn) > and therefore injects it anyway. This theory is because the 0x6f0 offset in > the page table corresponds to the 0xde000 part of the faulting address. > Maxim will look into it while I'm away. > > Paolo >
On 24/06/21 19:43, Sean Christopherson wrote: >> ./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \ >> -append 'npt_rw npt_rw_pfwalk' > Any chance you're running against an older KVM version? The test passes if I > run against a build with my MMU pile on top of kvm/queue, but fails on a random > older KVM. I'm running it against the current kvm/queue. Paolo
On Thu, Jun 24, 2021, Paolo Bonzini wrote: > On 24/06/21 19:43, Sean Christopherson wrote: > > > ./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \ > > > -append 'npt_rw npt_rw_pfwalk' > > Any chance you're running against an older KVM version? The test passes if I > > run against a build with my MMU pile on top of kvm/queue, but fails on a random > > older KVM. > > I'm running it against the current kvm/queue. Huh. Still passes for me on 292fedba687b ("Merge branch 'kvm-c-bit' into HEAD"). I'm running on AMD EPYC 7B12 / Rome, if that helps at all.
On Thu, 2021-06-24 at 17:43 +0000, Sean Christopherson wrote: > On Thu, Jun 24, 2021, Paolo Bonzini wrote: > > On 22/06/21 23:00, Sean Christopherson wrote: > > > Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will > > > provide a superset of their functionality, e.g. the current tests are > > > limited in the sense that they test a single entry and a single bit, > > > e.g. don't test conditionally-reserved bits. > > > > > > The npt_rsvd test in particular is quite nasty as it subtly relies on > > > EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness > > > (it's forced for_all_ tests, presumably to get the desired PFEC.FETCH=1 > > > for this one test). > > > > > > Signed-off-by: Sean Christopherson<seanjc@google.com> > > > --- > > > x86/svm_tests.c | 45 --------------------------------------------- > > > 1 file changed, 45 deletions(-) > > > > This exposes a KVM bug, reproducible with > > > > ./x86/run x86/svm.flat -smp 2 -cpu max,+svm -m 4g \ > > -append 'npt_rw npt_rw_pfwalk' > > Any chance you're running against an older KVM version? The test passes if I > run against a build with my MMU pile on top of kvm/queue, but fails on a random > older KVM. > > Side topic, these tests all fail to invalidate TLB entries after modifying PTEs. > I suspect they work in part because KVM flushes and syncs on all nested SVM > transitions... I also now tried to reproduce and the test passes. Best regards, Maxim Levitsky > > > While running npt_rw_pfwalk, the #NPF gets an incorrect EXITINFO2 > > (address for the NPF location; on my machine it gets 0xbfede6f0 instead of > > 0xbfede000). The same tests work with QEMU from git. > > > > I didn't quite finish analyzing it, but my current theory is > > that KVM receives a pagewalk NPF for a *different* page walk that is caused > > by read-only page tables; then it finds that the page walk to 0xbfede6f0 > > *does fail* (after all the correct and wrong EXITINFO2 belong to the same pfn) > > and therefore injects it anyway. This theory is because the 0x6f0 offset in > > the page table corresponds to the 0xde000 part of the faulting address. > > Maxim will look into it while I'm away. > > > > Paolo > >
diff --git a/x86/svm_tests.c b/x86/svm_tests.c index 506bd75..96add48 100644 --- a/x86/svm_tests.c +++ b/x86/svm_tests.c @@ -774,28 +774,6 @@ static bool npt_us_check(struct svm_test *test) && (vmcb->control.exit_info_1 == 0x100000005ULL); } -u64 save_pde; - -static void npt_rsvd_prepare(struct svm_test *test) -{ - u64 *pde; - - pde = npt_get_pde((u64) null_test); - - save_pde = *pde; - *pde = (1ULL << 19) | (1ULL << 7) | 0x27; -} - -static bool npt_rsvd_check(struct svm_test *test) -{ - u64 *pde = npt_get_pde((u64) null_test); - - *pde = save_pde; - - return (vmcb->control.exit_code == SVM_EXIT_NPF) - && (vmcb->control.exit_info_1 == 0x10000001dULL); -} - static void npt_rw_prepare(struct svm_test *test) { @@ -844,23 +822,6 @@ static bool npt_rw_pfwalk_check(struct svm_test *test) && (vmcb->control.exit_info_2 == read_cr3()); } -static void npt_rsvd_pfwalk_prepare(struct svm_test *test) -{ - u64 *pdpe; - - pdpe = npt_get_pml4e(); - pdpe[0] |= (1ULL << 8); -} - -static bool npt_rsvd_pfwalk_check(struct svm_test *test) -{ - u64 *pdpe = npt_get_pml4e(); - pdpe[0] &= ~(1ULL << 8); - - return (vmcb->control.exit_code == SVM_EXIT_NPF) - && (vmcb->control.exit_info_1 == 0x20000000fULL); -} - static void npt_l1mmio_prepare(struct svm_test *test) { } @@ -2719,15 +2680,9 @@ struct svm_test svm_tests[] = { { "npt_us", npt_supported, npt_us_prepare, default_prepare_gif_clear, npt_us_test, default_finished, npt_us_check }, - { "npt_rsvd", npt_supported, npt_rsvd_prepare, - default_prepare_gif_clear, null_test, - default_finished, npt_rsvd_check }, { "npt_rw", npt_supported, npt_rw_prepare, default_prepare_gif_clear, npt_rw_test, default_finished, npt_rw_check }, - { "npt_rsvd_pfwalk", npt_supported, npt_rsvd_pfwalk_prepare, - default_prepare_gif_clear, null_test, - default_finished, npt_rsvd_pfwalk_check }, { "npt_rw_pfwalk", npt_supported, npt_rw_pfwalk_prepare, default_prepare_gif_clear, null_test, default_finished, npt_rw_pfwalk_check },
Remove two of nSVM's NPT reserved bits test, a soon-to-be-added test will provide a superset of their functionality, e.g. the current tests are limited in the sense that they test a single entry and a single bit, e.g. don't test conditionally-reserved bits. The npt_rsvd test in particular is quite nasty as it subtly relies on EFER.NX=1; dropping the test will allow cleaning up the EFER.NX weirdness (it's forced for _all_ tests, presumably to get the desired PFEC.FETCH=1 for this one test). Signed-off-by: Sean Christopherson <seanjc@google.com> --- x86/svm_tests.c | 45 --------------------------------------------- 1 file changed, 45 deletions(-)