Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully

[Finn Thain <fthain@linux-m68k.org>]

On Sun, 28 Apr 2024, Michael Schmitz wrote:

> In my tests, the fault seen there had been caused by the movesl in the 
> section above, hence the fixup at 20b.
> 

Well, that was my point. A fault at MOVE.W should get fixed up at 20b, not 
50b. So the patch is wrong, isn't it?

> >
> > Should we put a NOP here to avoid having the unknown next instruction 
> > (label 9) in the exception table? We can't actually fix up a fault 
> > there unless by chance it was the MOVES that caused it.
> 
> ... As to the unknown instructions following the final exception label: 
> These functions, though they contain inline assembly code are not 
> further inlined, and the instructions following the inline assembly are 
> simple boilerplate register restore stack saves that ought not to fault 
> (an invalid stack pointer would have faulted on function entry, if at 
> all).
> 

But that's just luck. IMHO, the asm is a foot-gun even if it has not gone 
off yet.

> On balance, I am confident the code is correct as-is. You (and in 
> particular, Geert) may argue though that the NOP approach follows the 
> principle of least surprises, and can be considered safe to apply 
> without further testing on 68060 and Coldfire.
> 

If you're saying that your patch addresses a different bug, fair enough.

All I'm saying is that, since you're adding the NOP anyway, you could make 
better use of it.

Anyway, like you, I am keen to hear from others regarding the API issue.