Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: amd64/fpu: Avoid multiple FPU resets
Cc:
Mark Kettenis <mark.kettenis@xs4all.nl>, Philip Guenther <guenther@gmail.com>, cludwig@genua.de, tech@openbsd.org
Date:
Fri, 13 Jun 2025 18:49:12 -0600

Download raw body.

Thread
Theo de Raadt <deraadt@openbsd.org> wrote:

> Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> 
> > > From: Philip Guenther <guenther@gmail.com>
> > > Date: Fri, 13 Jun 2025 11:37:03 -0700
> > > 
> > > On Fri, Jun 13, 2025 at 5:32 AM Christian Ludwig <cludwig@genua.de> 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.
>