Skip to content

Commit 80c883d

Browse files
jsmattsonjrsean-jc
authored andcommitted
KVM: x86: Use a switch statement and macros in __feature_translate()
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>
1 parent eefe5e6 commit 80c883d

1 file changed

Lines changed: 13 additions & 14 deletions

File tree

arch/x86/kvm/reverse_cpuid.h

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -116,20 +116,19 @@ static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
116116
*/
117117
static __always_inline u32 __feature_translate(int x86_feature)
118118
{
119-
if (x86_feature == X86_FEATURE_SGX1)
120-
return KVM_X86_FEATURE_SGX1;
121-
else if (x86_feature == X86_FEATURE_SGX2)
122-
return KVM_X86_FEATURE_SGX2;
123-
else if (x86_feature == X86_FEATURE_SGX_EDECCSSA)
124-
return KVM_X86_FEATURE_SGX_EDECCSSA;
125-
else if (x86_feature == X86_FEATURE_CONSTANT_TSC)
126-
return KVM_X86_FEATURE_CONSTANT_TSC;
127-
else if (x86_feature == X86_FEATURE_PERFMON_V2)
128-
return KVM_X86_FEATURE_PERFMON_V2;
129-
else if (x86_feature == X86_FEATURE_RRSBA_CTRL)
130-
return KVM_X86_FEATURE_RRSBA_CTRL;
131-
132-
return x86_feature;
119+
#define KVM_X86_TRANSLATE_FEATURE(f) \
120+
case X86_FEATURE_##f: return KVM_X86_FEATURE_##f
121+
122+
switch (x86_feature) {
123+
KVM_X86_TRANSLATE_FEATURE(SGX1);
124+
KVM_X86_TRANSLATE_FEATURE(SGX2);
125+
KVM_X86_TRANSLATE_FEATURE(SGX_EDECCSSA);
126+
KVM_X86_TRANSLATE_FEATURE(CONSTANT_TSC);
127+
KVM_X86_TRANSLATE_FEATURE(PERFMON_V2);
128+
KVM_X86_TRANSLATE_FEATURE(RRSBA_CTRL);
129+
default:
130+
return x86_feature;
131+
}
133132
}
134133

135134
static __always_inline u32 __feature_leaf(int x86_feature)

0 commit comments

Comments
 (0)