Re: [kvm-unit-tests PATCH v3 15/27] svm: move some svm support functions into lib/x86/svm_lib.h

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

Am 22/11/2022 um 17:11 schrieb Maxim Levitsky:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  lib/x86/svm_lib.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>  x86/svm.c         | 36 +-------------------------------
>  x86/svm.h         | 18 ----------------
>  x86/svm_npt.c     |  1 +
>  x86/svm_tests.c   |  1 +
>  5 files changed, 56 insertions(+), 53 deletions(-)
>  create mode 100644 lib/x86/svm_lib.h
> 
> diff --git a/lib/x86/svm_lib.h b/lib/x86/svm_lib.h
> new file mode 100644
> index 00000000..04910281
> --- /dev/null
> +++ b/lib/x86/svm_lib.h
> @@ -0,0 +1,53 @@
> +#ifndef SRC_LIB_X86_SVM_LIB_H_
> +#define SRC_LIB_X86_SVM_LIB_H_
> +
> +#include <x86/svm.h>
> +#include "processor.h"
> +
> +static inline bool npt_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_NPT);
> +}
> +
> +static inline bool vgif_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_VGIF);
> +}
> +
> +static inline bool lbrv_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_LBRV);
> +}
> +
> +static inline bool tsc_scale_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_TSCRATEMSR);
> +}
> +
> +static inline bool pause_filter_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_PAUSEFILTER);
> +}
> +
> +static inline bool pause_threshold_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_PFTHRESHOLD);
> +}
> +
> +static inline void vmmcall(void)
> +{
> +	asm volatile ("vmmcall" : : : "memory");
> +}
> +
> +static inline void stgi(void)
> +{
> +	asm volatile ("stgi");
> +}
> +
> +static inline void clgi(void)
> +{
> +	asm volatile ("clgi");
> +}
> +
Not an expert at all on this, but sti() and cli() in patch 1 are in
processor.h and stgi (g stansd for global?) and clgi are in a different
header? What about maybe moving them together?

> +
> +#endif /* SRC_LIB_X86_SVM_LIB_H_ */
> diff --git a/x86/svm.c b/x86/svm.c
> index 0b2a1d69..8d90a242 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -14,6 +14,7 @@
>  #include "alloc_page.h"
>  #include "isr.h"
>  #include "apic.h"
> +#include "svm_lib.h"
>  
>  /* for the nested page table*/
>  u64 *pml4e;
> @@ -54,32 +55,6 @@ bool default_supported(void)
>  	return true;
>  }
>  
> -bool vgif_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_VGIF);
> -}
> -
> -bool lbrv_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_LBRV);
> -}
> -
> -bool tsc_scale_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_TSCRATEMSR);
> -}
> -
> -bool pause_filter_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_PAUSEFILTER);
> -}
> -
> -bool pause_threshold_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_PFTHRESHOLD);
> -}
> -
> -
>  void default_prepare(struct svm_test *test)
>  {
>  	vmcb_ident(vmcb);
> @@ -94,10 +69,6 @@ bool default_finished(struct svm_test *test)
>  	return true; /* one vmexit */
>  }
>  
> -bool npt_supported(void)
> -{
> -	return this_cpu_has(X86_FEATURE_NPT);
> -}
>  
>  int get_test_stage(struct svm_test *test)
>  {
> @@ -128,11 +99,6 @@ static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
>  	seg->base = base;
>  }
>  
> -inline void vmmcall(void)
> -{
> -	asm volatile ("vmmcall" : : : "memory");
> -}
> -
>  static test_guest_func guest_main;
>  
>  void test_set_guest(test_guest_func func)
> diff --git a/x86/svm.h b/x86/svm.h
> index 3cd7ce8b..7cb1b898 100644
> --- a/x86/svm.h
> +++ b/x86/svm.h
> @@ -53,21 +53,14 @@ u64 *npt_get_pdpe(u64 address);
>  u64 *npt_get_pml4e(void);
>  bool smp_supported(void);
>  bool default_supported(void);
> -bool vgif_supported(void);
> -bool lbrv_supported(void);
> -bool tsc_scale_supported(void);
> -bool pause_filter_supported(void);
> -bool pause_threshold_supported(void);
>  void default_prepare(struct svm_test *test);
>  void default_prepare_gif_clear(struct svm_test *test);
>  bool default_finished(struct svm_test *test);
> -bool npt_supported(void);
>  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);
> -void vmmcall(void);
>  int __svm_vmrun(u64 rip);
>  void __svm_bare_vmrun(void);
>  int svm_vmrun(void);
> @@ -75,17 +68,6 @@ void test_set_guest(test_guest_func func);
>  
>  extern struct vmcb *vmcb;
>  
> -static inline void stgi(void)
> -{
> -    asm volatile ("stgi");
> -}
> -
> -static inline void clgi(void)
> -{
> -    asm volatile ("clgi");
> -}
> -
> -
>  
>  #define SAVE_GPR_C                              \
>          "xchg %%rbx, regs+0x8\n\t"              \
> diff --git a/x86/svm_npt.c b/x86/svm_npt.c
> index b791f1ac..8aac0bb6 100644
> --- a/x86/svm_npt.c
> +++ b/x86/svm_npt.c
> @@ -2,6 +2,7 @@
>  #include "vm.h"
>  #include "alloc_page.h"
>  #include "vmalloc.h"
> +#include "svm_lib.h"
>  
>  static void *scratch_page;
>  
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 202e9271..f86c2fa4 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -12,6 +12,7 @@
>  #include "delay.h"
>  #include "x86/usermode.h"
>  #include "vmalloc.h"
> +#include "svm_lib.h"
>  
>  #define SVM_EXIT_MAX_DR_INTERCEPT 0x3f
>  
>