Support hardware perf counters on xtensa

Restore original a0 in the kernel exception stack frame. This way it looks like the frame that got interrupt/exception did alloca (copy a0 and a1 potentially spilled under old stack to the new location as well) to save registers and then did a call to handler. The point where interrupt/exception was taken is not in the stack chain, only in pt_regs (call4 from that address can be simulated to keep it in the stack trace).

Signed-off-by Max Filippov <[email protected]>

---
 arch/xtensa/kernel/entry.S | 60 +++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

arch/xtensa/kernel/entry.S +35 -26

index 82bbfa5..ab6a857 100644
@@ -218,7 +218,7 @@ _user_exception:
        /* We are back to the original stack pointer (a1) */
 
 2:     /* Now, jump to the common exception handler. */
-
+       movi    a0, 0                   # terminate user stack trace with 0
        j       common_exception
 
 ENDPROC(user_exception)

@@ -264,6 +264,19 @@ ENTRY(kernel_exception)
        .globl _kernel_exception
 _kernel_exception:
 
+       /* Copy spill slots of a0 and a1 to imitate movsp
+        * in order to keep exception stack continuous
+        */
+       l32i    a0, a1, PT_AREG1
+       addi    a2, a1, -16
+       addi    a0, a0, -16
+       l32i    a3, a0, 0
+       l32i    a0, a0, 4
+       s32i    a3, a2, 0
+       s32i    a0, a2, 4
+
+       l32i    a0, a1, PT_AREG0        # restore saved a0
+
        /* Save SAR and turn off single stepping */
 
        movi    a2, 0

@@ -338,52 +351,50 @@ common_exception:
        xsr     a2, lcount
        s32i    a2, a1, PT_LCOUNT
 
-       /* It is now save to restore the EXC_TABLE_FIXUP variable. */
+       /* It is now safe to restore the EXC_TABLE_FIXUP variable. */
 
-       rsr     a0, exccause
        movi    a3, 0
        rsr     a2, excsave1
-       s32i    a0, a1, PT_EXCCAUSE
        s32i    a3, a2, EXC_TABLE_FIXUP
 
-       /* All unrecoverable states are saved on stack, now, and a1 is valid,
-        * so we can allow exceptions and interrupts (*) again.
-        * Set PS(EXCM = 0, UM = 0, RING = 0, OWB = 0, WOE = 1, INTLEVEL = X)
+       /* Save remaining unrecoverable states (exccause, ps) on stack.
+        * Now we can allow exceptions again. In case we've got an interrupt
+        * PS.INTLEVEL is set to LOCKLEVEL disabling furhter interrupts,
+        * otherwise it's left unchanged.
         *
-        * (*) We only allow interrupts if they were previously enabled and
-        *     we're not handling an IRQ
+        * Set PS(EXCM = 0, UM = 0, RING = 0, OWB = 0, WOE = 1, INTLEVEL = X)
         */
 
        rsr     a3, ps
-       addi    a0, a0, -EXCCAUSE_LEVEL1_INTERRUPT
-       movi    a2, LOCKLEVEL
+       movi    a2, (1 << PS_WOE_BIT)
        extui   a3, a3, PS_INTLEVEL_SHIFT, PS_INTLEVEL_WIDTH
-                                       # a3 = PS.INTLEVEL
-       moveqz  a3, a2, a0              # a3 = LOCKLEVEL iff interrupt
-       movi    a2, 1 << PS_WOE_BIT
        or      a3, a3, a2
-       rsr     a0, exccause
-       xsr     a3, ps
 
+       rsr     a2, exccause
+       bnei    a2, EXCCAUSE_LEVEL1_INTERRUPT, 1f
+       movi    a3, (1 << PS_WOE_BIT) | LOCKLEVEL
+1:
+       xsr     a3, ps
+       s32i    a2, a1, PT_EXCCAUSE
        s32i    a3, a1, PT_PS           # save ps
 
        /* Save lbeg, lend */
 
-       rsr     a2, lbeg
+       rsr     a4, lbeg
        rsr     a3, lend
-       s32i    a2, a1, PT_LBEG
+       s32i    a4, a1, PT_LBEG
        s32i    a3, a1, PT_LEND
 
        /* Save SCOMPARE1 */
 
 #if XCHAL_HAVE_S32C1I
-       rsr     a2, scompare1
-       s32i    a2, a1, PT_SCOMPARE1
+       rsr     a3, scompare1
+       s32i    a3, a1, PT_SCOMPARE1
 #endif
 
        /* Save optional registers. */
 
-       save_xtregs_opt a1 a2 a4 a5 a6 a7 PT_XTREGS_OPT
+       save_xtregs_opt a1 a3 a4 a5 a6 a7 PT_XTREGS_OPT
 
 #ifdef CONFIG_TRACE_IRQFLAGS
        l32i    a4, a1, PT_DEPC

@@ -391,8 +402,7 @@ common_exception:
         * while PS.EXCM was set, i.e. interrupts disabled.
         */
        bgeui   a4, VALID_DOUBLE_EXCEPTION_ADDRESS, 1f
-       l32i    a4, a1, PT_EXCCAUSE
-       bnei    a4, EXCCAUSE_LEVEL1_INTERRUPT, 1f
+       bnei    a2, EXCCAUSE_LEVEL1_INTERRUPT, 1f
        /* We came here with an interrupt means interrupts were enabled
         * and we've just disabled them.
         */

@@ -407,8 +417,8 @@ common_exception:
 
        rsr     a4, excsave1
        mov     a6, a1                  # pass stack frame
-       mov     a7, a0                  # pass EXCCAUSE
-       addx4   a4, a0, a4
+       mov     a7, a2                  # pass EXCCAUSE
+       addx4   a4, a2, a4
        l32i    a4, a4, EXC_TABLE_DEFAULT               # load handler
 
        /* Call the second-level handler */
--

1.8.1.4

Not such a big fan about these changes as they introduce additional load and stores and a branch.

Copy spill area:
- is this only for debugging? Could the debugger identify the 'kernel exception' frame and handle it appropriately?

- if we need it for debugging/perf, maybe move these line under a kernel option (could be enabled by default)

- since we know the kernel stack frame size, why can't we just load from that offset (instead of the l32i and add). Also, can't we use s32e (need to look up the spec again).

Branch:
- is there any logical change in the code or are these only to avoid using a0?

- if for a0, couldn't we load a0 just before the 'xsr a3, ps' and keep the code as before?

Quoted 2 lines

Branch:
- is there any logical change in the code or are these only to avoid using a0?

Yes.

arch/xtensa/kernel/perf_event.c new file +21 -0

+static int xtensa_pmu_event_init(struct perf_event *event)
+{
+	int ret;
+
+	switch (event->attr.type) {
+	case PERF_TYPE_HARDWARE:
+		if (event->attr.config >= ARRAY_SIZE(xtensa_hw_ctl) ||
+		    xtensa_hw_ctl[event->attr.config] == 0)
+			return -ENOENT;
+		event->hw.config = xtensa_hw_ctl[event->attr.config];
+		return 0;
+
+	case PERF_TYPE_HW_CACHE:
+		ret = xtensa_pmu_cache_event(event->attr.config);
+		if (ret < 0)
+			return ret;
+		event->hw.config = ret;
+		return 0;
+
+	case PERF_TYPE_RAW:
+		/* Not 'previous counter' select */

arch/xtensa/kernel/perf_event.c new file +5 -0

+
+	default:
+		return -ENOENT;
+	}
+}

Since this PMU implements TYPE_HARDWARE/TYPE_HW_CACHE/TYPE_RAW, returning -ENOENT for those is wrong.

If the configuration is invalid, return -EINVAL.

arch/xtensa/kernel/perf_event.c line 436

Quoted 4 lines · Max Filippov

arch/xtensa/kernel/perf_event.c new file +4 -0

+static int __init xtensa_pmu_init(void)
+{
+     int ret;
+     int irq = irq_create_mapping(NULL, XCHAL_PROFILING_INTERRUPT);

Does this platform have interrupt priorities which you can partially mask in order to create NMI like behaviour?

Quoted 8 lines · Peter Zijlstra

arch/xtensa/kernel/perf_event.c line 436

Quoted 4 lines · Max Filippov

arch/xtensa/kernel/perf_event.c new file +4 -0

+static int __init xtensa_pmu_init(void)
+{
+     int ret;
+     int irq = irq_create_mapping(NULL, XCHAL_PROFILING_INTERRUPT);

Does this platform have interrupt priorities which you can partially mask in order to create NMI like behaviour?

Not sure what you mean by "NMI like".

Interrupt priorities are fixed in the current xtensa architecture, and we can in theory mask certain level and below, but practically we always mask all low- and medium- level interrupts.

Also we currently can't have handlers for high priority interrupts written in C.

Quoted 11 lines · Max Filippov

Quoted 8 lines · Peter Zijlstra

arch/xtensa/kernel/perf_event.c line 436

Quoted 4 lines · Max Filippov

arch/xtensa/kernel/perf_event.c new file +4 -0

+static int __init xtensa_pmu_init(void)
+{
+     int ret;
+     int irq = irq_create_mapping(NULL, XCHAL_PROFILING_INTERRUPT);

Does this platform have interrupt priorities which you can partially mask in order to create NMI like behaviour?

Not sure what you mean by "NMI like".

There's a number of archs where we implement NMIs by having local_irq_disable() only disable part of the interrupt priority range and making sure all 'normal' IRQs are mapped in that priority range.

We then map our NMI handlers to a priority above the 'normal' range, such that these interrupts can indeed happen when interrupts are 'disabled.

See for example:

b4f4372f96e0 ("sparc64: Make %pil level 15 a pseudo-NMI.") 0c25e9e6cbe7 ("sparc64: Adjust __raw_local_irq_save() to cooperate in NMIs.") c011f80ba091 ("sparc64: Add some more commentary to __raw_local_irq_save()")

Quoted 5 lines

Interrupt priorities are fixed in the current xtensa architecture, and we can in theory mask certain level and below, but practically we always mask all low- and medium- level interrupts.

Also we currently can't have handlers for high priority interrupts written in C.

Why not? Surely this can be cured with an assembly stub?

The advantage of having NMIs is that profiling information for the kernel becomes much more useful. Without this local_irq_enable() will be a very 'hot' function.

Quoted 21 lines · Peter Zijlstra

Quoted 11 lines · Max Filippov

Quoted 8 lines · Peter Zijlstra

arch/xtensa/kernel/perf_event.c line 436

Quoted 4 lines · Max Filippov

arch/xtensa/kernel/perf_event.c new file +4 -0

+static int __init xtensa_pmu_init(void)
+{
+     int ret;
+     int irq = irq_create_mapping(NULL, XCHAL_PROFILING_INTERRUPT);

Does this platform have interrupt priorities which you can partially mask in order to create NMI like behaviour?

Not sure what you mean by "NMI like".

There's a number of archs where we implement NMIs by having local_irq_disable() only disable part of the interrupt priority range and making sure all 'normal' IRQs are mapped in that priority range.

We then map our NMI handlers to a priority above the 'normal' range, such that these interrupts can indeed happen when interrupts are 'disabled.

Quoted 5 lines

See for example:

b4f4372f96e0 ("sparc64: Make %pil level 15 a pseudo-NMI.") 0c25e9e6cbe7 ("sparc64: Adjust __raw_local_irq_save() to cooperate in NMIs.") c011f80ba091 ("sparc64: Add some more commentary to __raw_local_irq_save()")

Ok, I see. I guess I can change IRQ disabling logic to not mask perf IRQ in case it's configured as the only interrupt on its level and it's the highest medium-level IRQ.

Quoted 7 lines

Quoted 5 lines

Interrupt priorities are fixed in the current xtensa architecture, and we can in theory mask certain level and below, but practically we always mask all low- and medium- level interrupts.

Also we currently can't have handlers for high priority interrupts written in C.

Why not? Surely this can be cured with an assembly stub?

IIUC that was a deliberate architecture design choice and working around it penalizes all interrupt handlers. But let me take another close look.

Quoted 3 lines

The advantage of having NMIs is that profiling information for the kernel becomes much more useful. Without this local_irq_enable() will be a very 'hot' function.

I haven't noticed that in my testing.

--
Thanks.
-- Max