[PATCH v2 1/4] powerpc/instruction_dump: Fix kernel crash with show_instructions

[Christophe Leroy]

Le 24/05/2020 à 11:38, Aneesh Kumar K.V a écrit :
> With Hard Lockup watchdog, we can hit a BUG() if we take a watchdog
> interrupt when in OPAL mode. This happens in show_instructions()
> where the kernel takes the watchdog NMI IPI with MSR_IR == 0.
> With that show_instructions() updates the variable pc in the loop
> and the second iterations will result in BUG().
> 
> We hit the BUG_ON due the below check in  __va()
> 
>   #define __va(x)								\
> ({									\
> 	VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET);		\
> 	(void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET);	\
> })
> 
> Fixes: 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses")
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>

If still relevant, this series needs rebase.

Patch 1 was applied with changes, at least patch 3 won't apply

Thanks
Christophe


> ---
>   arch/powerpc/kernel/process.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 048d64c4e115..93bf4a766707 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1253,29 +1253,32 @@ struct task_struct *__switch_to(struct task_struct *prev,
>   static void show_instructions(struct pt_regs *regs)
>   {
>   	int i;
> +	unsigned long nip = regs->nip;
>   	unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
>   
>   	printk("Instruction dump:");
>   
> +#if !defined(CONFIG_BOOKE)
> +	/* If executing with the IMMU off, adjust pc rather
> +	 * than print XXXXXXXX.
> +	 */
> +	if (!(regs->msr & MSR_IR)) {
> +		pc = (unsigned long)phys_to_virt(pc);
> +		nip = (unsigned long)phys_to_virt(regs->nip);
> +	}
> +#endif
> +
>   	for (i = 0; i < NR_INSN_TO_PRINT; i++) {
>   		int instr;
>   
>   		if (!(i % 8))
>   			pr_cont("\n");
>   
> -#if !defined(CONFIG_BOOKE)
> -		/* If executing with the IMMU off, adjust pc rather
> -		 * than print XXXXXXXX.
> -		 */
> -		if (!(regs->msr & MSR_IR))
> -			pc = (unsigned long)phys_to_virt(pc);
> -#endif
> -
>   		if (!__kernel_text_address(pc) ||
>   		    probe_kernel_address((const void *)pc, instr)) {
>   			pr_cont("XXXXXXXX ");
>   		} else {
> -			if (regs->nip == pc)
> +			if (nip == pc)
>   				pr_cont("<%08x> ", instr);
>   			else
>   				pr_cont("%08x ", instr);