Re: [PATCH v2] m68k: save extra registers on more syscall entry points

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Linus,

that one boots OK now, thanks (applied on top of mine but that should 
not matter). I'll run a test on the actual hardware once the previous 
one is done.

Cheers,

	Michael


Am 19.06.2021 um 14:13 schrieb Linus Torvalds:
> On Fri, Jun 18, 2021 at 6:54 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> The registers being zero at that point is actually expected, so that's
>> not much of a hint. But yeah, clearly I got some stack initialization
>> offset or something wrong there, and I don't know modern m68k nearly
>> well enough to even guess where I screwed up.
>
> Oh. I think I might have an idea.
>
> In that ret_from_kernel_thread code, after it returns from thge kernel
> thread, I did
>
>         addql   #4,%sp
>         RESTORE_SWITCH_STACK
>         jra     ret_from_exception
>
> where that "RESTORE_SWITCH_STACK" loads the user space registers from
> the user-space switch stack.
>
> BUT.
>
> The switch stack also has that "retpc", and that one should just be popped.
>
> So I think that code should read
>
>         addql   #4,%sp
>         RESTORE_SWITCH_STACK
>         addql   #4,%sp
>         jra     ret_from_exception
>
> because we want to restore the switch stack registers (they'll all be
> 0) but we then want to get rid of retpc too before we jump to
> ret_from_exception, which will do the RESTORE_ALL.
>
> (The first 'addql' is to remove the argument we've pushed on the stack
> earlier in ret_from_kernel_thread, the second addql is to remove
> retpc).
>
>  All the *normal* uses of RESTORE_SWITCH_STACK will just do "rts", but
> that's because they were called from the shallower stack. The
> ret_from_kernel_thread case is kind of special, because it had that
> stack frame layout built up manually, rather than have a call to it.
>
> In that sense, it's a bit more like the 'do_trace_entry/exit' code,
> which builds that switch stack using
>
>         subql   #4,%sp
>         SAVE_SWITCH_STACK
>
> and then undoes it using that same
>
>         RESTORE_SWITCH_STACK
>         addql   #4,%sp
>
> sequence that I _should_ have done in ret_from_kernel_thread. (Also,
> see ret_from_signal)
>
> I've attached an updated patch just in case my incoherent ramblings
> above didn't explain what I meant. It's the same as the previous
> patch, it just has that one extra stack update there.
>
> Does _this_ one perhaps work?
>
>                  Linus
>