On 02/05/2019 10:10 AM, Michael Ellerman wrote:
Christophe Leroy <christophe.le...@c-s.fr> writes:
Le 20/12/2018 à 23:35, Benjamin Herrenschmidt a écrit :

    /*
     * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
@@ -205,20 +208,46 @@ transfer_to_handler_cont:
        mflr    r9
        lwz     r11,0(r9)               /* virtual address of handler */
        lwz     r9,4(r9)                /* where to go when done */
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
+       mtspr   SPRN_NRI, r0
+#endif

That's not part of your patch, it's already in the tree.

Yup rebase glitch.

   .../...

I tested it on the 8xx with the below changes in addition. No issue seen
so far.

Thanks !

I'll merge that in.

I'm currently working on a refactorisation and simplification of
exception and syscall entry on ppc32.

I plan to take your patch in my serie as it helps quite a bit. I hope
you don't mind. I expect to come out with a series this week.

Ben's AFK so go ahead and pull it in to your series if that helps you.
The main obscure area is that business with the irqsoff tracer and thus
the need to create stack frames around calls to trace_hardirqs_* ... we
do it in some places and not others, but I've not managed to make it
crash either. I need to get to the bottom of that, and possibly provide
proper macro helpers like ppc64 has to do it.

I can't see anything special around this in ppc32 code. As far as I
understand, a stack frame is put in place when there is a need to
save and restore some volatile registers. At the places where nothing
needs to be saved, nothing is done. I think that's the normal way for
any function call, isn't it ?

The concern was that the irqsoff tracer was doing
__builtin_return_address(1) (or some number > 0) and that crashes if
there aren't sufficiently many stack frames available.

See ftrace_return_address.

Possibly the answer is that we don't have CONFIG_FRAME_POINTER and so we
get the empty version of that.


Yes indeed, ftrace_return_address(1) is not __builtin_return_address(1) but 0ul as CONFIG_FRAME_POINTER is not defined. So the crash can't be due to that as it would then crash regardless of whether we set a stack frame or not. And anyway, as far as I understand, if the stack is properly initialised, __builtin_return_address(X) returns NULL and don't crash when the top of backtrace is reached.

Do you have more details about the said crash ? I think we should file an issue for it in our issue databse.

For the time being, I'll get rid of that unneccessary stack frame in entry_32.S as part of my syscall prolog optimising series.

Christophe

Reply via email to