From: "Theo de Raadt" Subject: Re: amd64/fpu: Avoid multiple FPU resets Cc: Mark Kettenis , Philip Guenther , cludwig@genua.de, tech@openbsd.org Date: Fri, 13 Jun 2025 18:49:12 -0600 Theo de Raadt wrote: > Mark Kettenis wrote: > > > > From: Philip Guenther > > > Date: Fri, 13 Jun 2025 11:37:03 -0700 > > > > > > On Fri, Jun 13, 2025 at 5:32 AM Christian Ludwig wrote: > > > > All kernel crypto code follows the scheme: > > > > > > > > for (objects) { > > > > fpu_kernel_enter(); > > > > ... > > > > fpu_kernel_exit(); > > > > } > > > > > > > > In every iteration, fpu_kernel_exit() resets the FPU state and > > > > fpu_kernel_enter() resets it, again. FPU resets are expensive on some > > > > platforms. Doing the operation twice per loop iteration is clearly not > > > > necessary. > > > > > > > > The FPU is always in one of two states when we reach fpu_kernel_enter(). > > > > It either holds user state when CPUPF_USERXSTATE is set or it is in > > > > reset state already. The context switching code and signal code follow > > > > this assumption, too. So we can simply drop resetting the FPU in > > > > fpu_kernel_enter() when it does not hold user state. > > > > > > Hmm, yes, all the places that clear CPUF_USERXSTATE reset the state. > > > > > > Does fpu_kernel_enter() get used from interrupt context? Do we have > > > to worry about an interrupt occurring between the clearing of the flag > > > and the resetting of the state? > > > > I'm pretty sure the intention was that you can onlu use > > fpu_kernel_enter() from process context. But I don't think we > > documented this anywhere and we don't enforce this. Should we stick > > an assertwaitok() in there? > > My point is has this covered pu some bugs. My point is has this covered up some bugs? [that is a question] Do we want to turn this into > an assert for a little while, then delete the assert when confidence. > The failure more is pretty drastic and undebuggable. OTOH, we are pretty > bad about removing unneccessary assert later on. >