]> xenbits.xensource.com Git - people/pauldu/linux.git/commitdiff
KVM: x86: Use a switch statement and macros in __feature_translate()
authorJim Mattson <jmattson@google.com>
Tue, 24 Oct 2023 00:16:36 +0000 (17:16 -0700)
committerSean Christopherson <seanjc@google.com>
Thu, 30 Nov 2023 20:27:02 +0000 (12:27 -0800)
Use a switch statement with macro-generated case statements to handle
translating feature flags in order to reduce the probability of runtime
errors due to copy+paste goofs, to make compile-time errors easier to
debug, and to make the code more readable.

E.g. the compiler won't directly generate an error for duplicate if
statements

if (x86_feature == X86_FEATURE_SGX1)
return KVM_X86_FEATURE_SGX1;
else if (x86_feature == X86_FEATURE_SGX2)
return KVM_X86_FEATURE_SGX1;

and so instead reverse_cpuid_check() will fail due to the untranslated
entry pointing at a Linux-defined leaf, which provides practically no
hint as to what is broken

  arch/x86/kvm/reverse_cpuid.h:108:2: error: call to __compiletime_assert_450 declared with 'error' attribute:
                                      BUILD_BUG_ON failed: x86_leaf == CPUID_LNX_4
          BUILD_BUG_ON(x86_leaf == CPUID_LNX_4);
          ^
whereas duplicate case statements very explicitly point at the offending
code:

  arch/x86/kvm/reverse_cpuid.h:125:2: error: duplicate case value '361'
          KVM_X86_TRANSLATE_FEATURE(SGX2);
          ^
  arch/x86/kvm/reverse_cpuid.h:124:2: error: duplicate case value '360'
          KVM_X86_TRANSLATE_FEATURE(SGX1);
          ^

And without macros, the opposite type of copy+paste goof doesn't generate
any error at compile-time, e.g. this yields no complaints:

        case X86_FEATURE_SGX1:
                return KVM_X86_FEATURE_SGX1;
        case X86_FEATURE_SGX2:
                return KVM_X86_FEATURE_SGX1;

Note, __feature_translate() is forcibly inlined and the feature is known
at compile-time, so the code generation between an if-elif sequence and a
switch statement should be identical.

Signed-off-by: Jim Mattson <jmattson@google.com>
Link: https://lore.kernel.org/r/20231024001636.890236-2-jmattson@google.com
[sean: use a macro, rewrite changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
arch/x86/kvm/reverse_cpuid.h

index 17007016d8b5e6df85cef3838d2c10ad8c08856c..aadefcaa9561d0a31e589784da7e871e4a0de2e0 100644 (file)
@@ -116,20 +116,19 @@ static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
  */
 static __always_inline u32 __feature_translate(int x86_feature)
 {
-       if (x86_feature == X86_FEATURE_SGX1)
-               return KVM_X86_FEATURE_SGX1;
-       else if (x86_feature == X86_FEATURE_SGX2)
-               return KVM_X86_FEATURE_SGX2;
-       else if (x86_feature == X86_FEATURE_SGX_EDECCSSA)
-               return KVM_X86_FEATURE_SGX_EDECCSSA;
-       else if (x86_feature == X86_FEATURE_CONSTANT_TSC)
-               return KVM_X86_FEATURE_CONSTANT_TSC;
-       else if (x86_feature == X86_FEATURE_PERFMON_V2)
-               return KVM_X86_FEATURE_PERFMON_V2;
-       else if (x86_feature == X86_FEATURE_RRSBA_CTRL)
-               return KVM_X86_FEATURE_RRSBA_CTRL;
-
-       return x86_feature;
+#define KVM_X86_TRANSLATE_FEATURE(f)   \
+       case X86_FEATURE_##f: return KVM_X86_FEATURE_##f
+
+       switch (x86_feature) {
+       KVM_X86_TRANSLATE_FEATURE(SGX1);
+       KVM_X86_TRANSLATE_FEATURE(SGX2);
+       KVM_X86_TRANSLATE_FEATURE(SGX_EDECCSSA);
+       KVM_X86_TRANSLATE_FEATURE(CONSTANT_TSC);
+       KVM_X86_TRANSLATE_FEATURE(PERFMON_V2);
+       KVM_X86_TRANSLATE_FEATURE(RRSBA_CTRL);
+       default:
+               return x86_feature;
+       }
 }
 
 static __always_inline u32 __feature_leaf(int x86_feature)