Re: [v3 PATCH 1/1] booke/kprobe: make program exception to use one dedicated exception stack

["tiejun.chen" <tiejun.chen@windriver.com>]

Benjamin Herrenschmidt wrote:
>>> As I understand it, the problem comes from the fact that stwu combines the
>>> creation of a stack frame with storing into that stack frame.  If they were
>> Yes.
>>
>>> separate instructions you'd have a new exception frame at a lower address
>>> by the time you actually store to the non-exception frame.
>> So when kprobe we should use a unique stack frame to skip that stack frame the
>> kprobed stwu want to create.
> 
> I still don't like that patch. Potentially the problem exist for all
> variants of powerpc, not just booke, and I'm not sure I like adding yet

Yes.

> another exception stack.

But I think we should extend easily this for other powerpc variants. And only
when enable CONFIG_KPROBES that dedicated exception stack is valid, so its not
such a big risk :)

> 
> Another (non-great) approach would be to special case stwu to the stack,
> and instead of doing the store while emulating the instruction, keep the
> store address around and do it later, after the stack has been unwound,
> in the exit path (a TIF flag to hit the slow path and then do it in the
> slow path).

Actually I also considered one idea that we do stw-update in the exit path like
your proposal. But I'm not sure if its worth intruding a new TIF flag only for
'stwu'. And if I understand what your exit path means properly, we should do
this on ret_from_except_full,

...
exc_exit_restart:
        lwz     r11,_NIP(r1)
        lwz     r12,_MSR(r1)
	
	Looks we have to add something to update as 'stwu' since _NIP/_MSR are also
corrupted potentially. So I feel we'll make this complicated if we really do here.

exc_exit_start:
        mtspr   SPRN_SRR0,r11
        mtspr   SPRN_SRR1,r12
        REST_2GPRS(11, r1)
        lwz     r1,GPR1(r1)
        .globl exc_exit_restart_end
exc_exit_restart_end:
        PPC405_ERR77_SYNC
        rfi
        b       .                       /* prevent prefetch past rfi */

If I'm wrong please correct me.

> 
> It sounds hackish but it makes it easier to fix everybody at once, there
> are "issues" with changing stacks especially on ppc64 and it would
> definitely be affected as well if the stack frame created is larger than
> our gap.

If we provide another exception stack like we did debug exception on ppc64, are
there those "issues" you said?

Thanks
Tiejun

> 
> Cheers,
> Ben.
> 
>