x86/fpu: Fix FPU state save area alignment bug
authorIngo Molnar <mingo@kernel.org>
Sun, 24 May 2015 07:58:12 +0000 (09:58 +0200)
committerIngo Molnar <mingo@kernel.org>
Mon, 25 May 2015 07:38:04 +0000 (09:38 +0200)
On most configs task-struct is cache line aligned, which makes
the XSAVE area's 64-byte required alignment work out fine.

But on some .config's task_struct is aligned only to 16 bytes
(enforced by ARCH_MIN_TASKALIGN), which makes things like
fpu__copy() (that XSAVEOPT uses) not work so well.

I broke this in:

  7366ed771f6e ("x86/fpu: Simplify FPU handling by embedding the fpstate in task_struct (again)")

which embedded the fpstate in the task_struct.

The alignment requirements of the FPU code were originally present
in ARCH_MIN_TASKALIGN, which still has a value of 16, which was the
alignment requirement of the FPU state area prior XSAVE. But this
link was not documented (and not required) and the link got lost
when the FPU state area was made dynamic years ago.

With XSAVEOPT the minimum alignment requirment went up to 64 bytes,
and the embedding of the FPU state area in task_struct exposed it
again - and '16' was not increased to '64'.

So fix this bug, but also try to address the underlying lost link
of information that made it easier to happen:

  - document ARCH_MIN_TASKALIGN a bit better

  - use alignof() to recover the current alignment requirements.
    This would work in the future as well, should the alignment
    requirements go up to 128 bytes with things like AVX512.

( We should probably also use the vSMP alignment rules for all
  of x86, but that's for another patch. )

Reported-by: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
arch/x86/include/asm/processor.h

index b9e487499ae2a30988d4eac15e9a71f90e6b3631..8e04f51d6bea49f50928ee858360cd833527b2ff 100644 (file)
@@ -53,11 +53,16 @@ static inline void *current_text_addr(void)
        return pc;
 }
 
+/*
+ * These alignment constraints are for performance in the vSMP case,
+ * but in the task_struct case we must also meet hardware imposed
+ * alignment requirements of the FPU state:
+ */
 #ifdef CONFIG_X86_VSMP
 # define ARCH_MIN_TASKALIGN            (1 << INTERNODE_CACHE_SHIFT)
 # define ARCH_MIN_MMSTRUCT_ALIGN       (1 << INTERNODE_CACHE_SHIFT)
 #else
-# define ARCH_MIN_TASKALIGN            16
+# define ARCH_MIN_TASKALIGN            __alignof__(union fpregs_state)
 # define ARCH_MIN_MMSTRUCT_ALIGN       0
 #endif