Messages in this thread Patch in this message | | | Date | Mon, 8 Feb 2021 13:33:30 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v3 08/21] x86/fpu/xstate: Define the scope of the initial xstate data |
| |
On Wed, Dec 23, 2020 at 07:57:04AM -0800, Chang S. Bae wrote: > init_fpstate is used to record the initial xstate value for convenience
convenience?
> and covers all the states. But it is wasteful to cover large states all > with trivial initial data. > > Limit init_fpstate by clarifying its size and coverage, which are all but > dynamic user states. The dynamic states are assumed to be large but having > initial data with zeros. > > No functional change until the kernel supports dynamic user states.
What does that mean?
This patch either makes no functional change or it does...
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> > Reviewed-by: Len Brown <len.brown@intel.com> > Cc: x86@kernel.org > Cc: linux-kernel@vger.kernel.org > --- > Changes from v2: > * Updated the changelog for clarification. > * Updated the code comments. > --- > arch/x86/include/asm/fpu/internal.h | 18 +++++++++++++++--- > arch/x86/include/asm/fpu/xstate.h | 1 + > arch/x86/kernel/fpu/core.c | 4 ++-- > arch/x86/kernel/fpu/xstate.c | 4 ++-- > 4 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h > index 37ea5e37f21c..bbdd304719c6 100644 > --- a/arch/x86/include/asm/fpu/internal.h > +++ b/arch/x86/include/asm/fpu/internal.h > @@ -80,6 +80,18 @@ static __always_inline __pure bool use_fxsr(void) > > extern union fpregs_state init_fpstate; > > +static inline u64 get_init_fpstate_mask(void) > +{ > + /* init_fpstate covers states in fpu->state. */ > + return (xfeatures_mask_all & ~xfeatures_mask_user_dynamic); > +}
If you're going to introduce such a helper, then use it everywhere in the code:
$ git grep "xfeatures_mask_all & ~xfeatures_mask_user_dynamic" arch/x86/kernel/fpu/core.c:239: dst_fpu->state_mask = xfeatures_mask_all & ~xfeatures_mask_user_dynamic; arch/x86/kernel/fpu/xstate.c:148: else if (mask == (xfeatures_mask_all & ~xfeatures_mask_user_dynamic)) arch/x86/kernel/fpu/xstate.c:932: current->thread.fpu.state_mask = (xfeatures_mask_all & ~xfeatures_mask_user_dynamic);
and if you do that, do that in a separate pre-patch which does only this conversion.
> +static inline unsigned int get_init_fpstate_size(void) > +{ > + /* fpu->state size is aligned with the init_fpstate size. */ > + return fpu_kernel_xstate_min_size; > +} > + > extern void fpstate_init(struct fpu *fpu); > #ifdef CONFIG_MATH_EMULATION > extern void fpstate_init_soft(struct swregs_state *soft); > @@ -269,12 +281,12 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu) > : "memory") > > /* > - * This function is called only during boot time when x86 caps are not set > - * up and alternative can not be used yet. > + * Use this function to dump the initial state, only during boot time when x86 > + * caps not set up and alternative not available yet. > */
What's the point of this change? Also, "dump"?!
> static inline void copy_xregs_to_kernel_booting(struct xregs_state *xstate) > { > - u64 mask = xfeatures_mask_all; > + u64 mask = get_init_fpstate_mask(); > u32 lmask = mask; > u32 hmask = mask >> 32; > int err; > diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h > index 379e8f8b8440..62f6583f34fa 100644 > --- a/arch/x86/include/asm/fpu/xstate.h > +++ b/arch/x86/include/asm/fpu/xstate.h > @@ -103,6 +103,7 @@ extern void __init update_regset_xstate_info(unsigned int size, > u64 xstate_mask); > > void *get_xsave_addr(struct fpu *fpu, int xfeature_nr); > +unsigned int get_xstate_size(u64 mask); > int alloc_xstate_buffer(struct fpu *fpu, u64 mask); > void free_xstate_buffer(struct fpu *fpu); > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index 6dafed34be4f..aad1a7102096 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -206,10 +206,10 @@ void fpstate_init(struct fpu *fpu) > return; > } > > - memset(state, 0, fpu_kernel_xstate_min_size); > + memset(state, 0, fpu ? get_xstate_size(fpu->state_mask) : get_init_fpstate_size()); > > if (static_cpu_has(X86_FEATURE_XSAVES)) > - fpstate_init_xstate(&state->xsave, xfeatures_mask_all); > + fpstate_init_xstate(&state->xsave, fpu ? fpu->state_mask : get_init_fpstate_mask());
<---- newline here.
> if (static_cpu_has(X86_FEATURE_FXSR)) > fpstate_init_fxstate(&state->fxsave); > else
...
-- Regards/Gruss, Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
| |