Re: [PATCH v4] powerpc/traps: Enhance readability for trap types
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2021-04-10 00:35:33
Also in:
lkml
Thanks for working on this, I think it's a nice cleanup and helps non-powerpc people understand the code a bit better. Excerpts from Xiongwei Song's message of April 10, 2021 12:28 am:
From: Xiongwei Song <redacted> Create a new header named traps.h, define macros to list ppc interrupt types in traps.h, replace the references of the trap hex values with these macros. Referred the hex numbers in arch/powerpc/kernel/exceptions-64e.S, arch/powerpc/kernel/exceptions-64s.S and arch/powerpc/include/asm/kvm_asm.h. Reported-by: kernel test robot <redacted>
It now looks like lkp asked for this whole cleanup patch. I would put [kernel test robot [off-list ref]] in your v3->4 changelog item.
Signed-off-by: Xiongwei Song <redacted> --- v3-v4: Fix compile issue: arch/powerpc/kernel/process.c:1473:14: error: 'INTERRUPT_MACHINE_CHECK' undeclared (first use in this function); did you mean 'TAINT_MACHINE_CHECK'? I didn't add "Reported-by: kernel test robot [off-list ref]" here, because it's improper for this patch.
[...]
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h new file mode 100644 index 000000000000..2e64e10afcef --- /dev/null +++ b/arch/powerpc/include/asm/traps.h@@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_PPC_TRAPS_H +#define _ASM_PPC_TRAPS_H
These could go in interrupt.h.
+#if defined(CONFIG_BOOKE) || defined(CONFIG_4xx) +#define INTERRUPT_MACHINE_CHECK 0x000 +#define INTERRUPT_CRITICAL_INPUT 0x100 +#define INTERRUPT_ALTIVEC_UNAVAIL 0x200 +#define INTERRUPT_PERFMON 0x260 +#define INTERRUPT_DOORBELL 0x280 +#define INTERRUPT_DEBUG 0xd00 +#else +#define INTERRUPT_SYSTEM_RESET 0x100 +#define INTERRUPT_MACHINE_CHECK 0x200
[...]
quoted hunk ↗ jump to hunk
@@ -1469,7 +1470,9 @@ static void __show_regs(struct pt_regs *regs) trap = TRAP(regs); if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR)) pr_cont("CFAR: "REG" ", regs->orig_gpr3); - if (trap == 0x200 || trap == 0x300 || trap == 0x600) { + if (trap == INTERRUPT_MACHINE_CHECK || + trap == INTERRUPT_DATA_STORAGE || + trap == INTERRUPT_ALIGNMENT) { if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE)) pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr); else
This is now a change in behaviour because previously BOOKE/4xx tested 0x200, but now it tests 0. That looks wrong for 4xx. 64e does put 0x000 there but I wonder if it should use 0x200 instead. Bit difficult to test this stuff, I do have some MCE injection patches for QEMU for 64s, might be able to look at porting them to 64e although I have no idea about booke machine checks. Anyway I don't think this patch should change generated code at all. Either change the code first with smaller patches, or make sure you keep the tests the same. Thanks, Nick