On Mon, Aug 05, 2024, mlevitsk@xxxxxxxxxx wrote:
> У пт, 2024-07-26 у 17:06 -0700, Sean Christopherson пише:
> > > > > And kvm_cpu_cap_init_begin, can set some cap_in_progress variable.
> > >
> > > Ya, but then compile-time asserts become run-time asserts.
>
> Not really, it all can be done with macros, in exactly the same way IMHO,
> we do have BUILD_BUG_ON after all.
>
> I am not against using macros, I am only against collecting a bitmask
> while applying various side effects, and then passing the bitmask to
> the kvm_cpu_cap_init.
Gah, I wasn't grokking that, obviously. Sorry for not catching on earlier.
> > > To provide equivalent functionality, we also would need to pass in extra
> > > state to begin/end() (as mentioned earlier).
>
> Besides the number of leaf currently initialized, I don't see which other
> extra state we need.
>
> In fact I can prove that this is possible:
>
> Roughly like this:
>
> #define kvm_cpu_cap_init_begin(leaf) \
> do { \
> const u32 __maybe_unused kvm_cpu_cap_init_in_progress = leaf; \
> u32 kvm_cpu_cap_emulated = 0; \
> u32 kvm_cpu_cap_synthesized = 0; \
> u32 kvm_cpu_cap_regular = 0;
Maybe "virtualized" instead of "regular"?
> #define feature_scattered(name) \
> BUILD_BUG_ON(X86_FEATURE_##name >= MAX_CPU_FEATURES); \
> KVM_VALIDATE_CPU_CAP_USAGE(name); \
> \
> if (boot_cpu_has(X86_FEATURE_##name) \
> kvm_cpu_cap_regular |= feature_bit(name);
>
>
> #define kvm_cpu_cap_init_end() \
> const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32); \
> \
> if (kvm_cpu_cap_init_in_progress < NCAPINTS) \
> kvm_cpu_caps[kvm_cpu_cap_init_in_progress] &= kvm_cpu_cap_regular; \
> else \
> kvm_cpu_caps[kvm_cpu_cap_init_in_progress] = kvm_cpu_cap_regular; \
> \
> kvm_cpu_caps[kvm_cpu_cap_init_in_progress] &= (raw_cpuid_get(cpuid) | \
> kvm_cpu_cap_synthesized); \
> kvm_cpu_caps[kvm_cpu_cap_init_in_progress] |= kvm_cpu_cap_emulated; \
> } while(0);
>
>
> And now we have:
>
> kvm_cpu_cap_init_begin(CPUID_12_EAX);
> feature_scattered(SGX1);
> feature_scattered(SGX2);
> feature_scattered(SGX_EDECCSSA);
> kvm_cpu_cap_init_end();
I don't love the syntax (mainly the need for a begin()+end()), but I'm a-ok
getting rid of the @mask param/input.
What about making kvm_cpu_cap_init() a variadic macro, with the relevant features
"unpacked" in the context of the macro? That would avoid the need for a trailing
macro, and would provide a clear indication of when/where the set of features is
"initialized".
The biggest downside I see is that the last entry can't have a trailing comma,
i.e. adding a new feature would require updating the previous feature too.
#define kvm_cpu_cap_init(leaf, init_features...) \
do { \
const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32); \
const u32 __maybe_unused kvm_cpu_cap_init_in_progress = leaf; \
u32 kvm_cpu_cap_virtualized= 0; \
u32 kvm_cpu_cap_emulated = 0; \
u32 kvm_cpu_cap_synthesized = 0; \
\
init_features; \
\
kvm_cpu_caps[leaf] = kvm_cpu_cap_virtualized; \
kvm_cpu_caps[leaf] &= (raw_cpuid_get(cpuid) | \
kvm_cpu_cap_synthesized); \
kvm_cpu_caps[leaf] |= kvm_cpu_cap_emulated; \
} while (0)
kvm_cpu_cap_init(CPUID_1_ECX,
VIRTUALIZED_F(XMM3),
VIRTUALIZED_F(PCLMULQDQ),
VIRTUALIZED_F(SSSE3),
VIRTUALIZED_F(FMA),
VIRTUALIZED_F(CX16),
VIRTUALIZED_F(PDCM),
VIRTUALIZED_F(PCID),
VIRTUALIZED_F(XMM4_1),
VIRTUALIZED_F(XMM4_2),
EMULATED_F(X2APIC),
VIRTUALIZED_F(MOVBE),
VIRTUALIZED_F(POPCNT),
EMULATED_F(TSC_DEADLINE_TIMER),
VIRTUALIZED_F(AES),
VIRTUALIZED_F(XSAVE),
// DYNAMIC_F(OSXSAVE),
VIRTUALIZED_F(AVX),
VIRTUALIZED_F(F16C),
VIRTUALIZED_F(RDRAND),
EMULATED_F(HYPERVISOR)
);
Alternatively, we could force a trailing comma by omitting the semicolon after
init_features, but that looks weird for the the macro itself, and arguably a bit
weird for the users too.
