[PATCH v4] powerpc/traps: Enhance readability for trap types

Nicholas Piggin npiggin at gmail.com
Sat Apr 10 10:35:23 AEST 2021


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 <sxwjean at gmail.com>
> 
> 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 <lkp at intel.com>

It now looks like lkp asked for this whole cleanup patch. I would
put [kernel test robot <lkp at intel.com>] in your v3->4 changelog
item.

> Signed-off-by: Xiongwei Song <sxwjean at gmail.com>
> ---
> 
> 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 <lkp at intel.com>" here,
> because it's improper for this patch.

[...]

> 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

[...]

> @@ -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


More information about the Linuxppc-dev mailing list