x86: svm: fixes
cover x86: svm: fixes
Excluding the first patch, the others are bug-fixes for the svm tests, that make the tests more robust, and less KVM-dependent.
The fourth patch might indicate there is a bug in KVM, but I did not get to check it.
Nadav Amit (5):
x86: Remove boot_idt assembly assignment
x86: svm: check TSC adjust support
x86: svm: flush TLB on each test
x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare
x86: svm: avoid advancing rip incorrectly on exc_inject
x86/cstart64.S | 3 --- x86/svm.c | 1 + x86/svm_tests.c | 15 ++++++++++----- 3 files changed, 11 insertions(+), 8 deletions(-)
Signature · 1 line
2.25.1
PATCH 1/5 x86: Remove boot_idt assembly assignment
boot_idt is now a symbol.
--- x86/cstart64.S | 3 --- 1 file changed, 3 deletions(-)
x86/cstart64.S +0 -3
@@ -2,15 +2,12 @@ #include "apic-defs.h" .globl boot_idt -boot_idt = 0 .globl idt_descr .globl tss_descr .globl gdt64_desc .globl online_cpus -boot_idt = 0 - ipi_vector = 0x20 max_cpus = MAX_TEST_CPUS
Signature · 1 line
2.25.1
PATCH 2/5 x86: svm: check TSC adjust support
In reply to Nadav AmitMSR_IA32_TSC_ADJUST may be supported by KVM on AMD machines, but it does not show on AMD manual. Check CPUID to see if it supported before running the relevant tests.
--- x86/svm_tests.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
x86/svm_tests.c +6 -1
@@ -893,6 +893,11 @@ static bool npt_rw_l1mmio_check(struct svm_test *test)#define TSC_OFFSET_VALUE (~0ull << 48)static bool ok; +static bool tsc_adjust_supported (void ) +{ +return this_cpu_has (X86_FEATURE_TSC_ADJUST ); +} +static void tsc_adjust_prepare (struct svm_test *test) {default_prepare (test);
@@ -2010,7 +2015,7 @@ struct svm_test svm_tests[] = { {"npt_rw_l1mmio" , npt_supported, npt_rw_l1mmio_prepare, default_prepare_gif_clear, npt_rw_l1mmio_test, default_finished, npt_rw_l1mmio_check }, - { "tsc_adjust", default_supported, tsc_adjust_prepare, + { "tsc_adjust", tsc_adjust_supported, tsc_adjust_prepare, default_prepare_gif_clear, tsc_adjust_test, default_finished, tsc_adjust_check }, { "latency_run_exit", default_supported, latency_prepare,
Signature · 1 line
2.25.1
PATCH 3/5 x86: svm: flush TLB on each test
In reply to Nadav AmitSeveral svm tests change PTEs but do not flush the TLB. To avoid messing around or encountering new bugs in the future, flush the TLB on every test.
--- x86/svm.c | 1 + 1 file changed, 1 insertion(+)
x86/svm.c +1 -0
@@ -170,6 +170,7 @@ void vmcb_ident(struct vmcb *vmcb)if (npt_supported ()) { ctrl->nested_ctl =1 ; ctrl->nested_cr3 = (u64 )pml4e; + ctrl->tlb_ctl =TLB_CONTROL_FLUSH_ALL_ASID ; } }
Signature · 1 line
2.25.1
PATCH 4/5 x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare
In reply to Nadav AmitAccording to AMD manual bit 8 in PDPE is not reserved, but bit 7.
--- x86/svm_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
x86/svm_tests.c +2 -2
@@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)vmcb_ident (vmcb); pdpe =npt_get_pdpe (); - pdpe[0 ] |= (1ULL <<8 ); + pdpe[0 ] |= (1ULL <<7 ); }static bool npt_rsvd_pfwalk_check (struct svm_test *test) {u64 *pdpe =npt_get_pdpe (); - pdpe[0 ] &= ~(1ULL <<8 ); + pdpe[0 ] &= ~(1ULL <<7 );return (vmcb->control.exit_code ==SVM_EXIT_NPF ) && (vmcb->control.exit_info_1 ==0x20000000eULL );
Signature · 1 line
2.25.1
Paolo Bonzini
maintainerRe: [kvm-unit-tests PATCH 4/5] x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare
Quoted 1 line · Nadav Amit
According to AMD manual bit 8 in PDPE is not reserved, but bit 7.
Indeed. Maybe it was a problem in the previous versions because I remember adding this check explicitly. I've sent a patch.
Paolo
Quoted 26 lines
Signed-off-by Nadav Amit <[email protected]>--- x86/svm_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)x86/svm_tests.c +2
-2@@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)vmcb_ident (vmcb); pdpe =npt_get_pdpe (); - pdpe[0 ] |= (1ULL <<8 ); + pdpe[0 ] |= (1ULL <<7 ); }static bool npt_rsvd_pfwalk_check (struct svm_test *test) {u64 *pdpe =npt_get_pdpe (); - pdpe[0 ] &= ~(1ULL <<8 ); + pdpe[0 ] &= ~(1ULL <<7 );return (vmcb->control.exit_code ==SVM_EXIT_NPF ) && (vmcb->control.exit_info_1 ==0x20000000eULL );
Paolo Bonzini
maintainerRe: [kvm-unit-tests PATCH 4/5] x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare
In reply to Nadav AmitQuoted 18 lines · Nadav Amit
According to AMD manual bit 8 in PDPE is not reserved, but bit 7.
Signed-off-by Nadav Amit <[email protected]>--- x86/svm_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)x86/svm_tests.c +1
-1@@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)vmcb_ident (vmcb); pdpe =npt_get_pdpe (); - pdpe[0 ] |= (1ULL <<8 ); + pdpe[0 ] |= (1ULL <<7 );
x86/svm_tests.c static void npt_rsvd_pfwalk_prepare(struct svm_test *test) · line 837
Quoted 10 lines
x86/svm_tests.c +1
-1}static bool npt_rsvd_pfwalk_check (struct svm_test *test) {u64 *pdpe =npt_get_pdpe (); - pdpe[0 ] &= ~(1ULL <<8 ); + pdpe[0 ] &= ~(1ULL <<7 );return (vmcb->control.exit_code ==SVM_EXIT_NPF ) && (vmcb->control.exit_info_1 ==0x20000000eULL );
Wait, bit 7 is not reserved, it's the PS bit. We need to use the PML4E instead (and then using bit 7 or bit 8 is irrelevant):
x86/svm.c +5 -0
@@ -49,6 +49,11 @@ u64 *npt_get_pdpe(void)return pdpe; } +u64 *npt_get_pml4e (void ) +{ +return pml4e; +} +bool smp_supported (void ) {return cpu_count () >1 ;
x86/svm.h +1 -0
@@ -365,6 +365,7 @@ typedef void (*test_guest_func)(struct svm_test *);u64 *npt_get_pte (u64 address);u64 *npt_get_pde (u64 address);u64 *npt_get_pdpe (void ); +u64 *npt_get_pml4e (void );bool smp_supported (void );bool default_supported (void );void default_prepare (struct svm_test *test);
x86/svm_tests.c +2 -2
@@ -824,13 +824,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)u64 *pdpe;vmcb_ident (vmcb); - pdpe =npt_get_pdpe (); + pdpe =npt_get_pml4e (); pdpe[0 ] |= (1ULL <<8 ); }static bool npt_rsvd_pfwalk_check (struct svm_test *test) { -u64 *pdpe =npt_get_pdpe (); +u64 *pdpe =npt_get_pml4e (); pdpe[0 ] &= ~(1ULL <<8 );return (vmcb->control.exit_code ==SVM_EXIT_NPF )
Re: [kvm-unit-tests PATCH 4/5] x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare
Quoted 28 lines · Paolo Bonzini
Quoted 25 lines · Nadav Amit
According to AMD manual bit 8 in PDPE is not reserved, but bit 7.
Signed-off-by Nadav Amit <[email protected]>---
x86/svm_tests.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)x86/svm_tests.c +2
-2@@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)vmcb_ident (vmcb); pdpe =npt_get_pdpe (); - pdpe[0 ] |= (1ULL <<8 ); + pdpe[0 ] |= (1ULL <<7 ); }static bool npt_rsvd_pfwalk_check (struct svm_test *test) {u64 *pdpe =npt_get_pdpe (); - pdpe[0 ] &= ~(1ULL <<8 ); + pdpe[0 ] &= ~(1ULL <<7 );
Quoted 5 lines
x86/svm_tests.c static void npt_rsvd_pfwalk_prepare(struct svm_test *test) · line 837
Quoted 2 lines
x86/svm_tests.c +0
-0return (vmcb->control.exit_code ==SVM_EXIT_NPF ) && (vmcb->control.exit_info_1 ==0x20000000eULL );Wait, bit 7 is not reserved, it's the PS bit. We need to use the PML4E instead (and then using bit 7 or bit 8 is irrelevant):
Err.. I remembered that bit 7 is not zero for no reason.
Now I wonder why it caused #PF at all. To be fair, I did not run it on bare-metal yet, since I do not have access to such a machine.
PATCH 5/5 x86: svm: avoid advancing rip incorrectly on exc_inject
In reply to Nadav Amitexc_inject advances the ripon every stage, so it can do so 3 times, but there are only 2 vmmcall instructions that the guest runs. So, if a failure happens on the last test, there is no vmmcall instruction to trigger an exit.
Advance the rip only in the two stages in which vmmcall is expected to run.
--- x86/svm_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
x86/svm_tests.c +2 -2
@@ -1593,8 +1593,6 @@ static void exc_inject_test(struct svm_test *test)static bool exc_inject_finished (struct svm_test *test) { - vmcb->save.rip +=3 ; -switch (get_test_stage (test)) {case 0 :if (vmcb->control.exit_code !=SVM_EXIT_VMMCALL ) {
@@ -1602,6 +1600,7 @@ static bool exc_inject_finished(struct svm_test *test) vmcb->control.exit_code);return true; } + vmcb->save.rip +=3 ; vmcb->control.event_inj =NMI_VECTOR |SVM_EVTINJ_TYPE_EXEPT |SVM_EVTINJ_VALID ;break ;
@@ -1621,6 +1620,7 @@ static bool exc_inject_finished(struct svm_test *test) vmcb->control.exit_code);return true; } + vmcb->save.rip +=3 ;report (count_exc ==1 ,"divide overflow exception injected" );report (!(vmcb->control.event_inj &SVM_EVTINJ_VALID ),"eventinj.VALID cleared" );break ;
Signature · 1 line
2.25.1