Re: [PATCH v6 03/10] x86, mpx: add macro cpu_has_mpx

[Dave Hansen <dave.hansen@intel.com>]

On 06/18/2014 07:44 AM, Borislav Petkov wrote:
> On Wed, Jun 18, 2014 at 07:35:17AM -0700, Dave Hansen wrote:
>> On 06/18/2014 02:57 AM, Borislav Petkov wrote:
>>>>> @@ -339,6 +339,12 @@ extern const char * const x86_power_flags[32];
>>>>>  #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)
>>> I think we don't want those macros anymore because they're obfuscating
>>> the code. You should use static_cpu_has instead.
>>
>> 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().
> 
> Why?

Are you seriously asking why we would want to cull out code guaranteed
to be unused?

People are going to want to turn this off at compile time.  When they
do, I want as much of the code to go away as is reasonably possible.
Adding a single-line #ifdef in a header qualifies as a pretty decent
tradeoff in my book.

I'm sure Qiaowei will do an experiment and show us what the code savings
are.

> Practically, distros will have it enabled anyway (you have X86_INTEL_MPX
> depend on CPU_SUP_INTEL).
> 
> Are you talking about the miniscule percentage of people building their
> own kernels?

The minuscule number of people not using a distro kernel?  Like, every
Android and Chrome device in the world?  How about the cloud providers
with millions of servers?