From: Mark Kettenis Subject: Re: amd64/fpu: Avoid multiple FPU resets To: Mark Kettenis Cc: deraadt@openbsd.org, guenther@gmail.com, cludwig@genua.de, tech@openbsd.org Date: Mon, 16 Jun 2025 17:55:29 +0200 > Date: Sun, 15 Jun 2025 18:35:43 +0200 > From: Mark Kettenis > > > From: "Theo de Raadt" > > Date: Sun, 15 Jun 2025 10:16:57 -0600 > > > > Philip Guenther wrote: > > > > > Okay, then I think the original diff is correct, though maybe we add > > > the assertwaitok() to fpu_kernel_enter() so we remain confident of > > > that. > > > > If an assert triggers at this point, in drm code, it is unlikely that > > a user will see it. I expect it will deadlock the kernel hard in a > > very invisible fashion... > > Well, assertwaitok() does a few checks. The first one is: > > splassert(IPL_NONE); > > This is the check we want to make sure that we're not called from > interrupt context. By default, this just prints a warning message, > which shouldn't cause a deadlock. > > There are other checks that may trigger a panic and therefore may > appear to the user as a deadlock. So I think it is better to just add > an splassert(IPL_NONE) into fpu_kernel_enter(). So like this. Should this go into snaps? Should I just commit it? Index: arch/amd64/amd64/fpu.c =================================================================== RCS file: /cvs/src/sys/arch/amd64/amd64/fpu.c,v diff -u -p -r1.44 fpu.c --- arch/amd64/amd64/fpu.c 22 May 2023 00:39:57 -0000 1.44 +++ arch/amd64/amd64/fpu.c 16 Jun 2025 15:51:15 -0000 @@ -146,6 +146,8 @@ fpu_kernel_enter(void) { struct cpu_info *ci = curcpu(); + splassert(IPL_NONE); + /* save curproc's FPU state if we haven't already */ if (ci->ci_pflags & CPUPF_USERXSTATE) { ci->ci_pflags &= ~CPUPF_USERXSTATE;