| Message ID | 20221020152404.283980-14-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm-unit-tests: set of fixes and new tests | expand |
On Thu, Oct 20, 2022, Maxim Levitsky wrote: Changelog please. > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > lib/x86/svm_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ > lib/x86/svm_lib.h | 4 ++++ What about calling these simply svm.{c,h} and renaming x86/svm.{c,h} to something like svm_common.{c,h}? Long term, it would be wonderful to rid of x86/svm.{c,h} by genericizing the test framework, e.g. there's a ton of duplicate code between SVM and VMX. > x86/svm.c | 54 ----------------------------------------------- > x86/svm.h | 1 - > 4 files changed, 58 insertions(+), 55 deletions(-) > > diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c > index 9e82e363..2b067c65 100644 > --- a/lib/x86/svm_lib.c > +++ b/lib/x86/svm_lib.c > @@ -103,3 +103,57 @@ void setup_svm(void) > > setup_npt(); > } > + > +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, > + u64 base, u32 limit, u32 attr) Funky indentation and wrap. void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, u64 base, u32 limit, u32 attr) > +{ > + seg->selector = selector; > + seg->attrib = attr; > + seg->limit = limit; > + seg->base = base; > +} > + > +void vmcb_ident(struct vmcb *vmcb) > +{ > + u64 vmcb_phys = virt_to_phys(vmcb); > + struct vmcb_save_area *save = &vmcb->save; > + struct vmcb_control_area *ctrl = &vmcb->control; > + u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK Ugh, a #define for '3' and '9' (in lib/x86/desc.h?) would be nice, but that can be left for another day/patch. > + | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; Pre-existing mess, but can you move the '|' to the previous line? And align the code? > + u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK > + | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK; | on the previous line. u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK;
On Thu, 2022-10-20 at 18:37 +0000, Sean Christopherson wrote: > On Thu, Oct 20, 2022, Maxim Levitsky wrote: > > Changelog please. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > lib/x86/svm_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ > > lib/x86/svm_lib.h | 4 ++++ > > What about calling these simply svm.{c,h} and renaming x86/svm.{c,h} to something > like svm_common.{c,h}? Long term, it would be wonderful to rid of x86/svm.{c,h} > by genericizing the test framework, e.g. there's a ton of duplicate code between > SVM and VMX. Makes sense. > > > x86/svm.c | 54 ----------------------------------------------- > > x86/svm.h | 1 - > > 4 files changed, 58 insertions(+), 55 deletions(-) > > > > diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c > > index 9e82e363..2b067c65 100644 > > --- a/lib/x86/svm_lib.c > > +++ b/lib/x86/svm_lib.c > > @@ -103,3 +103,57 @@ void setup_svm(void) > > > > setup_npt(); > > } > > + > > +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, > > + u64 base, u32 limit, u32 attr) > > Funky indentation and wrap. > > void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, u64 base, u32 limit, > u32 attr) > > > +{ > > + seg->selector = selector; > > + seg->attrib = attr; > > + seg->limit = limit; > > + seg->base = base; > > +} > > + > > +void vmcb_ident(struct vmcb *vmcb) > > +{ > > + u64 vmcb_phys = virt_to_phys(vmcb); > > + struct vmcb_save_area *save = &vmcb->save; > > + struct vmcb_control_area *ctrl = &vmcb->control; > > + u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK > > Ugh, a #define for '3' and '9' (in lib/x86/desc.h?) would be nice, but that can > be left for another day/patch. Exactly. > > > + | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; > > Pre-existing mess, but can you move the '|' to the previous line? And align the > code? > > > + u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK > > + | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK; > > > on the previous line. OK. > > u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK | > SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; > u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK | > SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK; > Best regards, Maxim Levitsky
diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c index 9e82e363..2b067c65 100644 --- a/lib/x86/svm_lib.c +++ b/lib/x86/svm_lib.c @@ -103,3 +103,57 @@ void setup_svm(void) setup_npt(); } + +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, + u64 base, u32 limit, u32 attr) +{ + seg->selector = selector; + seg->attrib = attr; + seg->limit = limit; + seg->base = base; +} + +void vmcb_ident(struct vmcb *vmcb) +{ + u64 vmcb_phys = virt_to_phys(vmcb); + struct vmcb_save_area *save = &vmcb->save; + struct vmcb_control_area *ctrl = &vmcb->control; + u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK + | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; + u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK + | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK; + struct descriptor_table_ptr desc_table_ptr; + + memset(vmcb, 0, sizeof(*vmcb)); + asm volatile ("vmsave %0" : : "a"(vmcb_phys) : "memory"); + vmcb_set_seg(&save->es, read_es(), 0, -1U, data_seg_attr); + vmcb_set_seg(&save->cs, read_cs(), 0, -1U, code_seg_attr); + vmcb_set_seg(&save->ss, read_ss(), 0, -1U, data_seg_attr); + vmcb_set_seg(&save->ds, read_ds(), 0, -1U, data_seg_attr); + sgdt(&desc_table_ptr); + vmcb_set_seg(&save->gdtr, 0, desc_table_ptr.base, desc_table_ptr.limit, 0); + sidt(&desc_table_ptr); + vmcb_set_seg(&save->idtr, 0, desc_table_ptr.base, desc_table_ptr.limit, 0); + ctrl->asid = 1; + save->cpl = 0; + save->efer = rdmsr(MSR_EFER); + save->cr4 = read_cr4(); + save->cr3 = read_cr3(); + save->cr0 = read_cr0(); + save->dr7 = read_dr7(); + save->dr6 = read_dr6(); + save->cr2 = read_cr2(); + save->g_pat = rdmsr(MSR_IA32_CR_PAT); + save->dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR); + ctrl->intercept = (1ULL << INTERCEPT_VMRUN) | + (1ULL << INTERCEPT_VMMCALL) | + (1ULL << INTERCEPT_SHUTDOWN); + ctrl->iopm_base_pa = virt_to_phys(io_bitmap); + ctrl->msrpm_base_pa = virt_to_phys(msr_bitmap); + + if (npt_supported()) { + ctrl->nested_ctl = 1; + ctrl->nested_cr3 = (u64)pml4e; + ctrl->tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID; + } +} diff --git a/lib/x86/svm_lib.h b/lib/x86/svm_lib.h index 50664b24..27c3b137 100644 --- a/lib/x86/svm_lib.h +++ b/lib/x86/svm_lib.h @@ -54,7 +54,11 @@ static inline void clgi(void) asm volatile ("clgi"); } +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, + u64 base, u32 limit, u32 attr); + void setup_svm(void); +void vmcb_ident(struct vmcb *vmcb); u64 *npt_get_pte(u64 address); u64 *npt_get_pde(u64 address); diff --git a/x86/svm.c b/x86/svm.c index bf8caf54..37b4cd38 100644 --- a/x86/svm.c +++ b/x86/svm.c @@ -63,15 +63,6 @@ void inc_test_stage(struct svm_test *test) barrier(); } -static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, - u64 base, u32 limit, u32 attr) -{ - seg->selector = selector; - seg->attrib = attr; - seg->limit = limit; - seg->base = base; -} - static test_guest_func guest_main; void test_set_guest(test_guest_func func) @@ -85,51 +76,6 @@ static void test_thunk(struct svm_test *test) vmmcall(); } -void vmcb_ident(struct vmcb *vmcb) -{ - u64 vmcb_phys = virt_to_phys(vmcb); - struct vmcb_save_area *save = &vmcb->save; - struct vmcb_control_area *ctrl = &vmcb->control; - u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK - | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK; - u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK - | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK; - struct descriptor_table_ptr desc_table_ptr; - - memset(vmcb, 0, sizeof(*vmcb)); - asm volatile ("vmsave %0" : : "a"(vmcb_phys) : "memory"); - vmcb_set_seg(&save->es, read_es(), 0, -1U, data_seg_attr); - vmcb_set_seg(&save->cs, read_cs(), 0, -1U, code_seg_attr); - vmcb_set_seg(&save->ss, read_ss(), 0, -1U, data_seg_attr); - vmcb_set_seg(&save->ds, read_ds(), 0, -1U, data_seg_attr); - sgdt(&desc_table_ptr); - vmcb_set_seg(&save->gdtr, 0, desc_table_ptr.base, desc_table_ptr.limit, 0); - sidt(&desc_table_ptr); - vmcb_set_seg(&save->idtr, 0, desc_table_ptr.base, desc_table_ptr.limit, 0); - ctrl->asid = 1; - save->cpl = 0; - save->efer = rdmsr(MSR_EFER); - save->cr4 = read_cr4(); - save->cr3 = read_cr3(); - save->cr0 = read_cr0(); - save->dr7 = read_dr7(); - save->dr6 = read_dr6(); - save->cr2 = read_cr2(); - save->g_pat = rdmsr(MSR_IA32_CR_PAT); - save->dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR); - ctrl->intercept = (1ULL << INTERCEPT_VMRUN) | - (1ULL << INTERCEPT_VMMCALL) | - (1ULL << INTERCEPT_SHUTDOWN); - ctrl->iopm_base_pa = virt_to_phys(svm_get_io_bitmap()); - ctrl->msrpm_base_pa = virt_to_phys(svm_get_msr_bitmap()); - - if (npt_supported()) { - ctrl->nested_ctl = 1; - ctrl->nested_cr3 = (u64)npt_get_pml4e(); - ctrl->tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID; - } -} - struct regs regs; struct regs get_regs(void) diff --git a/x86/svm.h b/x86/svm.h index f4343883..623f2b36 100644 --- a/x86/svm.h +++ b/x86/svm.h @@ -55,7 +55,6 @@ bool default_finished(struct svm_test *test); int get_test_stage(struct svm_test *test); void set_test_stage(struct svm_test *test, int s); void inc_test_stage(struct svm_test *test); -void vmcb_ident(struct vmcb *vmcb); struct regs get_regs(void); int __svm_vmrun(u64 rip); void __svm_bare_vmrun(void);
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- lib/x86/svm_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ lib/x86/svm_lib.h | 4 ++++ x86/svm.c | 54 ----------------------------------------------- x86/svm.h | 1 - 4 files changed, 58 insertions(+), 55 deletions(-)