From e6e019a801e99ba7888ed009c5c3b3c7b047af1e Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 12 Jan 2013 16:16:11 -0800 Subject: [PATCH] kernel - Fix signal FP save/restore issues when AVX is enabled * The kernel was not saving/restoring the full FP context when entering into or returning from a signal, leading to corrupt FP registers even when AVX is not used, when AVX is enabled in the kernel. ANY SIGNAL COULD CORRUPT THE FP STATE. * Fixed by adjusting the on-user-stack fpsave area sizes and operation. * This unfortunately changes a number of user visible structures. ucontext_t, mcontext_t, sigcontext, sigframe. It is POSSIBLE that most userland use cases will be unaffected, but I'm not holding my breath. Major-Sleuthing-by: ftigeot Testing-by: ftigeot, dillon --- sys/cpu/x86_64/include/signal.h | 11 ++++------- sys/cpu/x86_64/include/ucontext.h | 14 +++++++------- sys/platform/pc64/x86_64/machdep.c | 10 +++++++++- sys/platform/pc64/x86_64/npx.c | 16 ++++++++++++---- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/sys/cpu/x86_64/include/signal.h b/sys/cpu/x86_64/include/signal.h index f0ae11f6c3..2b43e386cd 100644 --- a/sys/cpu/x86_64/include/signal.h +++ b/sys/cpu/x86_64/include/signal.h @@ -93,14 +93,11 @@ struct sigcontext { unsigned int sc_fpformat; unsigned int sc_ownedfp; unsigned int sc_reserved; - unsigned int sc_unused01; - unsigned int sc_unused02; + unsigned int sc_unused[8]; - /* 16 byte aligned */ - - int sc_fpregs[128]; - int __spare__[16]; -}; + /* 64 byte aligned */ + int sc_fpregs[256]; /* 1024 bytes */ +} __attribute__((aligned(64))); #endif /* !_ANSI_SOURCE && !_POSIX_SOURCE */ diff --git a/sys/cpu/x86_64/include/ucontext.h b/sys/cpu/x86_64/include/ucontext.h index f0df094703..f19575b3b5 100644 --- a/sys/cpu/x86_64/include/ucontext.h +++ b/sys/cpu/x86_64/include/ucontext.h @@ -33,7 +33,7 @@ #ifndef _CPU_UCONTEXT_H_ #define _CPU_UCONTEXT_H_ -typedef struct __mcontext { +struct __mcontext { /* * The first 20 fields must match the definition of * sigcontext. So that we can support sigcontext @@ -73,18 +73,18 @@ typedef struct __mcontext { unsigned int mc_fpformat; unsigned int mc_ownedfp; unsigned int mc_reserved; - unsigned int mc_unused01; - unsigned int mc_unused02; + unsigned int mc_unused[8]; - /* 16 byte aligned */ + /* 64 byte aligned */ + int mc_fpregs[256]; /* 1024 bytes */ +} __attribute__((aligned(64))); - int mc_fpregs[128]; - int __spare__[16]; -} mcontext_t; +typedef struct __mcontext mcontext_t; #define _MC_FPFMT_NODEV 0x10000 /* device not present or configured */ #define _MC_FPFMT_387 0x10001 #define _MC_FPFMT_XMM 0x10002 +#define _MC_FPFMT_YMM 0x10003 #define _MC_FPOWNED_NONE 0x20000 /* FP state not used */ #define _MC_FPOWNED_FPU 0x20001 /* FP state came from FPU */ diff --git a/sys/platform/pc64/x86_64/machdep.c b/sys/platform/pc64/x86_64/machdep.c index d1ba94ff40..681810a5f9 100644 --- a/sys/platform/pc64/x86_64/machdep.c +++ b/sys/platform/pc64/x86_64/machdep.c @@ -482,7 +482,15 @@ sendsig(sig_t catcher, int sig, sigset_t *mask, u_long code) sp = (char *)regs->tf_rsp - sizeof(struct sigframe) - 128; } - /* Align to 16 bytes */ + /* + * XXX AVX needs 64-byte alignment but sigframe has other fields and + * the embedded ucontext is not at the front, so aligning this won't + * help us. Fortunately we bcopy in/out of the sigframe, so the + * kernel is ok. + * + * The problem though is if userland winds up trying to use the + * context directly. + */ sfp = (struct sigframe *)((intptr_t)sp & ~(intptr_t)0xF); /* Translate the signal is appropriate */ diff --git a/sys/platform/pc64/x86_64/npx.c b/sys/platform/pc64/x86_64/npx.c index 3150539de9..9361d32b47 100644 --- a/sys/platform/pc64/x86_64/npx.c +++ b/sys/platform/pc64/x86_64/npx.c @@ -445,13 +445,20 @@ npxpush(mcontext_t *mctx) } else { mctx->mc_ownedfp = _MC_FPOWNED_PCB; } - bcopy(td->td_savefpu, mctx->mc_fpregs, sizeof(mctx->mc_fpregs)); + KKASSERT(sizeof(*td->td_savefpu) <= sizeof(mctx->mc_fpregs)); + bcopy(td->td_savefpu, mctx->mc_fpregs, sizeof(*td->td_savefpu)); td->td_flags &= ~TDF_USINGFP; - mctx->mc_fpformat = +#ifndef CPU_DISABLE_AVX + if (cpu_xsave) + mctx->mc_fpformat = _MC_FPFMT_YMM; + else +#endif #ifndef CPU_DISABLE_SSE - (cpu_fxsr) ? _MC_FPFMT_XMM : + if (cpu_fxsr) + mctx->mc_fpformat = _MC_FPFMT_XMM; + else #endif - _MC_FPFMT_387; + mctx->mc_fpformat = _MC_FPFMT_387; } else { mctx->mc_ownedfp = _MC_FPOWNED_NONE; mctx->mc_fpformat = _MC_FPFMT_NODEV; @@ -497,6 +504,7 @@ npxpop(mcontext_t *mctx) */ if (td == mdcpu->gd_npxthread) npxsave(td->td_savefpu); + KKASSERT(sizeof(*td->td_savefpu) <= sizeof(mctx->mc_fpregs)); bcopy(mctx->mc_fpregs, td->td_savefpu, sizeof(*td->td_savefpu)); if ((td->td_savefpu->sv_xmm.sv_env.en_mxcsr & ~0xFFBF) #ifndef CPU_DISABLE_SSE -- 2.41.0