[PATCH] ARM: vfp: use asm volatile for FP control register accesses

[Nathan Chancellor]

On Wed, Mar 27, 2024 at 09:05:17AM +0200, Ard Biesheuvel wrote:
> On Wed, 27 Mar 2024 at 01:55, Nathan Chancellor <nathan at kernel.org> wrote:
> >
> > On Mon, Mar 18, 2024 at 10:30:05AM +0100, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb at kernel.org>
> > >
> > > Clang may reorder FP control register reads and writes, due to the fact
> > > that the inline asm() blocks in the read/write wrappers are not volatile
> > > qualified, and the compiler has no idea that these reads and writes may
> > > have side effects.
> > >
> > > In particular, reads of FPSCR may generate an UNDEF exception if a
> > > floating point exception is pending, and the FP emulation code in
> > > VFP_bounce() explicitly clears FP exceptions temporarily in order to be
> > > able to perform the emulation on behalf of user space. This requires
> > > that the writes to FPEXC are never reordered with respect to accesses to
> > > other FP control registers, such as FPSCR.
> > >
> > > So use asm volatile for both the read and the write helpers.
> > >
> > > Cc: <stable at kernel.org>
> > > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> >
> > This seems reasonable to me based on my understanding of GCC's
> > documentation. However, their documentation states "the compiler can
> > move even volatile asm instructions relative to other code, including
> > across jump instructions" and I feel like there was some discussion
> > around this sentence in the past but I can't remember what the
> > conclusion was, although I want to say Clang did not have the same
> > behavior.
> 
> The only thing that matters here is whether two asm blocks are emitted
> in a different order than they appear in the program, and I would be
> very surprised if volatile permits that. Otherwise, we might introduce
> a fake input dependency or a memory clobber instead.

Yeah, I think it is reasonable to go with this approach and fall back to
your follow up suggestion if this proves not to be robust enough.

> > Regardless:
> >
> > Acked-by: Nathan Chancellor <nathan at kernel.org>
> >
> 
> Thanks.
> 
> > I am just curious, how was this discovered or noticed? Was there a
> > report I missed?
> >
> 
> I noticed this when building a recent kernel for the original
> Raspberry Pi, which is ARMv6 not ARMv7, and has a VFP which partially
> relies on emulation. On more recent cores, we never hit the issue
> because emulation is never needed. On older cores, there is no VFP so
> we never reach this code path either.

Ah, good to know. I boot test -next on a Pi 3 with a 32-bit kernel, so
that explains why I have not seen any issues. Just wanted to make sure I
did not miss something :)

Cheers,
Nathan