Re: [PATCH 04/32] s390/bug: Indicate modified PSW in disassembly

[Ilya Leoshkevich <iii@linux.ibm.com>]

On 10/1/26 12:32, Heiko Carstens wrote:
> With [1] the PSW address is set to the return address of the calling
> __WARN_trap() function in order to provide a more useful disassembly of
> the caller rather than the helper itself.
> 
> However, since regs->psw.addr is modified, regs->int_code has nothing to
> do with the exception. In order to make that more obvious change the
> disassembler output.
> 
> Introduce a new PIF_WARN_TRAP_SKIPPED pt_regs flag and set it whenever
> __WARN_trap() is skipped. In the disassembler output print a "WARN RET"
> prefix to make it obvious that the PSW points to the return address of
> the warning trap function.
> 
> Also do not print the interruption code and instruction length code for
> this case, since they have nothing to do with the modified PSW.
> 
> Example output:
> 
> Krnl Code:
>             000003ffe185bc1c: e3f0ff98ff71 lay     %r15,-104(%r15)
>             000003ffe185bc22: e3e0f0980024 stg     %r14,152(%r15)
>             000003ffe185bc28: c0e5ff45ed94 brasl   %r14,000003ffe0119750
>   WARN RET >000003ffe185bc2e: c0e5ffa052b9 brasl   %r14,000003ffe0c661a0
>             000003ffe185bc34: c020fffe86d6 larl    %r2,000003ffe182c9e0
>             000003ffe185bc3a: e548f0a80006 mvghi   168(%r15),6
>             000003ffe185bc40: e548f0a00005 mvghi   160(%r15),5
>             000003ffe185bc46: a7690004     lghi    %r6,4
> 
> [1] 79996065cfa2 ("s390/bug: Skip __WARN_trap() in call traces")
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>   arch/s390/include/asm/ptrace.h |  2 ++
>   arch/s390/kernel/dis.c         | 23 ++++++++++-------------
>   arch/s390/kernel/dumpstack.c   |  7 +++++--
>   arch/s390/kernel/early.c       |  9 +++++++--
>   arch/s390/kernel/traps.c       |  1 +
>   5 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
> index 6411e3584283..c844b8aa0fd9 100644
> --- a/arch/s390/include/asm/ptrace.h
> +++ b/arch/s390/include/asm/ptrace.h
> @@ -17,11 +17,13 @@
>   #define PIF_PSW_ADDR_ADJUSTED		1	/* psw address has been adjusted */
>   #define PIF_SYSCALL_RET_SET		2	/* return value was set via ptrace */
>   #define PIF_FTRACE_FULL_REGS		3	/* all register contents valid (ftrace) */
> +#define PIF_WARN_TRAP_SKIPPED		4	/* __WARN_trap() was skipped */
>   
>   #define _PIF_SYSCALL			BIT(PIF_SYSCALL)
>   #define _PIF_ADDR_PSW_ADJUSTED		BIT(PIF_PSW_ADDR_ADJUSTED)
>   #define _PIF_SYSCALL_RET_SET		BIT(PIF_SYSCALL_RET_SET)
>   #define _PIF_FTRACE_FULL_REGS		BIT(PIF_FTRACE_FULL_REGS)
> +#define _PIF_WARN_TRAP_SKIPPED		BIT(PIF_WARN_TRAP_SKIPPED)
>   
>   #define PSW32_MASK_PER		_AC(0x40000000, UL)
>   #define PSW32_MASK_DAT		_AC(0x04000000, UL)
> diff --git a/arch/s390/kernel/dis.c b/arch/s390/kernel/dis.c
> index 1cec93895b3a..5a1a24d211c3 100644
> --- a/arch/s390/kernel/dis.c
> +++ b/arch/s390/kernel/dis.c
> @@ -540,34 +540,31 @@ void show_code(struct pt_regs *regs)
>   		start += 2;
>   	}
>   	/* Decode the instructions. */
> -	ptr = buffer;
> -	ptr += sprintf(ptr, "%s Code:", mode);
> +	printk("%s Code:\n", mode);
>   	hops = 0;
>   	while (start < end && hops < 8) {
> +		ptr = buffer;
>   		opsize = insn_length(code[start]);
> -		if  (start + opsize == 32)
> -			*ptr++ = '*';
> +		if (start == 32 && test_pt_regs_flag(regs, PIF_WARN_TRAP_SKIPPED))
> +			ptr += sprintf(ptr, " WARN RET >");
> +		else if (start + opsize == 32 && !test_pt_regs_flag(regs, PIF_WARN_TRAP_SKIPPED))
> +			ptr += sprintf(ptr, "          *");
>   		else if (start == 32)
> -			*ptr++ = '>';
> +			ptr += sprintf(ptr, "          >");
>   		else
> -			*ptr++ = ' ';
> +			ptr += sprintf(ptr, "           ");
>   		addr = pswaddr + start - 32;
>   		ptr += sprintf(ptr, "%px: ", (void *)addr);
>   		if (start + opsize >= end)
>   			break;
>   		for (i = 0; i < opsize; i++)
>   			ptr += sprintf(ptr, "%02x", code[start + i]);
> -		*ptr++ = '\t';
> -		if (i < 6)
> -			*ptr++ = '\t';
> +		ptr += sprintf(ptr, "%*s", 13 - 2 * opsize, " ");
>   		ptr += print_insn(ptr, code + start, addr);
>   		start += opsize;
> -		pr_cont("%s", buffer);
> -		ptr = buffer;
> -		ptr += sprintf(ptr, "\n          ");
> +		pr_cont("%s\n", buffer);

Should this really be pr_cont() and not printk()?
The KERN_CONT doc says:

     only done after a line that had no enclosing \n

and the new code prints everything line-by-line.