| Message ID | 20211011215813.558681373@linutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86/fpu: Preparatory cleanups for AMX support (part 1) | expand |
On Tue, Oct 12 2021 at 01:59, Thomas Gleixner wrote: > > The current series (#1) is based on: > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/fpu > > and also available from git: > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu-1 I've updated the git branch with the review comments which came in today addressed. The full stack is rebased on top of that along with a few other fixes. The delta patch to the current part-1 series is below. I'm going to wait a bit before sending out a V2 to give people time to react. Though I'm planning to send out part-2 based on the current state soonish. Thanks, tglx --- diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h index 4cf54d8ce17d..5ac5e4596b53 100644 --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -130,13 +130,13 @@ static inline void fpstate_init_soft(struct swregs_state *soft) {} /* State tracking */ DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx); -/* FPSTATE related functions which are exported to KVM */ +/* fpstate-related functions which are exported to KVM */ extern void fpu_init_fpstate_user(struct fpu *fpu); /* KVM specific functions */ extern void fpu_swap_kvm_fpu(struct fpu *save, struct fpu *rstor, u64 restore_mask); -extern int fpu_copy_kvm_uabi_to_vcpu(struct fpu *fpu, const void *buf, u64 xcr0, u32 *pkru); -extern void fpu_copy_vcpu_to_kvm_uabi(struct fpu *fpu, void *buf, unsigned int size, u32 pkru); +extern int fpu_copy_kvm_uabi_to_fpstate(struct fpu *fpu, const void *buf, u64 xcr0, u32 *pkru); +extern void fpu_copy_fpstate_to_kvm_uabi(struct fpu *fpu, void *buf, unsigned int size, u32 pkru); #endif /* _ASM_X86_FPU_API_H */ diff --git a/arch/x86/include/asm/fpu/sched.h b/arch/x86/include/asm/fpu/sched.h index 99a8820e8cc4..cdb78d590c86 100644 --- a/arch/x86/include/asm/fpu/sched.h +++ b/arch/x86/include/asm/fpu/sched.h @@ -11,7 +11,7 @@ extern void save_fpregs_to_fpstate(struct fpu *fpu); extern void fpu__drop(struct fpu *fpu); -extern int fpu_clone(struct task_struct *dst, unsigned long clone_flags); +extern int fpu_clone(struct task_struct *dst); extern void fpu_flush_thread(void); /* diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 1c5e753ba3f1..ac540a7d410e 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -184,7 +184,7 @@ void fpu_swap_kvm_fpu(struct fpu *save, struct fpu *rstor, u64 restore_mask) } EXPORT_SYMBOL_GPL(fpu_swap_kvm_fpu); -void fpu_copy_vcpu_to_kvm_uabi(struct fpu *fpu, void *buf, +void fpu_copy_fpstate_to_kvm_uabi(struct fpu *fpu, void *buf, unsigned int size, u32 pkru) { union fpregs_state *kstate = &fpu->state; @@ -196,12 +196,14 @@ void fpu_copy_vcpu_to_kvm_uabi(struct fpu *fpu, void *buf, XSTATE_COPY_XSAVE); } else { memcpy(&ustate->fxsave, &kstate->fxsave, sizeof(ustate->fxsave)); + /* Make it restorable on a XSAVE enabled host */ + ustate->xsave.header.xfeatures = XFEATURE_MASK_FPSSE; } } -EXPORT_SYMBOL_GPL(fpu_copy_vcpu_to_kvm_uabi); +EXPORT_SYMBOL_GPL(fpu_copy_fpstate_to_kvm_uabi); -int fpu_copy_kvm_uabi_to_vcpu(struct fpu *fpu, const void *buf, u64 xcr0, - u32 *vpkru) +int fpu_copy_kvm_uabi_to_fpstate(struct fpu *fpu, const void *buf, u64 xcr0, + u32 *vpkru) { union fpregs_state *kstate = &fpu->state; const union fpregs_state *ustate = buf; @@ -234,7 +236,7 @@ int fpu_copy_kvm_uabi_to_vcpu(struct fpu *fpu, const void *buf, u64 xcr0, xstate_init_xcomp_bv(&kstate->xsave, xfeatures_mask_uabi()); return 0; } -EXPORT_SYMBOL_GPL(fpu_copy_kvm_uabi_to_vcpu); +EXPORT_SYMBOL_GPL(fpu_copy_kvm_uabi_to_fpstate); #endif /* CONFIG_KVM */ void kernel_fpu_begin_mask(unsigned int kfpu_mask) @@ -344,7 +346,7 @@ EXPORT_SYMBOL_GPL(fpu_init_fpstate_user); #endif /* Clone current's FPU state on fork */ -int fpu_clone(struct task_struct *dst, unsigned long clone_flags) +int fpu_clone(struct task_struct *dst) { struct fpu *src_fpu = ¤t->thread.fpu; struct fpu *dst_fpu = &dst->thread.fpu; @@ -363,11 +365,9 @@ int fpu_clone(struct task_struct *dst, unsigned long clone_flags) /* * No FPU state inheritance for kernel threads and IO - * worker threads. Neither CLONE_THREAD needs a copy - * of the FPU state. + * worker threads. */ - if (clone_flags & CLONE_THREAD || - dst->flags & (PF_KTHREAD | PF_IO_WORKER)) { + if (dst->flags & (PF_KTHREAD | PF_IO_WORKER)) { /* Clear out the minimal state */ memcpy(&dst_fpu->state, &init_fpstate, init_fpstate_copy_size()); diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 6e729060beb3..b022df95a302 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1195,15 +1195,13 @@ int copy_sigframe_from_user_to_xstate(struct xregs_state *xsave, return copy_uabi_to_xstate(xsave, NULL, ubuf); } -static bool validate_xsaves_xrstors(u64 mask) +static bool validate_independent_components(u64 mask) { u64 xchk; if (WARN_ON_FPU(!cpu_feature_enabled(X86_FEATURE_XSAVES))) return false; - /* - * Validate that this is a independent compoment. - */ + xchk = ~xfeatures_mask_independent(); if (WARN_ON_ONCE(!mask || mask & xchk)) @@ -1222,13 +1220,13 @@ static bool validate_xsaves_xrstors(u64 mask) * buffer should be zeroed otherwise a consecutive XRSTORS from that buffer * can #GP. * - * The feature mask must be a subset of the independent features + * The feature mask must be a subset of the independent features. */ void xsaves(struct xregs_state *xstate, u64 mask) { int err; - if (!validate_xsaves_xrstors(mask)) + if (!validate_independent_components(mask)) return; XSTATE_OP(XSAVES, xstate, (u32)mask, (u32)(mask >> 32), err); @@ -1246,13 +1244,13 @@ void xsaves(struct xregs_state *xstate, u64 mask) * Proper usage is to restore the state which was saved with * xsaves() into @xstate. * - * The feature mask must be a subset of the independent features + * The feature mask must be a subset of the independent features. */ void xrstors(struct xregs_state *xstate, u64 mask) { int err; - if (!validate_xsaves_xrstors(mask)) + if (!validate_independent_components(mask)) return; XSTATE_OP(XRSTORS, xstate, (u32)mask, (u32)(mask >> 32), err); diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 83a34fd828d5..5cd82082353e 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -154,7 +154,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, unsigned long arg, frame->flags = X86_EFLAGS_FIXED; #endif - fpu_clone(p, clone_flags); + fpu_clone(p); /* Kernel thread ? */ if (unlikely(p->flags & PF_KTHREAD)) { diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index ac02945756ec..f7826148edc9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4701,9 +4701,9 @@ static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu, if (!vcpu->arch.guest_fpu) return; - fpu_copy_vcpu_to_kvm_uabi(vcpu->arch.guest_fpu, guest_xsave->region, - sizeof(guest_xsave->region), - vcpu->arch.pkru); + fpu_copy_fpstate_to_kvm_uabi(vcpu->arch.guest_fpu, guest_xsave->region, + sizeof(guest_xsave->region), + vcpu->arch.pkru); } static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu, @@ -4712,9 +4712,9 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu, if (!vcpu->arch.guest_fpu) return 0; - return fpu_copy_kvm_uabi_to_vcpu(vcpu->arch.guest_fpu, - guest_xsave->region, - supported_xcr0, &vcpu->arch.pkru); + return fpu_copy_kvm_uabi_to_fpstate(vcpu->arch.guest_fpu, + guest_xsave->region, + supported_xcr0, &vcpu->arch.pkru); } static void kvm_vcpu_ioctl_x86_get_xcrs(struct kvm_vcpu *vcpu, @@ -9787,8 +9787,8 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu) static void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) { /* - * Guest with protected state have guest_fpu == NULL which makes - * the swap only safe the host state. Exclude PKRU from restore as + * Guests with protected state have guest_fpu == NULL which makes + * the swap only save the host state. Exclude PKRU from restore as * it is restored separately in kvm_x86_ops.run(). */ fpu_swap_kvm_fpu(vcpu->arch.user_fpu, vcpu->arch.guest_fpu, @@ -9800,7 +9800,7 @@ static void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) static void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) { /* - * Guest with protected state have guest_fpu == NULL which makes + * Guests with protected state have guest_fpu == NULL which makes * swap only restore the host state. */ fpu_swap_kvm_fpu(vcpu->arch.guest_fpu, vcpu->arch.user_fpu, ~0ULL);
The recent attempts to support the new AMX feature just tried to bolt it into the exisiting FPU code: https://lore.kernel.org/r/20211001223728.9309-1-chang.seok.bae@intel.com As demonstrated with the supervisor bits, that's not really sensible and leads to similar issues. I've worked with Chang and Dave in the past few days on sorting this out. Many thanks for their effort and support! This series is a renewed effort to make this more palatable. It's the first part of a 4 part submission which work towards a clean AMX integration into the FPU code: 1) Cleans up existing mess. Historical leftovers, shortcomings and especially the universal kitchen sink asm/fpu/internal.h which is included all over the place for the wrong reasons. This series has a value independent of AMX, but allows to make the integration and conversion to the new world order of dynamically enabled feature bits simpler. 2) Introduces a container for the actual register storage which carries information about the kernel and user space features and sizes supported by it to easy the integration of dynamically enabled feature and the resulting different buffer sizes. 3) Replaces a ton of state variables by introducing structures which carry that information 4) The actual AMX and dynamic feature enable bits which have been significantly reworked on top of #1 - #3 and to address shortcomings of the previous submissions. The current series (#1) is based on: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/fpu and also available from git: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu-1 The full series which has #1-#4 included can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu Thanks, tglx --- arch/x86/events/perf_event.h | 1 arch/x86/ia32/ia32_signal.c | 1 arch/x86/include/asm/fpu/api.h | 31 ++ arch/x86/include/asm/fpu/internal.h | 530 ------------------------------------ arch/x86/include/asm/fpu/signal.h | 13 arch/x86/include/asm/fpu/xcr.h | 11 arch/x86/include/asm/fpu/xstate.h | 6 arch/x86/include/asm/pkru.h | 2 arch/x86/kernel/cpu/bugs.c | 2 arch/x86/kernel/cpu/common.c | 2 arch/x86/kernel/fpu/bugs.c | 2 arch/x86/kernel/fpu/core.c | 163 ++++++++--- arch/x86/kernel/fpu/init.c | 29 - arch/x86/kernel/fpu/regset.c | 6 arch/x86/kernel/fpu/signal.c | 21 - arch/x86/kernel/fpu/xstate.c | 164 ++++++----- arch/x86/kernel/process.c | 6 arch/x86/kernel/process_32.c | 5 arch/x86/kernel/process_64.c | 5 arch/x86/kernel/ptrace.c | 1 arch/x86/kernel/sev.c | 2 arch/x86/kernel/signal.c | 1 arch/x86/kernel/smpboot.c | 2 arch/x86/kernel/traps.c | 2 arch/x86/kvm/svm/sev.c | 2 arch/x86/kvm/vmx/vmx.c | 2 arch/x86/kvm/x86.c | 192 +------------ arch/x86/math-emu/fpu_entry.c | 2 arch/x86/mm/extable.c | 4 arch/x86/power/cpu.c | 2 b/arch/x86/include/asm/fpu/sched.h | 68 ++++ b/arch/x86/kernel/fpu/context.h | 85 +++++ b/arch/x86/kernel/fpu/internal.h | 30 ++ b/arch/x86/kernel/fpu/legacy.h | 115 +++++++ b/arch/x86/kernel/fpu/xstate.h | 198 +++++++++++++ 35 files changed, 822 insertions(+), 886 deletions(-)