[PATCH v4 2/2] ARM: vfp: clear fpscr length and stride bits on entry to sig handler

[Will Deacon]

On Mon, May 14, 2012 at 05:02:59PM +0100, Jon Medhurst (Tixy) wrote:
> On Mon, 2012-05-14 at 15:59 +0100, Will Deacon wrote:
> > The idea is that we disable VFP in hardware (via vfp_flush_hwstate) prior to
> > entering the signal handler. Then we clear hwstate->fpexc.FPEXC_EN so that
> > we can detect if VFP gets used during the handler, since the exception
> > handling code will set that back up if we trapon a VFP operation (see
> > vfp_support_entry in vfphw.S).
> 
> I had a look at vfp_support_entry. I can see it loading the hardware
> registers from the saved vfp state, but can't see where it updates the
> fpexc value in the saved vfp state. (I also can't think of why it would
> need to, but this is the first time I've looked Linux vfp code so there
> could be lots I've missed.)

You're right, in the lazy case (i.e. SMP) we update the hwstate on
context-switch rather than on the fault. Your quick fix looks good to me
(that is, unconditionally flushing the state after handling a signal). I was
initially worried that the added flushing would cause a performance hit on
the usual path, where VFP is not used in the handler, however since we flush
before the signal handler, it really won't make a lot of difference.

I took the liberty of writing a commit message, so can I add your S-o-B to
this please (I wasn't sure which email address to use)?

Thanks,

Will

Subject: [PATCH] ARM: vfp: fix VFP flushing regression on sigreturn path

Commit ff9a184c ("ARM: 7400/1: vfp: clear fpscr length and stride bits
on entry to sig handler") flushes the VFP state prior to entering a
signal handler so that a VFP operation inside the handler will trap and
force a restore of ABI-compliant registers. Restoring the registers on
the sigreturn path is predicated on the saved thread state indicating
that VFP was used by the handler -- however for SMP platforms this is
only set on context-switch, making the check unreliable and causing VFP
register corruption in userspace.

This patch unconditionally flushes the VFP state after a signal handler.
Since we already perform the flush before the handler and the flushing
itself happens lazily, the effect on performance is negligible.

Reported-by: Jon Medhurst <tixy at yxit.co.uk>
Signed-off-by: Will Deacon <will.deacon at arm.com>
---
 arch/arm/vfp/vfpmodule.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index c5767b5..b0197b2 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -577,12 +577,6 @@ int vfp_preserve_user_clear_hwstate(struct user_vfp __user *ufp,
 	 * entry.
 	 */
 	hwstate->fpscr &= ~(FPSCR_LENGTH_MASK | FPSCR_STRIDE_MASK);
-
-	/*
-	 * Disable VFP in the hwstate so that we can detect if it gets
-	 * used.
-	 */
-	hwstate->fpexc &= ~FPEXC_EN;
 	return 0;
 }
 
@@ -595,12 +589,8 @@ int vfp_restore_user_hwstate(struct user_vfp __user *ufp,
 	unsigned long fpexc;
 	int err = 0;
 
-	/*
-	 * If VFP has been used, then disable it to avoid corrupting
-	 * the new thread state.
-	 */
-	if (hwstate->fpexc & FPEXC_EN)
-		vfp_flush_hwstate(thread);
+	/* Disable VFP to avoid corrupting the new thread state. */
+	vfp_flush_hwstate(thread);
 
 	/*
 	 * Copy the floating point registers. There can be unused
-- 
1.7.4.1