On Tue, Mar 24, 2015 at 3:20 PM, Dave Hansen <dave.han...@intel.com> wrote: > From: Dave Hansen <dave.han...@linux.intel.com> > > The MPX code appears to be saving off the FPU in an unsafe > way. It does not disable preemption or ensure that the > FPU state has been allocated. > > This patch introduces a new helper which will do both of > those things internally to a helper. > > Signed-off-by: Dave Hansen <dave.han...@linux.intel.com> > Cc: Rik van Riel <r...@redhat.com> > Cc: Suresh Siddha <sbsid...@gmail.com> > Cc: Andy Lutomirski <l...@amacapital.net> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Ingo Molnar <mi...@redhat.com> > Cc: "H. Peter Anvin" <h...@zytor.com> > Cc: Fenghua Yu <fenghua...@intel.com> > Cc: the arch/x86 maintainers <x...@kernel.org> > --- > arch/x86/include/asm/xsave.h | 1 + > arch/x86/kernel/xsave.c | 32 ++++++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h > index c9a6d68..86b58fb 100644 > --- a/arch/x86/include/asm/xsave.h > +++ b/arch/x86/include/asm/xsave.h > @@ -252,6 +252,7 @@ static inline int xrestore_user(struct xsave_struct > __user *buf, u64 mask) > } > > void *get_xsave_addr(struct xsave_struct *xsave, int xstate); > +void *tsk_get_xsave_field(struct task_struct *tsk, int xstate_field); > void setup_xstate_comp(void); > > #endif > diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c > index 34f66e5..9919e7e 100644 > --- a/arch/x86/kernel/xsave.c > +++ b/arch/x86/kernel/xsave.c > @@ -740,3 +740,35 @@ void *get_xsave_addr(struct xsave_struct *xsave, int > xstate) > return (void *)xsave + xstate_comp_offsets[feature]; > } > EXPORT_SYMBOL_GPL(get_xsave_addr); > + > +/* > + * This wraps up the common operations that need to occur when retrieving > + * data from an xsave struct. It first ensures that the task was actually > + * using the FPU and retrieves the data in to a buffer. It then calculates > + * the offset of the requested field in the buffer. > + * > + * This function is safe to call whether the FPU is in use or not. > + * > + * Inputs: > + * tsk: the task from which we are fetching xsave state > + * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE, > + * etc.) > + * Output: > + * address of the state in the xsave area. > + */ > +void *tsk_get_xsave_field(struct task_struct *tsk, int xsave_field) > +{ > + union thread_xstate *xstate; > + > + unlazy_fpu(tsk); > + xstate = tsk->thread.fpu.state; > + /* > + * This might be unallocated if the FPU > + * was never in use. > + */ > + if (!xstate) > + return NULL; > + > + return get_xsave_addr(&xstate->xsave, xsave_field); > +} > +EXPORT_SYMBOL_GPL(tsk_get_xsave_field); > -- > 1.9.1 >
I have one objection to get_xsave_addr and tsk_get_xsave_field: I think that get_xsave_addr should be called either get_xsave_addr_for_read or get_xsave_addr_for_write, depending on which of those it does. Your function appears to be getting it for write (I assume that's what the unlazy_fpu is for), so I'd rather have it called tsk_get_xsave_field_for_write or something like that. --Andy -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/