* Dave Hansen <dave.han...@linux.intel.com> wrote:

> + * The MPX tables change sizes based on the size of the virtual
> + * (aka. linear) address space.  There is an MSR to tell the CPU
> + * whether we want the legacy-style ones or the larger ones when
> + * we are running with an eXtended virtual address space.
> + */
> +static void switch_mawa(struct mm_struct *prev, struct mm_struct *next)
> +{
> +     /*
> +      * Note: there is one and only one bit in use in the MSR
> +      * at this time, so we do not have to be concerned with
> +      * preseving any of the other bits.  Just write 0 or 1.
> +      */
> +     unsigned IA32_MPX_LAX_ENABLE_MASK = 0x00000001;
> +
> +     if (!cpu_feature_enabled(X86_FEATURE_MPX))
> +             return;
> +     /*
> +      * FIXME: do we want a check here for the 5-level paging
> +      * CR4 bit or CPUID bit, or is the mawa check below OK?
> +      * It's not obvious what would be the fastest or if it
> +      * matters.
> +      */
> +
> +     /*
> +      * Avoid the relatively costly MSR if we are not changing
> +      * MAWA state.  All processes not using MPX will have a
> +      * mpx_mawa_shift()=0, so we do not need to check
> +      * separately for whether MPX management is enabled.
> +      */
> +     if (mpx_mawa_shift(prev) == mpx_mawa_shift(next))
> +             return;

Please stop the senseless looking wrappery - if the field is name sensibly then 
it 
can be accessed directly through mm_struct.

> +
> +     if (mpx_mawa_shift(next)) {
> +             wrmsr(MSR_IA32_MPX_LAX, IA32_MPX_LAX_ENABLE_MASK, 0x0);
> +     } else {
> +             /* clear the enable bit: */
> +             wrmsr(MSR_IA32_MPX_LAX, 0x0, 0x0);
> +     }
> +}
> +
>  void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>                       struct task_struct *tsk)
>  {
> @@ -136,6 +177,7 @@ void switch_mm_irqs_off(struct mm_struct
>               /* Load per-mm CR4 state */
>               load_mm_cr4(next);
>  
> +             switch_mawa(prev, next);

This implementation adds about 4-5 unnecessary instructions to the context 
switching hot path of every non-MPX task, even on non-MPX hardware.

Please make sure that this is something like:

        if (unlikely(prev->mpx_msr_val != next->mpx_msr_val))
                switch_mpx(prev, next);

... which reduces the hot path overhead to something like 2 instruction (if we 
are 
lucky).

This can be put into switch_mpx() and can be inlined - just make sure that on a 
defconfig the generated machine code is sane.

Thanks,

        Ingo

Reply via email to