On Wed, Jun 18, 2014 at 09:25:57AM -0700, Dave Hansen wrote:
> On 06/18/2014 07:59 AM, H. Peter Anvin wrote:
> > On 06/18/2014 07:35 AM, Dave Hansen wrote:
> >> It looks like static_cpu_has() is the right thing to use instead of
> >> boot_cpu_has(). But, this doesn't just obfuscate things.
> >>
> >> We actually _want_ the compiler to cull code out when the config option
> >> is off. Things like do_bounds() will see code savings with _some_ kind
> >> of #ifdef rather than using static_cpu_has().
> >>
> >> So, we can either use the well worn, consistent with other features in
> >> x86, cpu_has_$foo approach. Or, we can roll our own macros.
> >
> > We could do something like:
> >
> > #define MPX_ENABLED (IS_ENABLED(CONFIG_X86_MPX) &&
> > static_cpu_has(X86_FEATURE_MPX))
>
> How about something like the attached patch?
>
> This lets us use static_cpu_has() for the checks, and allows us to
> easily add new checks for other features that might be compile-time
> disabled.
>
>
> ---
>
> b/arch/x86/include/asm/cpufeature.h | 26 ++++++++++++++++++++------
> b/arch/x86/kernel/mpx.c | 4 ++--
> b/arch/x86/kernel/traps.c | 2 +-
> 3 files changed, 23 insertions(+), 9 deletions(-)
>
> diff -puN arch/x86/include/asm/cpufeature.h~x86-disabled_mask
> arch/x86/include/asm/cpufeature.h
> --- a/arch/x86/include/asm/cpufeature.h~x86-disabled_mask 2014-06-18
> 08:48:41.329750895 -0700
> +++ b/arch/x86/include/asm/cpufeature.h 2014-06-18 09:19:19.143546973
> -0700
> @@ -339,12 +339,6 @@ extern const char * const x86_power_flag
> #define cpu_has_eager_fpu boot_cpu_has(X86_FEATURE_EAGER_FPU)
> #define cpu_has_topoext boot_cpu_has(X86_FEATURE_TOPOEXT)
>
> -#ifdef CONFIG_X86_INTEL_MPX
> -#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX)
> -#else
> -#define cpu_has_mpx 0
> -#endif /* CONFIG_X86_INTEL_MPX */
> -
> #ifdef CONFIG_X86_64
>
> #undef cpu_has_vme
> @@ -367,6 +361,22 @@ extern const char * const x86_power_flag
>
> #endif /* CONFIG_X86_64 */
>
> +/*
> + * Add features and their corresponding config options here
> + * if you want to have the compiler optimize out code that
> + * uses them.
> + *
> + * You should not use this function directly. Use
> + * static_cpu_has() so that you also benefit from alternatives
> + * when the features are enabled.
> + */
> +static __always_inline int __cpu_feature_compile_enabled(u16 bit)
> +{
> + if (bit == X86_FEATURE_MPX)
> + return IS_ENABLED(CONFIG_X86_MPX);
Right, this should be CONFIG_X86_INTEL_MPX but that's details.
I guess this might grow into a big if-else noodle case but when that
happens, we probably could add some sort of mapping between X86_FEATURE
bits to CONFIG_ items or so.
Then, you could probably add this call to the enclosing macros
static_cpu_has{,_safe} so that it gets evaluated first. The advantage is
that the IS_ENABLED test would work very early - always, actually - even
before alternatives have run and thus catch the cases where the feature
is config-disabled.
Otherwise, asm looks ok.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
