Le 08/09/2020 à 09:48, Nicholas Piggin a écrit :
Excerpts from Christophe Leroy's message of September 7, 2020 9:34 pm:
On Mon, 2020-09-07 at 11:20 +0200, Christophe Leroy wrote:

Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
Make interrupt handlers all just take the pt_regs * argument and load
DAR/DSISR etc from that. Make those that return a value return long.

I like this, it will likely simplify a bit the VMAP_STACK mess.

Not sure it is that easy. My board is stuck after the start of init.


On the 8xx, on Instruction TLB Error exception, we do

        andis.  r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */

On book3s/32, on ISI exception we do:
        andis.  r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */

On 40x and bookE, on ISI exception we do:
        li      r5,0                    /* Pass zero as arg3 */


And regs->dsisr will just contain nothing

So it means we should at least write back r5 into regs->dsisr from there
? The performance impact should be minimal as we already write _DAR so
the cache line should already be in the cache.

A hacky 'stw r5, _DSISR(r1)' in handle_page_fault() does the trick,
allthough we don't want to do it for both ISI and DSI at the end, so
you'll have to do it in every head_xxx.S

To get you series build and work, I did the following hacks:

Great, thanks for this.

diff --git a/arch/powerpc/include/asm/interrupt.h
b/arch/powerpc/include/asm/interrupt.h
index acfcc7d5779b..c11045d3113a 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -93,7 +93,9 @@ static inline void interrupt_nmi_exit_prepare(struct
pt_regs *regs, struct inter
  {
        nmi_exit();
+#ifdef CONFIG_PPC64
        this_cpu_set_ftrace_enabled(state->ftrace_enabled);
+#endif

This seems okay, not a hack.

  #ifdef CONFIG_PPC_BOOK3S_64
        /* Check we didn't change the pending interrupt mask. */
diff --git a/arch/powerpc/kernel/entry_32.S
b/arch/powerpc/kernel/entry_32.S
index f4d0af8e1136..66f7adbe1076 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -663,6 +663,7 @@ ppc_swapcontext:
   */
        .globl  handle_page_fault
  handle_page_fault:
+       stw     r5,_DSISR(r1)
        addi    r3,r1,STACK_FRAME_OVERHEAD
  #ifdef CONFIG_PPC_BOOK3S_32
        andis.  r0,r5,DSISR_DABRMATCH@h

Is this what you want to do for 32, or do you want to seperate
ISI and DSI sides?


No I think we want to separate ISI and DSI sides.

And I think the specific filtering done in ISI could be done in do_page_fault() in C. Ok, it would make a special handling for is_exec but there are already several places where the behaviour differs based on is_exec. The only thing we need to keep at ASM level is the DABR stuff for calling do_break() in handle_page_fault(), because it is used to decide whether full regs are saved or not. But it could be a test done earlier in the prolog and the result being kept in one of the CR bits.

That way we can avoid reloading dsisr and dar from the stack, especially for VMAP stack as they are saved quite early in the prolog.

Or we can take them out of the thread struct and save them in the stack a little latter in the prolog, but then we have to keep the RI bit a bit cleared longer.

While we are playing with do_page_fault(), did you have a look at the series I sent https://patchwork.ozlabs.org/project/linuxppc-dev/patch/7baae4086cbb9ffb08c933b065ff7d29dbc03dd6.1596734104.git.christophe.le...@csgroup.eu/

Christophe

Reply via email to