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.
