Re: [kvm-unit-tests PATCH v3 18/27] svm: move vmcb_ident to svm_lib.c

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2022-12-01 at 17:18 +0100, Emanuele Giuseppe Esposito wrote:
> 
> Am 22/11/2022 um 17:11 schrieb Maxim Levitsky:
> > Extract vmcb_ident to svm_lib.c
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> 
> Not sure if it holds for kvm unit tests, but indent of vmcb_set_seg
> parameters seems a little bit off.

True, I will fix.

Best regards,
	Maxim Levitsky

> 
> If that's fine:
> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@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(-)
> > 
> > diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c
> > index c7194909..aed757a1 100644
> > --- a/lib/x86/svm_lib.c
> > +++ b/lib/x86/svm_lib.c
> > @@ -109,3 +109,57 @@ bool setup_svm(void)
> >  	setup_npt();
> >  	return true;
> >  }
> > +
> > +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 f603ff93..3bb098dc 100644
> > --- a/lib/x86/svm_lib.h
> > +++ b/lib/x86/svm_lib.h
> > @@ -49,7 +49,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);
> > +
> >  bool 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 cf246c37..5e2c3a83 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 67f3205d..a4aabeb2 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);
> >