Re: [kvm-unit-tests PATCH 11/16] svm: add svm_suported

[Sean Christopherson <seanjc@xxxxxxxxxx>]

s/suported/supported

On Thu, Oct 20, 2022, Maxim Levitsky wrote:

Please provide a changelog.

> Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> ---
>  lib/x86/svm_lib.h | 5 +++++
>  x86/svm.c         | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/x86/svm_lib.h b/lib/x86/svm_lib.h
> index 04910281..2d13b066 100644
> --- a/lib/x86/svm_lib.h
> +++ b/lib/x86/svm_lib.h
> @@ -4,6 +4,11 @@
>  #include <x86/svm.h>
>  #include "processor.h"
>  
> +static inline bool svm_supported(void)
> +{
> +	return this_cpu_has(X86_FEATURE_SVM);

Why add a wrapper?  The only reason NPT and a few others have wrappers is to
play nice with svm_test's "bool (*supported)(void)" hook.

I would rather go the opposite direction and get rid of the wrappers, which IMO
only make it harder to understand what is being checked.

E.g. add a required_feature to the tests and use that for all X86_FEATURE_*
checks instead of adding wrappers.  And unless there's a supported helper I'm not
seeing, the .supported hook can go away entirely by adding a dedicated "smp_required"
flag.

We'd probaby want helper macros for SMP vs. non-SMP, e.g.

#define SVM_V1_TEST(name, feature, ...)
	{ #name, feature, false, ... }
#define SVM_SMP_V1_TEST(name, feature, ...)
	{ #name, feature, true, ... }

diff --git a/x86/svm.c b/x86/svm.c
index 7aa3ebd2..2a412c27 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -170,6 +170,7 @@ test_wanted(const char *name, char *filters[], int filter_count)
 
 int run_svm_tests(int ac, char **av, struct svm_test *svm_tests)
 {
+       bool smp_supported = cpu_count() > 1;
        int i = 0;
 
        ac--;
@@ -187,7 +188,10 @@ int run_svm_tests(int ac, char **av, struct svm_test *svm_tests)
        for (; svm_tests[i].name != NULL; i++) {
                if (!test_wanted(svm_tests[i].name, av, ac))
                        continue;
-               if (svm_tests[i].supported && !svm_tests[i].supported())
+               if (svm_tests[i].required_feature &&
+                   !this_cpu_has(svm_tests[i].required_feature))
+                       continue;
+               if (svm_tests[i].smp_required && !smp_supported)
                        continue;
                if (svm_tests[i].v2 == NULL) {
                        if (svm_tests[i].on_vcpu) {
diff --git a/x86/svm.h b/x86/svm.h
index 0c40a086..632287ca 100644
--- a/x86/svm.h
+++ b/x86/svm.h
@@ -9,7 +9,8 @@
 
 struct svm_test {
        const char *name;
-       bool (*supported)(void);
+       u64 required_feature;
+       bool smp_required;
        void (*prepare)(struct svm_test *test);
        void (*prepare_gif_clear)(struct svm_test *test);
        void (*guest_func)(struct svm_test *test);