ARM: VFP: Save / restore VFP state on the signal handler path
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
v2 cover ARM: VFP: Save / restore VFP state on the signal handler path
This fixes a VFP corruption problem in a process's main thread, when both the main thread and a signal handler triggered for the process uses floating point ops.
Changes since v1:
- save / restore VFP exception registers as well, to support user space exception handling/VFP emulation.
- extend the existing ptrace VFP struct with the exception regs, so that the VFP frame is equivalent for the ptrace and signal handler case.
--Imre
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
So far vfp_sync_state worked only for threads other than the current one. This worked for tracing other threads, but not for PTRACE_TRACEME. Syncing for the current thread will also be needed by an upcoming patch adding support for VFP context save / restore around signal handlers.
For SMP we need get_cpu now, since we have to protect the FPEXC register, other than this things remained the same for threads other than the current.
--- arch/arm/vfp/vfpmodule.c | 46 +++++++++++++++++++++++++++++++--------------- 1 files changed, 31 insertions(+), 15 deletions(-)
arch/arm/vfp/vfpmodule.c +31 -15
@@ -426,12 +426,19 @@ static inline void vfp_pm_init(void) { }#endif /* CONFIG_PM */ /* -* Synchronise the hardware VFP state of a thread other than current with the -* saved one. This function is used by the ptrace mechanism. +* Synchronise the hardware VFP state of a thread with the saved one. +* This function is used by the ptrace mechanism and the signal handler path. */ -#ifdef CONFIG_SMP void vfp_sync_state (struct thread_info *thread) { +unsigned int cpu =get_cpu (); +u32 fpexc =fmrx (FPEXC ); +int vfp_enabled; +int self; + + vfp_enabled = fpexc &FPEXC_EN ; + self = thread ==current_thread_info (); +#ifdef CONFIG_SMP /* * On SMP systems, the VFP state is automatically saved at every * context switch. We mark the thread VFP state as belonging to a
@@ -439,18 +446,22 @@ void vfp_sync_state(struct thread_info *thread)* needed. */ thread->vfpstate.hard.cpu =NR_CPUS ; -} -#else -void vfp_sync_state (struct thread_info *thread) -{ -unsigned int cpu =get_cpu (); -u32 fpexc =fmrx (FPEXC ); -/* -* If VFP is enabled, the previous state was already saved and -* last_VFP_context updated. +* Only the current thread's saved VFP context can be out-of-date. +* For others there is nothing else to do, since we already ensured +* force loading above. */ -if (fpexc &FPEXC_EN ) +if (!self) + goto out; +#endif +/* +* If the VFP is enabled only the current thread's saved VFP +* context can get out-of-date. For other threads the context +* was updated when the current thread started to use the VFP. +* This also means that the context will be reloaded next time +* the thread uses the VFP, so no need to enforce it. +*/ +if (vfp_enabled && !self) goto out;if (!last_VFP_context[cpu])
@@ -459,8 +470,14 @@ void vfp_sync_state(struct thread_info *thread)/* * Save the last VFP state on this CPU. */ -fmxr (FPEXC , fpexc |FPEXC_EN ); +if (!vfp_enabled) +fmxr (FPEXC , fpexc |FPEXC_EN );vfp_save_state (last_VFP_context[cpu], fpexc); +/* +* Disable VFP in case it was enabled so that the force reload +* can happen. +*/ + fpexc &= ~FPEXC_EN ;fmxr (FPEXC , fpexc);/*
@@ -472,7 +489,6 @@ void vfp_sync_state(struct thread_info *thread) out:put_cpu (); } -#endif #include <linux/smp.h>
Signature · 1 line
1.6.6
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
Quoted 8 lines · Imre Deak
So far
vfp_sync_stateworked only for threads other than the current one. This worked for tracing other threads, but not forPTRACE_TRACEME. Syncing for the current thread will also be needed by an upcoming patch adding support for VFP context save / restore around signal handlers.For SMP we need
get_cpunow, since we have to protect the FPEXC register, other than this things remained the same for threads other than the current.
I don't think we should make this function any more complicated than it already is.
For entering signal handlers, the situation is a lot more simple than that found in vfp_sync_state() - since we know that we're always going to be running the thread we're concerned with:
- if VFP is enabled
- use vfp_save_state() to save a copy of VFP state
- disable VFP
- if VFP is disabled
- save a copy of ti->vfp_state
On signal return:
- restore copy of ti->vfp_state
- atomically (wrt preemption)
- if last_VFP_context[cpu] is &ti->vfp_state
- NULL out last_VFP_context[cpu]
- disable VFP
There's no need to fiddle with ti->vfp_state.hard.cpu.
Note that saving the vfp_hard_struct to userspace is not on since it then becomes a userspace API; it has to be translated into something that we can keep better control over for userspace. Moreover, vfp_hard_struct contains state private to the kernel; allowing userspace to have control over things like 'cpu' is a very bad idea.
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Imre Deak
So far
vfp_sync_stateworked only for threads other than the current one. This worked for tracing other threads, but not forPTRACE_TRACEME. Syncing for the current thread will also be needed by an upcoming patch adding support for VFP context save / restore around signal handlers.
Quoted 6 lines
Quoted 3 lines
For SMP we need
get_cpunow, since we have to protect the FPEXC register, other than this things remained the same for threads other than the current.I don't think we should make this function any more complicated than it already is.
... and the more I look at vfp_sync_state(), the more I believe it's trying to do its job in a really obscure way.
Essentially, last_VFP_context[] tracks who owns the state in the VFP hardware. If last_VFP_context[] is the context for the thread which we're interested in, then the VFP hardware has context which is not saved in the software state - so we need to bring the software state up to date.
If last_VFP_context[] is for some other thread, we really don't care what state the VFP hardware is in; it doesn't contain any information pertinent to the thread we're trying to deal with - so leave the hardware well alone.
As a side effect, this makes vfp_sync_state() callable for the current thread, and allows it to do the right thing there as well.
arch/arm/vfp/vfpmodule.c +16 -20
@@ -444,32 +444,28 @@ void vfp_sync_state(struct thread_info *thread)void vfp_sync_state (struct thread_info *thread) {unsigned int cpu =get_cpu (); -u32 fpexc =fmrx (FPEXC );/* -* If VFP is enabled, the previous state was already saved and -* last_VFP_context updated. +* If the thread we're interested in is the current owner of the +* hardware VFP state, then we need to save its state. */ -if (fpexc &FPEXC_EN ) - goto out; - -if (!last_VFP_context[cpu]) - goto out; +if (last_VFP_context[cpu] == &thread->vfpstate) { +u32 fpexc =fmrx (FPEXC ); -/* -* Save the last VFP state on this CPU. -*/ -fmxr (FPEXC , fpexc |FPEXC_EN ); -vfp_save_state (last_VFP_context[cpu], fpexc); -fmxr (FPEXC , fpexc); +/* +* Save the last VFP state on this CPU. +*/ +fmxr (FPEXC , fpexc |FPEXC_EN ); +vfp_save_state (&thread->vfpstate, fpexc |FPEXC_EN ); +fmxr (FPEXC , fpexc); -/* -* Set the context to NULL to force a reload the next time the thread -* uses the VFP. -*/ - last_VFP_context[cpu] =NULL ; +/* +* Set the context to NULL to force a reload the next time +* the thread uses the VFP. +*/ + last_VFP_context[cpu] =NULL ; + } -out:put_cpu (); }#endif
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
arch/arm/vfp/vfpmodule.c void vfp_sync_state(struct thread_info *thread) · line 460
Quoted 6 lines · Russell King - ARM Linux
arch/arm/vfp/vfpmodule.c +6
-0+/* +* Save the last VFP state on this CPU. +*/ +fmxr (FPEXC , fpexc |FPEXC_EN ); +vfp_save_state (&thread->vfpstate, fpexc |FPEXC_EN ); +fmxr (FPEXC , fpexc);
Erm, slight correction here:
+ fmxr(FPEXC, fpexc & ~FPEXC_EN);
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
Review
Quoted 11 lines · Russell King - ARM Linux
Review
Quoted 6 lines · Russell King - ARM Linux
(quoted diff) +6
-0+ /* + * Save the last VFP state on this CPU. + */ + fmxr(FPEXC, fpexc | FPEXC_EN); + vfp_save_state(&thread->vfpstate, fpexc | FPEXC_EN); + fmxr(FPEXC, fpexc);Erm, slight correction here:
(quoted diff) +1
-0+ fmxr(FPEXC, fpexc & ~FPEXC_EN);
And finally... let's improve the ptrace code to allow more efficient monitoring of VFP state. There's no point invalidating the hardware context just because we want to inspect the VFP state via ptrace.
arch/arm/include/asm/thread_info.h +2 -1
@@ -115,7 +115,8 @@ extern void iwmmxt_task_restore(struct thread_info *, void *);extern void iwmmxt_task_release (struct thread_info *);extern void iwmmxt_task_switch (struct thread_info *); -extern void vfp_sync_state (struct thread_info *thread); +extern void vfp_sync_hwstate (struct thread_info *); +extern void vfp_flush_hwstate (struct thread_info *);#endif
arch/arm/kernel/ptrace.c +4 -2
@@ -700,7 +700,7 @@ static int ptrace_getvfpregs(struct task_struct *tsk, void __user *data)union vfp_state *vfp = &thread->vfpstate;struct user_vfp __user *ufp = data; -vfp_sync_state (thread); +vfp_sync_hwstate (thread);/* copy the floating point registers */ if (copy_to_user(&ufp->fpregs, &vfp->hard.fpregs,
@@ -723,7 +723,7 @@ static int ptrace_setvfpregs(struct task_struct *tsk, void __user *data)union vfp_state *vfp = &thread->vfpstate;struct user_vfp __user *ufp = data; -vfp_sync_state (thread); +vfp_sync_hwstate (thread);/* copy the floating point registers */ if (copy_from_user(&vfp->hard.fpregs, &ufp->fpregs,
@@ -734,6 +734,8 @@ static int ptrace_setvfpregs(struct task_struct *tsk, void __user *data)if (get_user (vfp->hard.fpscr, &ufp->fpscr))return -EFAULT ; +vfp_flush_hwstate (thread); + return 0; }#endif
arch/arm/vfp/vfpmodule.c +23 -2
@@ -430,7 +430,11 @@ static inline void vfp_pm_init(void) { }* saved one. This function is used by the ptrace mechanism. */ #ifdef CONFIG_SMP -void vfp_sync_state (struct thread_info *thread) +void vfp_sync_hwstate (struct thread_info *thread) +{ +} + +void vfp_flush_hwstate (struct thread_info *thread) {/* * On SMP systems, the VFP state is automatically saved at every
@@ -441,7 +445,7 @@ void vfp_sync_state(struct thread_info *thread) thread->vfpstate.hard.cpu = NR_CPUS; }#else -void vfp_sync_state (struct thread_info *thread) +void vfp_sync_hwstate (struct thread_info *thread) {unsigned int cpu =get_cpu ();
@@ -457,6 +461,23 @@ void vfp_sync_state(struct thread_info *thread)*/ fmxr (FPEXC , fpexc |FPEXC_EN );vfp_save_state (&thread->vfpstate, fpexc |FPEXC_EN ); +fmxr (FPEXC , fpexc); + } + +put_cpu (); +} + +void vfp_flush_hwstate (struct thread_info *thread) +{ +unsigned int cpu =get_cpu (); + +/* +* If the thread we're interested in is the current owner of the +* hardware VFP state, then we need to save its state. +*/ +if (last_VFP_context[cpu] == &thread->vfpstate) { +u32 fpexc =fmrx (FPEXC ); +fmxr (FPEXC , fpexc & ~FPEXC_EN );/*
RFC PATCH v2 1/2 ARM: VFP: add support to sync the VFP state of the current thread
In reply to Russell King - ARM LinuxQuoted 23 lines · ext Russell King - ARM Linux
Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Imre Deak
So far
vfp_sync_stateworked only for threads other than the current one. This worked for tracing other threads, but not forPTRACE_TRACEME. Syncing for the current thread will also be needed by an upcoming patch adding support for VFP context save / restore around signal handlers.Quoted 6 lines
Quoted 3 lines
For SMP we need
get_cpunow, since we have to protect the FPEXC register, other than this things remained the same for threads other than the current.I don't think we should make this function any more complicated than it already is.
... and the more I look at
vfp_sync_state(), the more I believe it's trying to do its job in a really obscure way.Essentially, last_VFP_context[] tracks who owns the state in the VFP hardware. If last_VFP_context[] is the context for the thread which we're interested in, then the VFP hardware has context which is not saved in the software state - so we need to bring the software state up to date.
Quoted 7 lines
If last_VFP_context[] is for some other thread, we really don't care what state the VFP hardware is in; it doesn't contain any information pertinent to the thread we're trying to deal with - so leave the hardware well alone.
As a side effect, this makes
vfp_sync_state()callable for the current thread, and allows it to do the right thing there as well.
Agreed, vfp_sync_state is cleaner and more efficient this way. One note is that the SMP version will still not update the software state in case it's called for the current thread and the VFP is enabled.
--Imre
Quoted 54 lines
arch/arm/vfp/vfpmodule.c +16
-20@@ -444,32 +444,28 @@ void vfp_sync_state(struct thread_info *thread)void vfp_sync_state (struct thread_info *thread) {unsigned int cpu =get_cpu (); -u32 fpexc =fmrx (FPEXC );/* -* If VFP is enabled, the previous state was already saved and -* last_VFP_context updated. +* If the thread we're interested in is the current owner of the +* hardware VFP state, then we need to save its state. */ -if (fpexc &FPEXC_EN ) - goto out; - -if (!last_VFP_context[cpu]) - goto out; +if (last_VFP_context[cpu] == &thread->vfpstate) { +u32 fpexc =fmrx (FPEXC ); -/* -* Save the last VFP state on this CPU. -*/ -fmxr (FPEXC , fpexc |FPEXC_EN ); -vfp_save_state (last_VFP_context[cpu], fpexc); -fmxr (FPEXC , fpexc); +/* +* Save the last VFP state on this CPU. +*/ +fmxr (FPEXC , fpexc |FPEXC_EN ); +vfp_save_state (&thread->vfpstate, fpexc |FPEXC_EN ); +fmxr (FPEXC , fpexc); -/* -* Set the context to NULL to force a reload the next time the thread -* uses the VFP. -*/ - last_VFP_context[cpu] =NULL ; +/* +* Set the context to NULL to force a reload the next time +* the thread uses the VFP. +*/ + last_VFP_context[cpu] =NULL ; + } -out:put_cpu (); }#endif
RFC PATCH v2 2/2 ARM: VFP: preserve the HW context when calling signal handlers
In reply to Imre DeakSignal handlers can use floating point, so prevent them to corrupt the main thread's VFP context. So far there were two signal stack frame formats defined based on the VFP implementation, but the user struct used for ptrace covers all posibilities, so use it for the signal stack too. This patch extends the user struct and leaves its magic number the same, in the hope that user space code does not depend on its size and can parse the original regs w/o problems.
Support to save / restore the exception registers was added by Will Deacon.
--- arch/arm/include/asm/ucontext.h | 19 +++----- arch/arm/include/asm/user.h | 3 + arch/arm/kernel/signal.c | 91 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 16 deletions(-)
arch/arm/include/asm/ucontext.h +7 -12
@@ -59,23 +59,18 @@ struct iwmmxt_sigframe {#endif /* CONFIG_IWMMXT */ #ifdef CONFIG_VFP -#if __LINUX_ARM_ARCH__ <6 -/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra -* word after the registers, and a word of padding@the end for -* alignment. */ #define VFP_MAGIC 0x56465001 -#define VFP_STORAGE_SIZE 152 -#else -#define VFP_MAGIC 0x56465002 -#define VFP_STORAGE_SIZE 144 -#endif struct vfp_sigframe {unsigned long magic;unsigned long size; -union vfp_state storage; -}; +struct user_vfp ufp; +} __attribute__((__aligned__ (8 ))); + +/* 8 byte for magic and size, 272 byte for ufp */ +#define VFP_STORAGE_SIZE sizeof(struct vfp_sigframe) +#endif /* CONFIG_VFP */ /*
@@ -91,7 +86,7 @@ struct aux_sigframe {#ifdef CONFIG_IWMMXT struct iwmmxt_sigframe iwmmxt;#endif -#if 0 && definedCONFIG_VFP /* Not yet saved. */ +#ifdef CONFIG_VFP struct vfp_sigframe vfp;#endif /* Something that isn't a valid magic number for any coprocessor. */
arch/arm/include/asm/user.h +3 -0
@@ -88,6 +88,9 @@ struct user{struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2; };#endif /* _ARM_USER_H */
arch/arm/kernel/signal.c +87 -4
115 lines not on this page · raw message
Signature · 1 line
1.6.6
RFC PATCH v2 2/2 ARM: VFP: preserve the HW context when calling signal handlers
arch/arm/include/asm/user.h struct user{ · line 93
Quoted 11 lines · Imre Deak
arch/arm/include/asm/user.h +3
-0@@ -88,6 +88,9 @@ struct user{struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2;
Absolutely no way is this anywhere near suitable. This is a _userspace_ _visible_ API change, breaking anyone using the ptrace API by overwriting additional memory which they will not be expecting.
RFC PATCH v2 2/2 ARM: VFP: preserve the HW context when calling signal handlers
Quoted 16 lines · ext Russell King - ARM Linux
arch/arm/include/asm/user.h struct user{ · line 93
Quoted 11 lines · Imre Deak
arch/arm/include/asm/user.h +3
-0@@ -88,6 +88,9 @@ struct user{struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2;Absolutely no way is this anywhere near suitable. This is a _userspace_ _visible_ API change, breaking anyone using the ptrace API by overwriting additional memory which they will not be expecting.
Right, don't know what made me think that this will work out. Perhaps someone mentioning that the corresponding IOCTL is not in use yet. But that was about half a year ago :)
I'll resend adding the new regs only to the signal frame, leaving the above as is.
--Imre
RFC PATCH v2 2/2 ARM: VFP: preserve the HW context when calling signal handlers
Quoted 6 lines · Imre Deak
Right, don't know what made me think that this will work out. Perhaps someone mentioning that the corresponding IOCTL is not in use yet. But that was about half a year ago :)
I'll resend adding the new regs only to the signal frame, leaving the above as is.
Second point on this. Currently, the VFP context which we thought about saving onto the sigframe looks like this:
#if __LINUX_ARM_ARCH__ < 6
/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra
* word after the registers, and a word of padding at the end for
* alignment. */
#define VFP_MAGIC 0x56465001
#define VFP_STORAGE_SIZE 152
#else
#define VFP_MAGIC 0x56465002
#define VFP_STORAGE_SIZE 144
#endif
struct vfp_sigframe
{
unsigned long magic;
unsigned long size;
union vfp_state storage;
};This is horribly outdated. We save:
- 16 or 32 64-bit registers depending on whether VFPv3
- one 32-bit word of fpmx state if < ARMv6
- one 32-bit word of fpexc
- one 32-bit word of fpscr
- one 32-bit word of fpinst
- one 32-bit word of fpinst2
- cpu if SMP
This gives potentially the following options:
VFPv3 ARMv6 SMP
n n n 16*8+5*4 = 148
y n n 32*8+5*4 = 276
n y n 16*8+4*4 = 144
y y n 32*8+4*4 = 272
n n y 16*8+6*4 = 152 *
y n y 32*8+6*4 = 280 *
n y y 16*8+5*4 = 148
y y y 32*8+5*4 = 276
The two marked with '*' are very unlikely to occur.
I think this technically comes under the heading of 'a disaster waiting to happen'.
We currently have no way to convey these possibilities to anything dealing with stack frames; certainly userspace applications which may decide to inspect the sigframe aren't going to deal with all these possibilities correctly - if we're lucky, they'll get one case right.
The stack frame should not care about whether we're running on SMP or not - and that rules out using vfp_hard_struct or vfp_state in the sigframe. So we're into having a different structure.
Since sigframes are tagged, let's make use of that facility. Let's save the 64-bit VFP registers - that way, the size of this structure defines how many registers there are. num_regs = struct size / 8.
Save fpmx_state as a separate tagged entity if it's present. (I doubt anyone has need to use this - it's just required to preserve VFP state.)
Then, save the remainder of the state information (fpexc, fpscr, fpinst, fpinst2 but _not_ cpu) as another separate tagged entity.
This means anyone who wants to inspect the VFP state has two or three tags to look for, but they're all well-defined, and are hopefully protected against the complexities of having to work out how to decode the current variable sized structure which we have@present.
RFC PATCH v2 2/2 ARM: VFP: preserve the HW context when calling signal handlers
Quoted 41 lines · ext Russell King - ARM Linux
Quoted 6 lines · Imre Deak
Right, don't know what made me think that this will work out. Perhaps someone mentioning that the corresponding IOCTL is not in use yet. But that was about half a year ago :)
I'll resend adding the new regs only to the signal frame, leaving the above as is.
Second point on this. Currently, the VFP context which we thought about saving onto the sigframe looks like this:
#if
__LINUX_ARM_ARCH__< 6
/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra
* word after the registers, and a word of padding at the end for
* alignment. */
#defineVFP_MAGIC0x56465001
#defineVFP_STORAGE_SIZE152
#else
#defineVFP_MAGIC0x56465002
#defineVFP_STORAGE_SIZE144
#endifstruct
vfp_sigframe{ unsigned long magic; unsigned long size; union vfp_state storage; };This is horribly outdated. We save:
- 16 or 32 64-bit registers depending on whether VFPv3
- one 32-bit word of fpmx state if < ARMv6
- one 32-bit word of fpexc
- one 32-bit word of fpscr
- one 32-bit word of fpinst
- one 32-bit word of fpinst2
- cpu if SMPThis gives potentially the following options:
Quoted 9 lines
VFPv3 ARMv6 SMP
n n n 16*8+5*4 = 148
y n n 32*8+5*4 = 276
n y n 16*8+4*4 = 144
y y n 32*8+4*4 = 272
n n y 16*8+6*4 = 152 *
y n y 32*8+6*4 = 280 *
n y y 16*8+5*4 = 148
y y y 32*8+5*4 = 276
But the proposed patch didn't use union vfp_state, but a fixed size struct for all 8 possibilities. Then the only drawback would be undefined regs in certain cases, but the register positions would be fixed. Also cpu is not part of the that struct.
Quoted 22 lines
The two marked with '*' are very unlikely to occur.
I think this technically comes under the heading of 'a disaster waiting to happen'.
We currently have no way to convey these possibilities to anything dealing with stack frames; certainly userspace applications which may decide to inspect the sigframe aren't going to deal with all these possibilities correctly - if we're lucky, they'll get one case right.
The stack frame should not care about whether we're running on SMP or not - and that rules out using
vfp_hard_structorvfp_statein the sigframe. So we're into having a different structure.Since sigframes are tagged, let's make use of that facility. Let's save the 64-bit VFP registers - that way, the size of this structure defines how many registers there are.
num_regs= struct size / 8.Save
fpmx_stateas a separate tagged entity if it's present. (I doubt anyone has need to use this - it's just required to preserve VFP state.)
Quoted 7 lines
Then, save the remainder of the state information (fpexc, fpscr, fpinst, fpinst2 but _not_ cpu) as another separate tagged entity.
This means anyone who wants to inspect the VFP state has two or three tags to look for, but they're all well-defined, and are hopefully protected against the complexities of having to work out how to decode the current variable sized structure which we have at present.
Would this still give a benefit over the one fixed struct solution?
--Imre
v3 cover ARM: VFP: Save / restore VFP state on the signal handler path
In reply to Imre DeakThis fixes a VFP corruption problem in a process's main thread, when both the main thread and a signal handler triggered for the process uses floating point ops.
Changes since v2 according to Russell's comments and patches:
- refactor vfp_sync_state to make it clearer
- break vfp_sync_state into vfp_{save/restore}_hwstate for efficiency
- Keep the additional VFP exception registers in a separate struct. This way we'll keep the existing ABI intact.
--Imre
v4 cover ARM: VFP: Save / restore VFP state on the signal handler path
Changes since v3:
- Rebased against mainline v2.6.34-rc3, since some parts have already been applied there.
Note that this problem might affect more people than previously thought. The CodeSourcery 2009q1-203 glibc_ports has some NEON optimized functions; for example using memset or memcpy in a signal handler and floating point in the main thread can corrupt the main thread's context. You can test this by replacing the floating point ops in the signal handler below with some dummy memset/memcpy.
I could only test these patches on a Cortex A8 UP system; it would be great if someone with an SMP board could give it a go. The test app to reproduce the problem (I've built it with -mfloat-abi=softfp -mfpu=neon):
#include <stdio.h>
#include <signal.h>
#include <stdlib.h>
#include <string.h>
#include <sys/time.h>
void handler(int signum)
{
volatile float f;
f = 50;
f /= 2;
f *= 2;
if (f != 50) {
printf("signal %3.3f\n", f);
exit(-1);
}
}int main(int argc, char **argv)
{
struct itimerval ival;
if (signal(SIGALRM, handler) == SIG_ERR) {
perror("signal");
exit(-1);
}
memset(&ival, 0, sizeof(ival));
ival.it_value.tv_usec = 1000;
ival.it_interval.tv_usec = 10000;
if (setitimer(ITIMER_REAL, &ival, NULL) < 0) {
perror("setitimer");
exit(-1);
}
while (1) {
volatile float f = 30;
f /= 3;
f *= 3;
if (f != 30) {
printf("main %3.3f\n", f);
exit(-1);
}
}
}v5 cover ARM: VFP: Save / restore VFP state on the signal handler path
From: Imre Deak <[email protected]>
Changes since v4:
- Do not add the packed attribute to user_vfp or user_vfp_exc. I added packed to these structs to prevent the extra padding in vfp_sigframe. But actually the padding is in accordance with the ARM ABI. Also removing the padding from user_vfp would break the ptrace interface.
--Imre
RFC PATCH v5 1/2 ARM: VFP: fix the SMP versions of vfp_{sync, flush}_hwstate
From: Imre Deak <[email protected]>
Recently the UP versions of these functions were refactored and as a side effect it became possible to call them for the current thread. This isn't true for the SMP versions however, so fix this up.
--- arch/arm/vfp/vfpmodule.c | 31 ++++++++++--------------------- 1 files changed, 10 insertions(+), 21 deletions(-)
arch/arm/vfp/vfpmodule.c +10 -21
@@ -428,26 +428,6 @@ static void vfp_pm_init(void)static inline void vfp_pm_init (void ) { }#endif /* CONFIG_PM */ -/* -* Synchronise the hardware VFP state of a thread other than current with the -* saved one. This function is used by the ptrace mechanism. -*/ -#ifdef CONFIG_SMP -void vfp_sync_hwstate (struct thread_info *thread) -{ -} - -void vfp_flush_hwstate (struct thread_info *thread) -{ -/* -* On SMP systems, the VFP state is automatically saved at every -* context switch. We mark the thread VFP state as belonging to a -* non-existent CPU so that the saved one will be reloaded when -* needed. -*/ - thread->vfpstate.hard.cpu =NR_CPUS ; -} -#else void vfp_sync_hwstate (struct thread_info *thread) {unsigned int cpu =get_cpu ();
@@ -490,9 +470,18 @@ void vfp_flush_hwstate(struct thread_info *thread) last_VFP_context[cpu] =NULL ; } +#ifdef CONFIG_SMP +/* +* For SMP we still have to take care of the case where the thread +* migrates to another CPU and then back to the original CPU on which +* the last VFP user is still the same thread. Mark the thread VFP +* state as belonging to a non-existent CPU so that the saved one will +* be reloaded in the above case. +*/ + thread->vfpstate.hard.cpu =NR_CPUS ; +#endif put_cpu (); } -#endif #include <linux/smp.h>
Signature · 1 line
1.7.0.2
RFC PATCH v5 1/2 ARM: VFP: fix the SMP versions of vfp_{sync,flush}_hwstate
Quoted 5 lines · imre.deak at nokia.com
From: Imre Deak <[email protected]>
Recently the UP versions of these functions were refactored and as a side effect it became possible to call them for the current thread. This isn't true for the SMP versions however, so fix this up.
This look safe to me; thanks.
RFC PATCH v5 2/2 ARM: VFP: preserve the HW context when calling signal handlers
In reply to imre.deak at nokia.comFrom: Imre Deak <[email protected]>
Signal handlers can use floating point, so prevent them to corrupt the main thread's VFP context. So far there were two signal stack frame formats defined based on the VFP implementation, but the user struct used for ptrace covers all posibilities, so use it for the signal stack too.
Introduce also a new user struct for VFP exception registers. In this too fields not relevant to the current VFP architecture are ignored.
Support to save / restore the exception registers was added by Will Deacon.
--- arch/arm/include/asm/ucontext.h | 23 +++++----- arch/arm/include/asm/user.h | 12 +++++- arch/arm/kernel/signal.c | 93 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 111 insertions(+), 17 deletions(-)
arch/arm/include/asm/ucontext.h +11 -12
@@ -59,23 +59,22 @@ struct iwmmxt_sigframe {#endif /* CONFIG_IWMMXT */ #ifdef CONFIG_VFP -#if __LINUX_ARM_ARCH__ <6 -/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra -* word after the registers, and a word of padding@the end for -* alignment. */ #define VFP_MAGIC 0x56465001 -#define VFP_STORAGE_SIZE 152 -#else -#define VFP_MAGIC 0x56465002 -#define VFP_STORAGE_SIZE 144 -#endif struct vfp_sigframe {unsigned long magic;unsigned long size; -union vfp_state storage; -}; +struct user_vfp ufp; +struct user_vfp_exc ufp_exc; +} __attribute__((__aligned__ (8 ))); + +/* +* 8 byte for magic and size, 264 byte for ufp, 12 bytes for ufp_exc, +* 4 bytes padding. +*/ +#define VFP_STORAGE_SIZE sizeof(struct vfp_sigframe) +#endif /* CONFIG_VFP */ /*
@@ -91,7 +90,7 @@ struct aux_sigframe {#ifdef CONFIG_IWMMXT struct iwmmxt_sigframe iwmmxt;#endif -#if 0 && definedCONFIG_VFP /* Not yet saved. */ +#ifdef CONFIG_VFP struct vfp_sigframe vfp;#endif /* Something that isn't a valid magic number for any coprocessor. */
arch/arm/include/asm/user.h +11 -1
@@ -83,11 +83,21 @@ struct user{/* * User specific VFP registers. If only VFPv2 is present, registers 16 to 31 -* are ignored by the ptrace system call. +* are ignored by the ptrace system call and the signal handler. */ struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; }; +/* +* VFP exception registers exposed to user space during signal delivery. +* Fields not relavant to the current VFP architecture are ignored. +*/ +struct user_vfp_exc { +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2; +}; +#endif /* _ARM_USER_H */
arch/arm/kernel/signal.c +89 -4
117 lines not on this page · raw message
Signature · 1 line
1.7.0.2
RFC PATCH v5 2/2 ARM: VFP: preserve the HW context when calling signal handlers
Quoted 8 lines · imre.deak at nokia.com
From: Imre Deak <[email protected]>
Signal handlers can use floating point, so prevent them to corrupt the main thread's VFP context. So far there were two signal stack frame formats defined based on the VFP implementation, but the user struct used for ptrace covers all posibilities, so use it for the signal stack too.
Quoted 6 lines
Introduce also a new user struct for VFP exception registers. In this too fields not relevant to the current VFP architecture are ignored.
Support to save / restore the exception registers was added by Will Deacon.
Look fine to me, but I'd like an ack from Daniel since he partly looks after the toolchain side of things and has dealt with this area before.
v5 cover ARM: VFP: Save / restore VFP state on the signal handler path
In reply to imre.deak at nokia.comQuoted 7 lines · imre.deak at nokia.com
From: Imre Deak <[email protected]>
Changes since v4:
- Do not add the packed attribute touser_vfporuser_vfp_exc. I added packed to these structs to prevent the extra padding invfp_sigframe. But actually the padding is in accordance with the ARM ABI.
EABI, OABI or both?
In general it's a really good idea, if you know there will be padding, to insert dummy fields where the padding goes so that it doesn't make any difference what compiler, ABI and settings are used.
-- Jamie
v5 cover ARM: VFP: Save / restore VFP state on the signal handler path
Quoted 10 lines · ext Jamie Lokier
Quoted 7 lines · imre.deak at nokia.com
From: Imre Deak <[email protected]>
Changes since v4:
- Do not add the packed attribute touser_vfporuser_vfp_exc. I added packed to these structs to prevent the extra padding invfp_sigframe. But actually the padding is in accordance with the ARM ABI.EABI, OABI or both?
I checked only EABI, which specifies alignment to the struct's alignment, which is 8 bytes for struct user_vfp.
I haven't found an official OABI spec, but some googling turned up that there the requirement is only to align to word size, so we'd have a difference in the struct vfp_sigframe's size and the alignment of the user_vfp_exc within.
Quoted 3 lines
In general it's a really good idea, if you know there will be padding, to insert dummy fields where the padding goes so that it doesn't make any difference what compiler, ABI and settings are used.
Ok. If I understand correctly this is needed for kernels with support for both OABI and EABI.
There is implicit alignment in struct user_vfp and struct user_vfp_exc. I can fix up user_vfp_exc since it's new and not used by anyone, but user_vfp is used by ptrace. I don't see a problem adding the padding to it either since the kernel ever accesses it on a field-by-field basis, but for kernels built with an OABI compiler this would mean an increase in the struct's size. If this is not a problem I'd follow up with a 2/2 patch according to the following.
Please let me know if it's ok with you.
--Imre
arch/arm/include/asm/ucontext.h +8 -12
@@ -59,23 +59,19 @@ struct iwmmxt_sigframe {#endif /* CONFIG_IWMMXT */ #ifdef CONFIG_VFP -#if __LINUX_ARM_ARCH__ <6 -/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra -* word after the registers, and a word of padding@the end for -* alignment. */ #define VFP_MAGIC 0x56465001 -#define VFP_STORAGE_SIZE 152 -#else -#define VFP_MAGIC 0x56465002 -#define VFP_STORAGE_SIZE 144 -#endif struct vfp_sigframe {unsigned long magic;unsigned long size; -union vfp_state storage; -}; +struct user_vfp ufp; +struct user_vfp_exc ufp_exc; +} __attribute__((__aligned__ (8 ))); + +/* 8 bytes for magic and size, 264 bytes for ufp, 16 bytes for ufp_exc. */ +#define VFP_STORAGE_SIZE sizeof(struct vfp_sigframe) +#endif /* CONFIG_VFP */ /*
@@ -91,7 +87,7 @@ struct aux_sigframe {#ifdef CONFIG_IWMMXT struct iwmmxt_sigframe iwmmxt;#endif -#if 0 && definedCONFIG_VFP /* Not yet saved. */ +#ifdef CONFIG_VFP struct vfp_sigframe vfp;#endif /* Something that isn't a valid magic number for any coprocessor. */
arch/arm/include/asm/user.h +13 -1
@@ -83,11 +83,23 @@ struct user{/* * User specific VFP registers. If only VFPv2 is present, registers 16 to 31 -* are ignored by the ptrace system call. +* are ignored by the ptrace system call and the signal handler. */ struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; +unsigned long pad; +}; + +/* +* VFP exception registers exposed to user space during signal delivery. +* Fields not relavant to the current VFP architecture are ignored. +*/ +struct user_vfp_exc { +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2; +unsigned long pad; };#endif /* _ARM_USER_H */
v5 cover ARM: VFP: Save / restore VFP state on the signal handler path
Quoted 12 lines · Imre Deak
Quoted 10 lines · ext Jamie Lokier
Quoted 7 lines · imre.deak at nokia.com
From: Imre Deak <[email protected]>
Changes since v4:
- Do not add the packed attribute touser_vfporuser_vfp_exc. I added packed to these structs to prevent the extra padding invfp_sigframe. But actually the padding is in accordance with the ARM ABI.EABI, OABI or both?
Quoted 7 lines
I checked only EABI, which specifies alignment to the struct's alignment, which is 8 bytes for struct
user_vfp.I haven't found an official OABI spec, but some googling turned up that there the requirement is only to align to word size, so we'd have a difference in the struct vfp_sigframe's size and the alignment of the
user_vfp_excwithin.
I think that VFP is defined for EABI only anyway. I doubt people are using VFP with OABI in practice either.
Nicolas
v4 cover ARM: VFP: Save / restore VFP state on the signal handler path
In reply to imre.deak at nokia.comQuoted 11 lines · imre.deak at nokia.com
Changes since v3:
- Rebased against mainline v2.6.34-rc3, since some parts have already been applied there.Note that this problem might affect more people than previously thought. The CodeSourcery 2009q1-203
glibc_portshas some NEON optimized functions; for example using memset or memcpy in a signal handler and floating point in the main thread can corrupt the main thread's context. You can test this by replacing the floating point ops in the signal handler below with some dummy memset/memcpy.
Quoted 3 lines
I could only test these patches on a Cortex A8 UP system; it would be great if someone with an SMP board could give it a go. The test app to reproduce the problem (I've built it with -mfloat-abi=softfp -mfpu=neon):
Do you think this is relevant for ARM11 SMP (ARM11 MPCore), too? Or do you think this is necessary only for NEON systems, i.e. Cortex Ax?
Thanks and best regards
Dirk
v4 cover ARM: VFP: Save / restore VFP state on the signal handler path
Quoted 12 lines · ext Dirk Behme
Quoted 11 lines · imre.deak at nokia.com
Changes since v3:
- Rebased against mainline v2.6.34-rc3, since some parts have already been applied there.Note that this problem might affect more people than previously thought. The CodeSourcery 2009q1-203
glibc_portshas some NEON optimized functions; for example using memset or memcpy in a signal handler and floating point in the main thread can corrupt the main thread's context. You can test this by replacing the floating point ops in the signal handler below with some dummy memset/memcpy.
Quoted 6 lines
Quoted 3 lines
I could only test these patches on a Cortex A8 UP system; it would be great if someone with an SMP board could give it a go. The test app to reproduce the problem (I've built it with -mfloat-abi=softfp -mfpu=neon):
Do you think this is relevant for ARM11 SMP (ARM11 MPCore), too? Or do you think this is necessary only for NEON systems, i.e. Cortex Ax?
I think ARM11 has VFP, so it is relevant for it. This actually means that it's enough to provide -mfpu=vfp above..
--Imre
v4 cover ARM: VFP: Save / restore VFP state on the signal handler path
Quoted 6 lines · Imre Deak
Quoted 2 lines · ext Dirk Behme
Do you think this is relevant for ARM11 SMP (ARM11 MPCore), too? Or do you think this is necessary only for NEON systems, i.e. Cortex Ax?
I think ARM11 has VFP, so it is relevant for it. This actually means that it's enough to provide -mfpu=vfp above..
I know of at least one MPCore (ARM11) SoC where the VFP design option was not enabled. Keep that in mind.
RFC PATCH v4 1/3 ARM: VFP: fix the SMP versions of vfp_{sync, flush}_hwstate
In reply to imre.deak at nokia.comFrom: Imre Deak <[email protected]>
Recently the UP versions of these functions were refactored and as a side effect it became possible to call them for the current thread. This isn't true for the SMP versions however, so fix this up.
--- arch/arm/vfp/vfpmodule.c | 31 ++++++++++--------------------- 1 files changed, 10 insertions(+), 21 deletions(-)
arch/arm/vfp/vfpmodule.c +10 -21
@@ -428,26 +428,6 @@ static void vfp_pm_init(void)static inline void vfp_pm_init (void ) { }#endif /* CONFIG_PM */ -/* -* Synchronise the hardware VFP state of a thread other than current with the -* saved one. This function is used by the ptrace mechanism. -*/ -#ifdef CONFIG_SMP -void vfp_sync_hwstate (struct thread_info *thread) -{ -} - -void vfp_flush_hwstate (struct thread_info *thread) -{ -/* -* On SMP systems, the VFP state is automatically saved at every -* context switch. We mark the thread VFP state as belonging to a -* non-existent CPU so that the saved one will be reloaded when -* needed. -*/ - thread->vfpstate.hard.cpu =NR_CPUS ; -} -#else void vfp_sync_hwstate (struct thread_info *thread) {unsigned int cpu =get_cpu ();
@@ -490,9 +470,18 @@ void vfp_flush_hwstate(struct thread_info *thread) last_VFP_context[cpu] =NULL ; } +#ifdef CONFIG_SMP +/* +* For SMP we still have to take care of the case where the thread +* migrates to another CPU and then back to the original CPU on which +* the last VFP user is still the same thread. Mark the thread VFP +* state as belonging to a non-existent CPU so that the saved one will +* be reloaded in the above case. +*/ + thread->vfpstate.hard.cpu =NR_CPUS ; +#endif put_cpu (); } -#endif #include <linux/smp.h>
Signature · 1 line
1.7.0.2
RFC PATCH v4 2/3 ARM: VFP: make user_vfp struct packed
In reply to imre.deak at nokia.comFrom: Imre Deak <[email protected]>
An upcoming patch will include user_vfp as a member in another struct and this will cause the next member to be 8 byte aligned as well. Since the struct is part of an ABI it's better not leave such alignment decisions to the compiler.
--- arch/arm/include/asm/user.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/include/asm/user.h +1 -1
@@ -88,6 +88,6 @@ struct user{struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; -}; +} __attribute__((packed));#endif /* _ARM_USER_H */
Signature · 1 line
1.7.0.2
RFC PATCH v4 3/3 ARM: VFP: preserve the HW context when calling signal handlers
In reply to imre.deak at nokia.comFrom: Imre Deak <[email protected]>
Signal handlers can use floating point, so prevent them to corrupt the main thread's VFP context. So far there were two signal stack frame formats defined based on the VFP implementation, but the user struct used for ptrace covers all posibilities, so use it for the signal stack too.
Introduce also a new user struct for VFP exception registers. In this too fields not relevant to the current VFP architecture are ignored.
Support to save / restore the exception registers was added by Will Deacon.
--- arch/arm/include/asm/ucontext.h | 20 +++----- arch/arm/include/asm/user.h | 12 +++++- arch/arm/kernel/signal.c | 93 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 108 insertions(+), 17 deletions(-)
arch/arm/include/asm/ucontext.h +8 -12
@@ -59,23 +59,19 @@ struct iwmmxt_sigframe {#endif /* CONFIG_IWMMXT */ #ifdef CONFIG_VFP -#if __LINUX_ARM_ARCH__ <6 -/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra -* word after the registers, and a word of padding@the end for -* alignment. */ #define VFP_MAGIC 0x56465001 -#define VFP_STORAGE_SIZE 152 -#else -#define VFP_MAGIC 0x56465002 -#define VFP_STORAGE_SIZE 144 -#endif struct vfp_sigframe {unsigned long magic;unsigned long size; -union vfp_state storage; -}; +struct user_vfp ufp; +struct user_vfp_exc ufp_exc; +} __attribute__((__aligned__ (8 ))); + +/* 8 byte for magic and size, 272 byte for ufp */ +#define VFP_STORAGE_SIZE sizeof(struct vfp_sigframe) +#endif /* CONFIG_VFP */ /*
@@ -91,7 +87,7 @@ struct aux_sigframe {#ifdef CONFIG_IWMMXT struct iwmmxt_sigframe iwmmxt;#endif -#if 0 && definedCONFIG_VFP /* Not yet saved. */ +#ifdef CONFIG_VFP struct vfp_sigframe vfp;#endif /* Something that isn't a valid magic number for any coprocessor. */
arch/arm/include/asm/user.h +11 -1
@@ -83,11 +83,21 @@ struct user{/* * User specific VFP registers. If only VFPv2 is present, registers 16 to 31 -* are ignored by the ptrace system call. +* are ignored by the ptrace system call and the signal handler. */ struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; } __attribute__((packed)); +/* +* VFP exception registers exposed to user space during signal delivery. +* Fields not relavant to the current VFP architecture are ignored. +*/ +struct user_vfp_exc { +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2; +} __attribute__((packed)); +#endif /* _ARM_USER_H */
arch/arm/kernel/signal.c +89 -4
117 lines not on this page · raw message
Signature · 1 line
1.7.0.2
RFC PATCH v3 1/3 ARM: VFP: add support to sync the VFP state of the current thread
In reply to Imre DeakFrom: Imre Deak <[email protected]>
So far vfp_sync_state worked only for threads other than the current one. This worked for tracing other threads, but not for PTRACE_TRACEME. Syncing for the current thread will also be needed by an upcoming patch adding support for VFP context save / restore around signal handlers.
Also break the current vfp_sync_state into vfp_{save/restore}_hwstate to make things more efficient.
Based on a patch from Russell King.
--- arch/arm/include/asm/thread_info.h | 3 +- arch/arm/kernel/ptrace.c | 6 ++- arch/arm/vfp/vfpmodule.c | 71 +++++++++++++++++++----------------- 3 files changed, 44 insertions(+), 36 deletions(-)
arch/arm/include/asm/thread_info.h +2 -1
@@ -115,7 +115,8 @@ extern void iwmmxt_task_restore(struct thread_info *, void *);extern void iwmmxt_task_release (struct thread_info *);extern void iwmmxt_task_switch (struct thread_info *); -extern void vfp_sync_state (struct thread_info *thread); +extern void vfp_save_hwstate (struct thread_info *); +extern void vfp_restore_hwstate (struct thread_info *);#endif
arch/arm/kernel/ptrace.c +4 -2
@@ -669,7 +669,7 @@ static int ptrace_getvfpregs(struct task_struct *tsk, void __user *data)union vfp_state *vfp = &thread->vfpstate;struct user_vfp __user *ufp = data; -vfp_sync_state (thread); +vfp_save_hwstate (thread);/* copy the floating point registers */ if (copy_to_user(&ufp->fpregs, &vfp->hard.fpregs,
@@ -692,7 +692,7 @@ static int ptrace_setvfpregs(struct task_struct *tsk, void __user *data)union vfp_state *vfp = &thread->vfpstate;struct user_vfp __user *ufp = data; -vfp_sync_state (thread); +vfp_save_hwstate (thread);/* copy the floating point registers */ if (copy_from_user(&vfp->hard.fpregs, &ufp->fpregs,
@@ -703,6 +703,8 @@ static int ptrace_setvfpregs(struct task_struct *tsk, void __user *data)if (get_user (vfp->hard.fpscr, &ufp->fpscr))return -EFAULT ; +vfp_restore_hwstate (thread); + return 0; }#endif
arch/arm/vfp/vfpmodule.c +38 -33
92 lines not on this page · raw message
Signature · 1 line
1.7.0.2
RFC PATCH v3 2/3 ARM: VFP: make user_vfp struct packed
In reply to Imre DeakFrom: Imre Deak <[email protected]>
An upcoming patch will include user_vfp as a member in another struct and this will cause the next member to be 8 byte aligned as well. Since the struct is part of an ABI it's better not leave such alignment decisions to the compiler.
--- arch/arm/include/asm/user.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/include/asm/user.h +1 -1
@@ -88,6 +88,6 @@ struct user{struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; -}; +} __attribute__((packed));#endif /* _ARM_USER_H */
Signature · 1 line
1.7.0.2
RFC PATCH v3 3/3 ARM: VFP: preserve the HW context when calling signal handlers
In reply to Imre DeakFrom: Imre Deak <[email protected]>
Signal handlers can use floating point, so prevent them to corrupt the main thread's VFP context. So far there were two signal stack frame formats defined based on the VFP implementation, but the user struct used for ptrace covers all posibilities, so use it for the signal stack too.
Introduce also a new user struct for VFP exception registers. In this too fields not relevant to the current VFP architecture are ignored.
Support to save / restore the exception registers was added by Will Deacon.
--- arch/arm/include/asm/ucontext.h | 20 +++----- arch/arm/include/asm/user.h | 12 +++++- arch/arm/kernel/signal.c | 93 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 108 insertions(+), 17 deletions(-)
arch/arm/include/asm/ucontext.h +8 -12
@@ -59,23 +59,19 @@ struct iwmmxt_sigframe {#endif /* CONFIG_IWMMXT */ #ifdef CONFIG_VFP -#if __LINUX_ARM_ARCH__ <6 -/* For ARM pre-v6, we use fstmiax and fldmiax. This adds one extra -* word after the registers, and a word of padding@the end for -* alignment. */ #define VFP_MAGIC 0x56465001 -#define VFP_STORAGE_SIZE 152 -#else -#define VFP_MAGIC 0x56465002 -#define VFP_STORAGE_SIZE 144 -#endif struct vfp_sigframe {unsigned long magic;unsigned long size; -union vfp_state storage; -}; +struct user_vfp ufp; +struct user_vfp_exc ufp_exc; +} __attribute__((__aligned__ (8 ))); + +/* 8 byte for magic and size, 272 byte for ufp */ +#define VFP_STORAGE_SIZE sizeof(struct vfp_sigframe) +#endif /* CONFIG_VFP */ /*
@@ -91,7 +87,7 @@ struct aux_sigframe {#ifdef CONFIG_IWMMXT struct iwmmxt_sigframe iwmmxt;#endif -#if 0 && definedCONFIG_VFP /* Not yet saved. */ +#ifdef CONFIG_VFP struct vfp_sigframe vfp;#endif /* Something that isn't a valid magic number for any coprocessor. */
arch/arm/include/asm/user.h +11 -1
@@ -83,11 +83,21 @@ struct user{/* * User specific VFP registers. If only VFPv2 is present, registers 16 to 31 -* are ignored by the ptrace system call. +* are ignored by the ptrace system call and the signal handler. */ struct user_vfp {unsigned long long fpregs[32 ];unsigned long fpscr; } __attribute__((packed)); +/* +* VFP exception registers exposed to user space during signal delivery. +* Fields not relavant to the current VFP architecture are ignored. +*/ +struct user_vfp_exc { +unsigned long fpexc; +unsigned long fpinst; +unsigned long fpinst2; +} __attribute__((packed)); +#endif /* _ARM_USER_H */
arch/arm/kernel/signal.c +89 -4
117 lines not on this page · raw message
Signature · 1 line
1.7.0.2