On Mon, May 09, 2016 at 01:45:59PM -0700, Yu-cheng Yu wrote: > User space uses standard format xsave area. fpstate in signal frame should > have standard format size. > > To explicitly distinguish between xstate size in kernel space and the one > in user space, we rename xstate_size to kernel_xstate_size. This patch is
Let's call it xstate_kernel_size or fpu_xstate_kernel_size and thus keep all the FPU-related variables and functions in the same namespace. > not fixing a bug. It just makes kernel code more clear. > > So we define the xsave area sizes in two global variables: > > kernel_xstate_size (previous xstate_size): the xsave area size used in > xsave area allocated in kernel > user_xstate_size: the xsave area size used in xsave area used by user. > > In no "xsaves" case, xsave area in both user space and kernel space are in > standard format. Therefore, kernel_xstate_size and user_xstate_size are > equal. > > In "xsaves" case, xsave area in user space is in standard format while > xsave area in kernel space is in compact format. Therefore, kernel's > xstate size is less than user's xstate size. Those last two paragraphs look like a good candidates for comments above that new variable. > > Signed-off-by: Fenghua Yu <[email protected]> > Signed-off-by: Yu-cheng Yu <[email protected]> This SOB chain needs fixing too. > Reviewed-by: Dave Hansen <[email protected]> > --- > arch/x86/include/asm/processor.h | 2 +- > arch/x86/kernel/fpu/core.c | 6 +++--- > arch/x86/kernel/fpu/init.c | 18 +++++++++--------- > arch/x86/kernel/fpu/signal.c | 2 +- > arch/x86/kernel/fpu/xstate.c | 8 ++++---- > 5 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 132b4ca..db7f0f9 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -367,7 +367,7 @@ DECLARE_PER_CPU(struct irq_stack *, hardirq_stack); > DECLARE_PER_CPU(struct irq_stack *, softirq_stack); > #endif /* X86_64 */ > > -extern unsigned int xstate_size; > +extern unsigned int kernel_xstate_size; > extern unsigned int user_xstate_size; > > struct perf_event; > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index 8e37cc8..41ab106 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -222,7 +222,7 @@ void fpstate_init(union fpregs_state *state) > return; > } > > - memset(state, 0, xstate_size); > + memset(state, 0, kernel_xstate_size); > > if (cpu_has_fxsr) > fpstate_init_fxstate(&state->fxsave); > @@ -247,7 +247,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu) > * leak into the child task: > */ > if (use_eager_fpu()) > - memset(&dst_fpu->state.xsave, 0, xstate_size); > + memset(&dst_fpu->state.xsave, 0, kernel_xstate_size); > > /* > * Save current FPU registers directly into the child > @@ -266,7 +266,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu) > */ > preempt_disable(); > if (!copy_fpregs_to_fpstate(dst_fpu)) { > - memcpy(&src_fpu->state, &dst_fpu->state, xstate_size); > + memcpy(&src_fpu->state, &dst_fpu->state, kernel_xstate_size); > > if (use_eager_fpu()) > copy_kernel_to_fpregs(&src_fpu->state); > diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c > index 7ea80c2..549ff59 100644 > --- a/arch/x86/kernel/fpu/init.c > +++ b/arch/x86/kernel/fpu/init.c > @@ -145,8 +145,8 @@ static void __init fpu__init_system_generic(void) > * This is inherent to the XSAVE architecture which puts all state > * components into a single, continuous memory block: > */ > -unsigned int xstate_size; > -EXPORT_SYMBOL_GPL(xstate_size); /* * Needs a comment here explaining what it is exactly. */ > +unsigned int kernel_xstate_size; > +EXPORT_SYMBOL_GPL(kernel_xstate_size); > > /* Get alignment of the TYPE. */ > #define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test) > @@ -178,7 +178,7 @@ static void __init fpu__init_task_struct_size(void) > * Add back the dynamically-calculated register state > * size. > */ > - task_size += xstate_size; > + task_size += kernel_xstate_size; > > /* > * We dynamically size 'struct fpu', so we require that > @@ -195,7 +195,7 @@ static void __init fpu__init_task_struct_size(void) > } > > /* > - * Set up the user and kernel xstate_size based on the legacy FPU context > size. > + * Set up the user and kernel xstate sizes based on the legacy FPU context > size. > * > * We set this up first, and later it will be overwritten by > * fpu__init_system_xstate() if the CPU knows about xstates. > @@ -208,7 +208,7 @@ static void __init > fpu__init_system_xstate_size_legacy(void) > on_boot_cpu = 0; > > /* > - * Note that xstate_size might be overwriten later during > + * Note that xstate sizes might be overwriten later during s/overwriten/overwritten/ > * fpu__init_system_xstate(). > */ ... > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index d8aa7d2..20c6631 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -532,7 +532,7 @@ static void do_extra_xstate_size_checks(void) > */ > paranoid_xstate_size += xfeature_size(i); > } > - XSTATE_WARN_ON(paranoid_xstate_size != xstate_size); > + XSTATE_WARN_ON(paranoid_xstate_size != kernel_xstate_size); > } > > > @@ -611,7 +611,7 @@ static int init_xstate_size(void) > * The size is OK, we are definitely going to use xsave, > * make it known to the world that we need more space. > */ > - xstate_size = possible_xstate_size; > + kernel_xstate_size = possible_xstate_size; > do_extra_xstate_size_checks(); > > /* > @@ -674,14 +674,14 @@ void __init fpu__init_system_xstate(void) > return; > } > > - update_regset_xstate_info(xstate_size, xfeatures_mask); > + update_regset_xstate_info(kernel_xstate_size, xfeatures_mask); > fpu__init_prepare_fx_sw_frame(); > setup_init_fpu_buf(); > setup_xstate_comp(); > > pr_info("x86/fpu: Enabled xstate features 0x%llx, context size is %d > bytes, using '%s' format.\n", > xfeatures_mask, > - xstate_size, > + kernel_xstate_size, > cpu_has_xsaves ? "compacted" : "standard"); I think we should dump user_xstate_size in the compacted case since it is != kernel_xstate_size. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
