Page MenuHomeFreeBSD

Only mess with VFP state on the CPU for curthread for get/set_vfpcontext.
ClosedPublic

Authored by jhb on Sep 9 2017, 2:35 PM.

Details

Summary

Future changes will use these functions to fetch and store VFP state for
threads other than curthread.

Test Plan

This is part of a patch series for using VFP with gdb on armv7.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb created this revision.Sep 9 2017, 2:35 PM
emaste added a subscriber: emaste.Sep 9 2017, 2:39 PM
stevek accepted this revision.Sep 9 2017, 3:03 PM
stevek added a reviewer: stevek.

Looks good.

This revision is now accepted and ready to land.Sep 9 2017, 3:04 PM
mmel accepted this revision.Sep 10 2017, 5:32 AM
andrew added a subscriber: andrew.Sep 12 2017, 7:56 AM

I have a slight concern about a non-current thread being run at the same time as this leading to a potential data race.

sys/arm/arm/machdep.c
419–421 ↗(On Diff #32848)

Should the critical section be reduced? What about if td was scheduled during this memcpy & updated its vfp registers?

jhb added inline comments.Sep 12 2017, 7:00 PM
sys/arm/arm/machdep.c
419–421 ↗(On Diff #32848)

If it is curthread then all it will do if it gets preempted and rescheduled is then execute this memcpy(). If it isn't curthread, the thread is stopped (either due to a ptrace() stop or because we are single-threaded to write out a core dump).

andrew added inline comments.Sep 12 2017, 10:53 PM
sys/arm/arm/machdep.c
419–421 ↗(On Diff #32848)

Is there a way to assert the thread is stopped?

jhb added inline comments.Sep 12 2017, 11:12 PM
sys/arm/arm/machdep.c
419–421 ↗(On Diff #32848)

Possibly? We don't do this on any other architecture for fill_*regs though, it's just part of the API contract. (This is equivalent in nature to the patch I committed for arm64 to fix panics for PT_GETFPREGS a few weeks ago.)

andrew accepted this revision.Sep 13 2017, 4:47 PM
jhb updated this revision to Diff 33042.Sep 14 2017, 12:10 AM
  • Assert threads are suspended for get/set_vfpcontext.
This revision now requires review to proceed.Sep 14 2017, 12:10 AM
This revision was automatically updated to reflect the committed changes.