Page MenuHomeFreeBSD

LinuxKPI: Add explicit software context to FPU sections
ClosedPublic

Authored by wulf on Nov 28 2023, 9:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 14, 2:45 PM
Unknown Object (File)
Wed, May 8, 11:44 AM
Unknown Object (File)
Wed, May 8, 11:44 AM
Unknown Object (File)
Wed, May 8, 11:44 AM
Unknown Object (File)
Wed, May 8, 11:44 AM
Unknown Object (File)
Wed, May 8, 11:44 AM
Unknown Object (File)
Wed, May 8, 9:36 AM
Unknown Object (File)
Thu, May 2, 10:32 PM

Details

Summary

Amdgpu driver does a lot of memory allocations in FPU-protected sections
of code for certain display cores, e.g. for DCN30. This does not work
currently on FreeBSD as its malloc function can not be run within a
critical section. Allocate memory for FPU context to overcome such
restriction.

Sponsored by: Serenity Cyber Security, LLC
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

wulf requested review of this revision.Nov 28 2023, 9:37 PM
This revision is now accepted and ready to land.Nov 29 2023, 6:20 AM

Well, we support using fpu_kern_enter() with an explicit context, and then the FPU section will not have such restrictions. That is, you can embed a struct fpu_kern_ctx in the task_struct and call fpu_kern_enter(... FPU_KERN_KTHR) when fpu_ctx_level transitions from 0 to 1. This kind of hack wouldn't be necessary then.

sys/compat/linuxkpi/common/src/linux_slab.c
222 ↗(On Diff #130708)

Rather than fiddling with linuxkpi fpu state here, IMO it would be clear to introduce some helper function in linux_fpu.c to do it.

wulf retitled this revision from LinuxKPI: Allow kmalloc to be called when FPU protection is enabled to LinuxKPI: Add explicit software context to FPU sections.
wulf edited the summary of this revision. (Show Details)

Added explicit software context to FPU sections

This revision now requires review to proceed.Dec 17 2023, 10:51 AM
sys/compat/linuxkpi/common/include/linux/compat.h
36

BTW, 32-bit arm has this function now.

sys/compat/linuxkpi/common/src/linux_current.c
160
168

IMO it is better to always specify NOWAIT rather than trying to guess whether the caller can sleep. This check is incomplete: for instance, if the thread holds a mtx(9), it may not perform a M_WAITOK allocation, but THREAD_CAN_SLEEP() may still return true. THREAD_CAN_SLEEP() is mostly useful for assertions.

Is there some code which enters an FPU section from an ithread? Is it possible to perform the allocation with M_WAITOK if the thread is not an ithread, and use FPU_KERN_NOCTX if the current thread is an ithread?

Always use NOWAIT
Add arm and powerpc64 to supported archs
Don't allocate FPU context in critical sections

This revision is now accepted and ready to land.Dec 20 2023, 3:09 PM
sys/compat/linuxkpi/common/src/linux_current.c
168

Is there some code which enters an FPU section from an ithread? Is it possible to perform the allocation with M_WAITOK if the thread is not an ithread, and use FPU_KERN_NOCTX if the current thread is an ithread?

I do not know. Most probably, no. I tracked origin of couple FPU usages and it appeared to be device attach and resume handlers.

sys/compat/linuxkpi/common/src/linux_current.c
168

Ok. Well, here is one more design which might be more robust:

  • At linuxkpi initialization time, allocate one ctx and one mutex. Could also be one per CPU or some similar scheme if we are worried about lock contention.
  • To enter an FPU section, acquire the lock and call fpu_kern_enter() with the context.
  • To exit the section, call fpu_kern_leave() and release the lock.

Then you do not need to lazily allocate fpu contexts.