On Wed, 2023-08-16 at 07:20 -0700, Sean Christopherson wrote: > On Wed, Aug 16, 2023, Kai Huang wrote: > > > diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h > > > new file mode 100644 > > > index 000000000000..40ce8e6608cd > > > --- /dev/null > > > +++ b/arch/x86/kvm/governed_features.h > > > @@ -0,0 +1,9 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#if !defined(KVM_GOVERNED_FEATURE) || defined(KVM_GOVERNED_X86_FEATURE) > > > +BUILD_BUG() > > > +#endif > > > + > > > +#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x) > > > + > > > +#undef KVM_GOVERNED_X86_FEATURE > > > +#undef KVM_GOVERNED_FEATURE > > > > Nit: > > > > Do you want to move the very last > > > > #undef KVM_GOVERNED_FEATURE > > > > out of "governed_features.h", but to the place(s) where the macro is defined? > > > > Yes there will be multiple: > > > > #define KVM_GOVERNED_FEATURE(x) ... > > #include "governed_features.h" > > #undef KVM_GOVERNED_FEATURE > > > > But this looks clearer to me. > > I agree the symmetry looks better, but doing the #undef in governed_features.h > is much more robust. E.g. having the #undef in the header makes it all but impossible > to have a bug where we forget to #undef KVM_GOVERNED_FEATURE. Or worse, have two > bugs where we forget to #undef and then also forget to #define in a later include > and consume the stale #define. Fair enough. > > And I also want to follow the pattern used by kvm-x86-ops.h. Right. I forgot to check this. Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>
