Skip to content

Commit f9dfb5e

Browse files
KAGA-KOKOsuryasaimadhu
authored andcommitted
x86/fpu: Make init_fpstate correct with optimized XSAVE
The XSAVE init code initializes all enabled and supported components with XRSTOR(S) to init state. Then it XSAVEs the state of the components back into init_fpstate which is used in several places to fill in the init state of components. This works correctly with XSAVE, but not with XSAVEOPT and XSAVES because those use the init optimization and skip writing state of components which are in init state. So init_fpstate.xsave still contains all zeroes after this operation. There are two ways to solve that: 1) Use XSAVE unconditionally, but that requires to reshuffle the buffer when XSAVES is enabled because XSAVES uses compacted format. 2) Save the components which are known to have a non-zero init state by other means. Looking deeper, #2 is the right thing to do because all components the kernel supports have all-zeroes init state except the legacy features (FP, SSE). Those cannot be hard coded because the states are not identical on all CPUs, but they can be saved with FXSAVE which avoids all conditionals. Use FXSAVE to save the legacy FP/SSE components in init_fpstate along with a BUILD_BUG_ON() which reminds developers to validate that a newly added component has all zeroes init state. As a bonus remove the now unused copy_xregs_to_kernel_booting() crutch. The XSAVE and reshuffle method can still be implemented in the unlikely case that components are added which have a non-zero init state and no other means to save them. For now, FXSAVE is just simple and good enough. [ bp: Fix a typo or two in the text. ] Fixes: 6bad06b ("x86, xsave: Use xsaveopt in context-switch path when supported") Signed-off-by: Thomas Gleixner <[email protected]> Signed-off-by: Borislav Petkov <[email protected]> Reviewed-by: Borislav Petkov <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected]
1 parent 9301982 commit f9dfb5e

File tree

2 files changed

+46
-25
lines changed

2 files changed

+46
-25
lines changed

arch/x86/include/asm/fpu/internal.h

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,14 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu)
204204
asm volatile("fxsaveq %[fx]" : [fx] "=m" (fpu->state.fxsave));
205205
}
206206

207+
static inline void fxsave(struct fxregs_state *fx)
208+
{
209+
if (IS_ENABLED(CONFIG_X86_32))
210+
asm volatile( "fxsave %[fx]" : [fx] "=m" (*fx));
211+
else
212+
asm volatile("fxsaveq %[fx]" : [fx] "=m" (*fx));
213+
}
214+
207215
/* These macros all use (%edi)/(%rdi) as the single memory argument. */
208216
#define XSAVE ".byte " REX_PREFIX "0x0f,0xae,0x27"
209217
#define XSAVEOPT ".byte " REX_PREFIX "0x0f,0xae,0x37"
@@ -268,28 +276,6 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu)
268276
: "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
269277
: "memory")
270278

271-
/*
272-
* This function is called only during boot time when x86 caps are not set
273-
* up and alternative can not be used yet.
274-
*/
275-
static inline void copy_xregs_to_kernel_booting(struct xregs_state *xstate)
276-
{
277-
u64 mask = xfeatures_mask_all;
278-
u32 lmask = mask;
279-
u32 hmask = mask >> 32;
280-
int err;
281-
282-
WARN_ON(system_state != SYSTEM_BOOTING);
283-
284-
if (boot_cpu_has(X86_FEATURE_XSAVES))
285-
XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
286-
else
287-
XSTATE_OP(XSAVE, xstate, lmask, hmask, err);
288-
289-
/* We should never fault when copying to a kernel buffer: */
290-
WARN_ON_FPU(err);
291-
}
292-
293279
/*
294280
* This function is called only during boot time when x86 caps are not set
295281
* up and alternative can not be used yet.

arch/x86/kernel/fpu/xstate.c

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,13 +440,36 @@ static void __init print_xstate_offset_size(void)
440440
}
441441
}
442442

443+
/*
444+
* All supported features have either init state all zeros or are
445+
* handled in setup_init_fpu() individually. This is an explicit
446+
* feature list and does not use XFEATURE_MASK*SUPPORTED to catch
447+
* newly added supported features at build time and make people
448+
* actually look at the init state for the new feature.
449+
*/
450+
#define XFEATURES_INIT_FPSTATE_HANDLED \
451+
(XFEATURE_MASK_FP | \
452+
XFEATURE_MASK_SSE | \
453+
XFEATURE_MASK_YMM | \
454+
XFEATURE_MASK_OPMASK | \
455+
XFEATURE_MASK_ZMM_Hi256 | \
456+
XFEATURE_MASK_Hi16_ZMM | \
457+
XFEATURE_MASK_PKRU | \
458+
XFEATURE_MASK_BNDREGS | \
459+
XFEATURE_MASK_BNDCSR | \
460+
XFEATURE_MASK_PASID)
461+
443462
/*
444463
* setup the xstate image representing the init state
445464
*/
446465
static void __init setup_init_fpu_buf(void)
447466
{
448467
static int on_boot_cpu __initdata = 1;
449468

469+
BUILD_BUG_ON((XFEATURE_MASK_USER_SUPPORTED |
470+
XFEATURE_MASK_SUPERVISOR_SUPPORTED) !=
471+
XFEATURES_INIT_FPSTATE_HANDLED);
472+
450473
WARN_ON_FPU(!on_boot_cpu);
451474
on_boot_cpu = 0;
452475

@@ -466,10 +489,22 @@ static void __init setup_init_fpu_buf(void)
466489
copy_kernel_to_xregs_booting(&init_fpstate.xsave);
467490

468491
/*
469-
* Dump the init state again. This is to identify the init state
470-
* of any feature which is not represented by all zero's.
492+
* All components are now in init state. Read the state back so
493+
* that init_fpstate contains all non-zero init state. This only
494+
* works with XSAVE, but not with XSAVEOPT and XSAVES because
495+
* those use the init optimization which skips writing data for
496+
* components in init state.
497+
*
498+
* XSAVE could be used, but that would require to reshuffle the
499+
* data when XSAVES is available because XSAVES uses xstate
500+
* compaction. But doing so is a pointless exercise because most
501+
* components have an all zeros init state except for the legacy
502+
* ones (FP and SSE). Those can be saved with FXSAVE into the
503+
* legacy area. Adding new features requires to ensure that init
504+
* state is all zeroes or if not to add the necessary handling
505+
* here.
471506
*/
472-
copy_xregs_to_kernel_booting(&init_fpstate.xsave);
507+
fxsave(&init_fpstate.fxsave);
473508
}
474509

475510
static int xfeature_uncompacted_offset(int xfeature_nr)

0 commit comments

Comments
 (0)