Am 18.07.2021 um 08:09 schrieb Michael Schmitz:
> Hi Eric,
>
> Am 18.07.2021 um 06:52 schrieb Eric W. Biederman:
>>> I should have looked more closely at skeleton.S - most FPU exceptions
>>> handled there call trap_c the same way as is done for generic traps,
>>> i.e. SAVE_ALL_INT before, ret_from_exception after.
>>>
>>> Instead of adding code to entry.S, much better to add it in
>>> skeleton.S. I'll try to come up with a way to test this code path
>>> (calling fpsp040_die from the dz exception hander seems much the
>>> easiest way) to make sure this doesn't have side effects.
>>>
>>> Does do_exit() ever return?
>>
>> No.  The function do_exit never returns.
>
> Fine - nothing to worry about as regards restoring the stack pointer
> correctly then.
>
>> If it is not too much difficulty I would be in favor of having the code
>> do force_sigsegv(SIGSEGV), instead of calling do_exit directly.
>
> That _would_ force a return, right? The exception handling in skeleton.S
> won't be set up for that.

See attached patch - note that when you change fpsp040_die() to call 
force_sig(SIGSEGV), the access exception handler will return to whatever 
function in fpsp040 attempted the user space access, and continue that 
operation with quite likely bogus data. That may well force another FPU 
trap before the SIGSEGV is delivered (will force_sig() immediately force 
a trap, or wait until returning to user space?).

Compile tested - haven't found an easy way to execute that code path yet.

Cheers,

	Michael


>
>> Looking at that code I have not been able to figure out the call paths
>> that get into skeleton.S.  I am not certain saving all of the registers
>> on an the exceptions that reach there make sense.  In practice I suspect
>
> The registers are saved only so trap_c has a stack frame to work with.
> In that sense, adding a stack frame before calling fpsp040_die is no
> different.
>
>> taking an exception is much more expensive than saving the registers
>> so it
>> might not make any difference.  But this definitely looks like code that
>> is performance sensitive.
>
> We're only planning to add a stack frame save before calling out of the
> user access exception handler, right? I doubt that will be called very
> often.
>
>> My sense when I was reading through skeleton.S was just one or two
>> registers were saved before the instruction emulation was called.
>
> skeleton.S only contains the entry points for code to handle FPU
> exceptions, from what I've seen (plus the user space access code).
>
> Wherever that exception handling requires calling into the C exception
> handler (trap_c), a stack frame is added.
>
> Cheers,
>
>     Michael
>
>>