Don't panic for PT_GETFPREGS.

[jhb]

Summary

Only fetch the VFP state from the CPU if the thread whose registers are being requested is
the current thread. If a stopped thread's registers are being fetched by a debugger, the
saved state in the PCB is already valid.

Test Plan

• info registers in gdb panics a debug kernel when PT_GETFPGREGS is invoked on a non-running thread.

[andrew]

Where is this being called from? I think the bug is calling vfp_save_state on a non-current thread.

[jhb]

fill_fpregs(). Once a thread uses the FPU once the "started" flag is set but never cleared. The approach in the patch matches the behavior of sys/amd64/amd64/fpu.c:fpugetregs().

One thing I'm not quite certain amd64 does is if the thread is suspended in ptracestop(), is the FPU state "flushed" out or could it be sitting in some other CPU that the debugger isn't running on when it calls ptrace(). In that case amd64 would return stale info. In some ways I think amd64 would be better off not doing lazy save/restore of the FPU state during context switches as that would solve that.

[andrew]

On arm64 if a thread has used the vfp unit we will store the state on context switch. This means we should only need to call vfp_save_state when we need access to the fpu registers from the current thread. I don't think fill_fpregs has been tested as lldb didn't have vfp support when we did the initial port.

sys/arm64/arm64/vfp.c

208 ↗

(On Diff #31909)

Feel free to commit the comment fix.

[jhb]

Ok. So would you rather me change fill_regs() to only call vfp_save_state() if td == curthread? Also, it seems we could at that point make vfp_save_state() assume curthread the way vfp_restore_state() assumes curthread.

[andrew]

Yes. I'm not sure why I made the save KPI look like it does. I think curthread is correct at the point it's called in cpu_switch(), however it would pay to check that.

[jhb]

Make the vfp_save_state() call conditional instead.

[jhb]

I didn't change the vfp_save_state() API yet. curthread is correct in the cpu_switch case at least, and savectx() is what passes NULL as the thread pointer. I think it would be safe to have it work like vfp_restore_state() and assume curpcb / curthread.

[jhb]

Committed in rS322436.