Details

Reviewers

andrew
jhb
jrtc27
mmel
manu

Commits

rG55872d471d05: Arch64: Clear VFP state on execve()
rG953a7d7c61f3: Arch64: Clear VFP state on execve()

Summary

I noticed that many of the math-related tests were failing on AArch64.
After a lot of debugging, I noticed that the floating point exception flags
were not being reset when starting a new process. This change resets the
VFP inside exec_setregs() to ensure no VFP register state is leaked from
parent processes to children.

This commit also moves the clearing of fpcr that was added in 65618fdda0f27
from fork() to execve() since that makes more sense: fork() can retain
current register values, but execve() should result in a well-defined
clean state.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37573
Build 34462: arc lint + arc unit

Event Timeline

arichardson created this revision.Mar 4 2021, 3:08 PM

Herald added a reviewer: manu. · View Herald TranscriptMar 4 2021, 3:08 PM

Herald added subscribers: emaste, imp. · View Herald Transcript

arichardson requested review of this revision.Mar 4 2021, 3:08 PM

Harbormaster completed remote builds in B37573: Diff 85138.Mar 4 2021, 3:08 PM

arichardson mentioned this in rG0072e5e0f3a0: sys/arm64/arm64/vfp.c: Fix -Wunused and -Wpointer-sign warnings.Mar 4 2021, 6:45 PM

Also reset VFP for LInux and FreeBSD32 processes
Drop unrelated changes

Harbormaster completed remote builds in B37586: Diff 85164.Mar 4 2021, 8:01 PM

arichardson edited the summary of this revision. (Show Details)Mar 4 2021, 8:01 PM

andrew added inline comments.Mar 5 2021, 8:42 AM

sys/arm64/arm64/machdep.c
557	This should be in `vfp.c` and should probably call `vfp_discard` to ensure the hardware state is updated if it's enabled.

arichardson added inline comments.Mar 5 2021, 9:58 AM

sys/arm64/arm64/machdep.c
557	I just tried moving it to vfp.c but that makes things really ugly if I want to handle !VFP cases. Can't I just add #ifdef VFP vfp_discard(td); #endif to the end of this function?

Move it to vfp.c

Harbormaster completed remote builds in B37593: Diff 85185.Mar 5 2021, 10:06 AM

andrew added inline comments.Mar 5 2021, 12:23 PM

sys/arm64/arm64/vfp.c
392	I don't think this is needed in the `!VFP` case. It resets values that should only ever be used when the `VFP` is present and the kernel is built to use it. If this is the case it could be renamed to `vfp_reset_state` and moved to be near `vfp_save_state`.
402	If an interrupt that causes the thread to switch to a new CPU we could end up with an inconsistent state. The simplest fix is to move this `critical_enter` call to above the `bzero`. I think it would also work to call `vfp_discard` first, then clear the state as this will ensure the VFP unit is off.