On Tue, Sep 10, 2024, Maxim Levitsky wrote: > On Mon, 2024-08-05 at 15:00 -0700, Sean Christopherson wrote: > > If we go with ALIASED_F() (or ALIASED_8000_0001_F()), then that macro is all that > > is needed, and it's bulletproof. E.g. there is no KVM_X86_FEATURE_FPU_ALIAS that > > can be queried, and thus no need to be ensure it's defined in cpuid.c and #undef'd > > after its use. > > > > Hmm, I supposed we could harden the aliased feature usage in the same way as the > > ALIASED_F(), e.g. > > > > #define __X86_FEATURE_8000_0001_ALIAS(feature) \ > > ({ \ > > BUILD_BUG_ON(__feature_leaf(X86_FEATURE_##name) != CPUID_1_EDX); \ > > BUILD_BUG_ON(kvm_cpu_cap_init_in_progress != CPUID_8000_0001_EDX); \ > > (feature + (CPUID_8000_0001_EDX - CPUID_1_EDX) * 32); \ > > }) > > > > If something tries to use an X86_FEATURE_*_ALIAS outside if kvm_cpu_cap_init(), > > it would need to define and set kvm_cpu_cap_init_in_progress, i.e. would really > > have to try to mess up. > > > > Effectively the only differences are that KVM would have ~10 or so more lines of > > code to define the X86_FEATURE_*_ALIAS macros, and that the usage would look like: > > > > VIRTUALIZED_F(FPU_ALIAS) > > > > versus > > > > ALIASED_F(FPU) > > > This is exactly my point. I want to avoid profiliation of the _F macros, because > later, we will need to figure out what each of them (e.g ALIASED_F) does. > > A whole leaf alias, is once in x86 arch life misfeature, and it is very likely that > Intel/AMD won't add more such aliases. > > Why VIRTUALIZED_F though, it wasn't in the patch series? Normal F() should be enough > IMHO. I'm a-ok with F(), I simply thought there was a desire for more verbosity across the board. > > At that point, I'm ok with defining each alias, though I honestly still don't > > understand the motivation for defining single-use macros. > > > > The idea is that nobody will need to look at these macros > (e.g__X86_FEATURE_8000_0001_ALIAS() and its usages), because it's clear what > they do, they just define few extra CPUID features that nobody really cares > about. > > ALIASED_F() on the other hand is yet another _F macro() and we will need, > once again and again to figure out why it is there, what it does, etc. That seems easily solved by naming the macro ALIASED_8000_0001_F(). I don't see how that's any less clear than __X86_FEATURE_8000_0001_ALIAS(), and as above, there are several advantages to defining the alias in the context of the leaf builder.