| Message ID | 20220512074321.2090073-2-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: add get_reg_val helper | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/apply | fail | Patch does not apply to bpf-next |
On Thu, May 12, 2022 at 12:43:17AM -0700, Dave Marchevsky wrote: > The file's fpregs_state_valid function is useful outside of > arch/x86/kernel/fpu dir. Further commits in this series use > fpregs_state_valid to determine whether a BPF helper should fetch > fpu reg value from xsave'd memory or register. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > arch/x86/{kernel => include/asm}/fpu/context.h | 2 ++ > arch/x86/kernel/fpu/core.c | 2 +- > arch/x86/kernel/fpu/regset.c | 2 +- > arch/x86/kernel/fpu/signal.c | 2 +- > arch/x86/kernel/fpu/xstate.c | 2 +- > 5 files changed, 6 insertions(+), 4 deletions(-) > rename arch/x86/{kernel => include/asm}/fpu/context.h (96%) > > diff --git a/arch/x86/kernel/fpu/context.h b/arch/x86/include/asm/fpu/context.h > similarity index 96% > rename from arch/x86/kernel/fpu/context.h > rename to arch/x86/include/asm/fpu/context.h > index 958accf2ccf0..39dac18cd22c 100644 > --- a/arch/x86/kernel/fpu/context.h > +++ b/arch/x86/include/asm/fpu/context.h > @@ -51,6 +51,8 @@ static inline void fpregs_activate(struct fpu *fpu) > trace_x86_fpu_regs_activated(fpu); > } > > +extern void restore_fpregs_from_fpstate(struct fpstate *fpstate, u64 mask); This signature is already included in arch/x86/include/asm/fpu/signal.h. Should we just include that header from arch/x86/include/asm/fpu/context.h rather than declaring the signature twice? > + > /* Internal helper for switch_fpu_return() and signal frame setup */ > static inline void fpregs_restore_userregs(void) > { > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index c049561f373a..5296112d4273 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -7,6 +7,7 @@ > * Gareth Hughes <gareth@valinux.com>, May 2000 > */ > #include <asm/fpu/api.h> > +#include <asm/fpu/context.h> > #include <asm/fpu/regset.h> > #include <asm/fpu/sched.h> > #include <asm/fpu/signal.h> > @@ -18,7 +19,6 @@ > #include <linux/pkeys.h> > #include <linux/vmalloc.h> > > -#include "context.h" > #include "internal.h" > #include "legacy.h" > #include "xstate.h" > diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c > index 75ffaef8c299..f93336f332e3 100644 > --- a/arch/x86/kernel/fpu/regset.c > +++ b/arch/x86/kernel/fpu/regset.c > @@ -6,10 +6,10 @@ > #include <linux/vmalloc.h> > > #include <asm/fpu/api.h> > +#include <asm/fpu/context.h> > #include <asm/fpu/signal.h> > #include <asm/fpu/regset.h> > > -#include "context.h" > #include "internal.h" > #include "legacy.h" > #include "xstate.h" > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c > index 91d4b6de58ab..f099a56c9a93 100644 > --- a/arch/x86/kernel/fpu/signal.c > +++ b/arch/x86/kernel/fpu/signal.c > @@ -7,6 +7,7 @@ > #include <linux/cpu.h> > #include <linux/pagemap.h> > > +#include <asm/fpu/context.h> > #include <asm/fpu/signal.h> > #include <asm/fpu/regset.h> > #include <asm/fpu/xstate.h> > @@ -15,7 +16,6 @@ > #include <asm/trapnr.h> > #include <asm/trace/fpu.h> > > -#include "context.h" > #include "internal.h" > #include "legacy.h" > #include "xstate.h" > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 39e1c8626ab9..ab5e26075716 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -15,6 +15,7 @@ > #include <linux/vmalloc.h> > > #include <asm/fpu/api.h> > +#include <asm/fpu/context.h> > #include <asm/fpu/regset.h> > #include <asm/fpu/signal.h> > #include <asm/fpu/xcr.h> > @@ -23,7 +24,6 @@ > #include <asm/prctl.h> > #include <asm/elf.h> > > -#include "context.h" > #include "internal.h" > #include "legacy.h" > #include "xstate.h" > -- > 2.30.2 > Looks reasonable otherwise, though it's unclear to me (as I'm not an expert in this code) whether we should or shouldn't export all of these functions if some of them are specific to the logic in arch/x86/kernel/fpu. It looks like there's already a precedent for doing that with e.g. fpu__clear_user_states(), so it seems fine. Thanks, David
On Thu, May 12, 2022 at 12:43:17AM -0700, Dave Marchevsky wrote: > The file's fpregs_state_valid function is useful outside of > arch/x86/kernel/fpu dir. Further commits in this series use > fpregs_state_valid to determine whether a BPF helper should fetch > fpu reg value from xsave'd memory or register. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > arch/x86/{kernel => include/asm}/fpu/context.h | 2 ++ > arch/x86/kernel/fpu/core.c | 2 +- > arch/x86/kernel/fpu/regset.c | 2 +- > arch/x86/kernel/fpu/signal.c | 2 +- > arch/x86/kernel/fpu/xstate.c | 2 +- > 5 files changed, 6 insertions(+), 4 deletions(-) > rename arch/x86/{kernel => include/asm}/fpu/context.h (96%) Why rename and cause all this churn?
diff --git a/arch/x86/kernel/fpu/context.h b/arch/x86/include/asm/fpu/context.h similarity index 96% rename from arch/x86/kernel/fpu/context.h rename to arch/x86/include/asm/fpu/context.h index 958accf2ccf0..39dac18cd22c 100644 --- a/arch/x86/kernel/fpu/context.h +++ b/arch/x86/include/asm/fpu/context.h @@ -51,6 +51,8 @@ static inline void fpregs_activate(struct fpu *fpu) trace_x86_fpu_regs_activated(fpu); } +extern void restore_fpregs_from_fpstate(struct fpstate *fpstate, u64 mask); + /* Internal helper for switch_fpu_return() and signal frame setup */ static inline void fpregs_restore_userregs(void) { diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index c049561f373a..5296112d4273 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -7,6 +7,7 @@ * Gareth Hughes <gareth@valinux.com>, May 2000 */ #include <asm/fpu/api.h> +#include <asm/fpu/context.h> #include <asm/fpu/regset.h> #include <asm/fpu/sched.h> #include <asm/fpu/signal.h> @@ -18,7 +19,6 @@ #include <linux/pkeys.h> #include <linux/vmalloc.h> -#include "context.h" #include "internal.h" #include "legacy.h" #include "xstate.h" diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 75ffaef8c299..f93336f332e3 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -6,10 +6,10 @@ #include <linux/vmalloc.h> #include <asm/fpu/api.h> +#include <asm/fpu/context.h> #include <asm/fpu/signal.h> #include <asm/fpu/regset.h> -#include "context.h" #include "internal.h" #include "legacy.h" #include "xstate.h" diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index 91d4b6de58ab..f099a56c9a93 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -7,6 +7,7 @@ #include <linux/cpu.h> #include <linux/pagemap.h> +#include <asm/fpu/context.h> #include <asm/fpu/signal.h> #include <asm/fpu/regset.h> #include <asm/fpu/xstate.h> @@ -15,7 +16,6 @@ #include <asm/trapnr.h> #include <asm/trace/fpu.h> -#include "context.h" #include "internal.h" #include "legacy.h" #include "xstate.h" diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 39e1c8626ab9..ab5e26075716 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -15,6 +15,7 @@ #include <linux/vmalloc.h> #include <asm/fpu/api.h> +#include <asm/fpu/context.h> #include <asm/fpu/regset.h> #include <asm/fpu/signal.h> #include <asm/fpu/xcr.h> @@ -23,7 +24,6 @@ #include <asm/prctl.h> #include <asm/elf.h> -#include "context.h" #include "internal.h" #include "legacy.h" #include "xstate.h"
The file's fpregs_state_valid function is useful outside of arch/x86/kernel/fpu dir. Further commits in this series use fpregs_state_valid to determine whether a BPF helper should fetch fpu reg value from xsave'd memory or register. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- arch/x86/{kernel => include/asm}/fpu/context.h | 2 ++ arch/x86/kernel/fpu/core.c | 2 +- arch/x86/kernel/fpu/regset.c | 2 +- arch/x86/kernel/fpu/signal.c | 2 +- arch/x86/kernel/fpu/xstate.c | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) rename arch/x86/{kernel => include/asm}/fpu/context.h (96%)