Hi, Tao Thank you for the fix. On Wed, Sep 18, 2024 at 7:44 AM <devel-request(a)lists.crash-utility.osci.io&gt; wrote: > > Date: Wed, 18 Sep 2024 11:42:03 +1200 > From: Tao Liu <ltao(a)redhat.com&gt; > Subject: [Crash-utility] [PATCH v1 1/3] ppc64: Fix bt printing error > stack trace > To: devel(a)lists.crash-utility.osci.io > Cc: adityag(a)linux.ibm.com, Tao Liu <ltao(a)redhat.com&gt; > Message-ID: <20240917234205.7783-2-ltao(a)redhat.com&gt; > Content-Type: text/plain; charset="US-ASCII"; x-default=true > > A error stack trace of bt cmd observed: > > crash> bt 1 > PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd" > #0 [c0000000037735c0] _end at c0000000037154b0 (unreliable) > #1 [c000000003773770] __switch_to at c00000000001fa9c > #2 [c0000000037737d0] __schedule at c00000000112e4ec > #3 [c0000000037738b0] schedule at c00000000112ea80 > ... > > The #0 stack trace is incorrect, the function address shouldn't exceed _end. > The reason is for kernel>=v6.2, the offset of pt_regs to sp changed from > STACK_FRAME_OVERHEAD, i.e 112, to STACK_SWITCH_FRAME_REGS. For > CONFIG_PPC64_ELF_ABI_V1, it's 112, for ABI_V2, it's 48. So the nip will read a > wrong value from stack when ABI_V2 enabled. > Can you help to add the related kernel commits to patch log? That will help me a lot to review the patches.

> > To determine if ABI_V2 enabled is tricky. This patch do it by check the > following: Can you try to read the value of e_flags from the elf header and to determine what ABI version it is?

Thanks Lianbo > > > In arch/powerpc/include/asm/ppc_asm.h: > #ifdef CONFIG_PPC64_ELF_ABI_V2 > #define STK_GOT 24 > #else > #define STK_GOT 40 > > In arch/powerpc/kernel/tm.S: > _GLOBAL(tm_reclaim) > mfcr r5 > mflr r0 > stw r5, 8(r1) > std r0, 16(r1) > std r2, STK_GOT(r1) > ... > > So a disassemble on tm_reclaim, and extract the STK_GOT value from std > instruction is used as the approach. > > After the patch: > crash> bt 1 > PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd" > #0 [c0000000037737d0] __schedule at c00000000112e4ec > #1 [c0000000037738b0] schedule at c00000000112ea80 > ... > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > defs.h | 1 + > ppc64.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 58 insertions(+), 3 deletions(-) > > diff --git a/defs.h b/defs.h > index 2231cb6..d5cb8cc 100644 > --- a/defs.h > +++ b/defs.h > @@ -4643,6 +4643,7 @@ struct efi_memory_desc_t { > #define MSR_PR_LG 14 /* Problem State / Privilege Level */ > /* Used to find the user or kernel-mode frame*/ > > +#define STACK_SWITCH_FRAME_REGS 48 > #define STACK_FRAME_OVERHEAD 112 > #define EXCP_FRAME_MARKER 0x7265677368657265 > > diff --git a/ppc64.c b/ppc64.c > index e8930a1..6e5f155 100644 > --- a/ppc64.c > +++ b/ppc64.c > @@ -72,6 +72,7 @@ static ulong pud_page_vaddr_l4(ulong pud); > static ulong pmd_page_vaddr_l4(ulong pmd); > static int is_opal_context(ulong sp, ulong nip); > void opalmsg(void); > +static bool is_ppc64_elf_abi_v2(void); > > static int is_opal_context(ulong sp, ulong nip) > { > @@ -2813,6 +2814,51 @@ ppc64_get_sp(ulong task) > return sp; > } > > +static bool > +is_ppc64_elf_abi_v2(void) > +{ > + char buf1[BUFSIZE]; > + char *pos1, *pos2; > + int errflag = 0; > + ulong stk_got = 0; > + static bool ret = false; > + static bool checked = false; > + > + if (checked == true || !symbol_exists("tm_reclaim")) > + return ret; > + > + sprintf(buf1, "x/16i tm_reclaim"); > + open_tmpfile(); > + if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR)) > + goto out; > + checked = true; > + rewind(pc->tmpfile); > + while (fgets(buf1, BUFSIZE, pc->tmpfile)) { > + // "std r2, STK_GOT(r1)" is expected > + if (strstr(buf1, "std") && > + strstr(buf1, "(r1)") && > + (pos1 = strstr(buf1, "r2,"))) { > + pos1 += strlen("r2,"); > + for (pos2 = pos1; *pos2 != '\0' && *pos2 != '('; pos2++); > + *pos2 = '\0'; > + stk_got = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); > + break; > + } > + } > + > + if (!errflag) { > + switch (stk_got) { > + case 24: > + ret = true; > + case 40: > + goto out; > + } > + } > + error(WARNING, "Unstable elf_abi v1/v2 detection.\n"); > +out: > + close_tmpfile(); > + return ret; > +} > > /* > * get the SP and PC values for idle tasks. > @@ -2834,9 +2880,17 @@ get_ppc64_frame(struct bt_info *bt, ulong *getpc, ulong *getsp) > sp = ppc64_get_sp(task); > if (!INSTACK(sp, bt)) > goto out; > - readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, &regs, > - sizeof(struct ppc64_pt_regs), > - "PPC64 pt_regs", FAULT_ON_ERROR); > + > + if (THIS_KERNEL_VERSION >= LINUX(6,2,0) && is_ppc64_elf_abi_v2()) { > + readmem(sp+STACK_SWITCH_FRAME_REGS, KVADDR, &regs, > + sizeof(struct ppc64_pt_regs), > + "PPC64 pt_regs", FAULT_ON_ERROR); > + } else { > + readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, &regs, > + sizeof(struct ppc64_pt_regs), > + "PPC64 pt_regs", FAULT_ON_ERROR); > + } > + > ip = regs.nip; > closest = closest_symbol(ip); > if (STREQ(closest, ".__switch_to") || STREQ(closest, "__switch_to")) { > -- > 2.40.1

On Thu, Sep 19, 2024 at 11:28 AM Tao Liu <ltao(a)redhat.com&gt; wrote: > > Hi lianbo, > > On Thu, Sep 19, 2024 at 2:41 PM lijiang <lijiang(a)redhat.com&gt; wrote: > > > > Hi, Tao > > > > Thank you for the fix. > > > > On Wed, Sep 18, 2024 at 7:44 AM <devel-request(a)lists.crash-utility.osci.io&gt; wrote: > >> > >> Date: Wed, 18 Sep 2024 11:42:03 +1200 > >> From: Tao Liu <ltao(a)redhat.com&gt; > >> Subject: [Crash-utility] [PATCH v1 1/3] ppc64: Fix bt printing error > >> stack trace > >> To: devel(a)lists.crash-utility.osci.io > >> Cc: adityag(a)linux.ibm.com, Tao Liu <ltao(a)redhat.com&gt; > >> Message-ID: <20240917234205.7783-2-ltao(a)redhat.com&gt; > >> Content-Type: text/plain; charset="US-ASCII"; x-default=true > >> > >> A error stack trace of bt cmd observed: > >> > >> crash> bt 1 > >> PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd" > >> #0 [c0000000037735c0] _end at c0000000037154b0 (unreliable) > >> #1 [c000000003773770] __switch_to at c00000000001fa9c > >> #2 [c0000000037737d0] __schedule at c00000000112e4ec > >> #3 [c0000000037738b0] schedule at c00000000112ea80 > >> ... > >> > >> The #0 stack trace is incorrect, the function address shouldn't exceed _end. > >> The reason is for kernel>=v6.2, the offset of pt_regs to sp changed from > >> STACK_FRAME_OVERHEAD, i.e 112, to STACK_SWITCH_FRAME_REGS. For > >> CONFIG_PPC64_ELF_ABI_V1, it's 112, for ABI_V2, it's 48. So the nip will read a > >> wrong value from stack when ABI_V2 enabled. > >> > > > > Can you help to add the related kernel commits to patch log? That will help me a lot to review the patches. > > > Sure, will update it in v2. > > >> > >> To determine if ABI_V2 enabled is tricky. This patch do it by check the > >> following: > > > > > > Can you try to read the value of e_flags from the elf header and to determine what ABI version it is? > > > Do you mean by checking the following? > > arch/powerpc/kernel/fadump.c:fadump_init_elfcore_header(): > > if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2)) > elf->e_flags = 2; > else if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V1)) > elf->e_flags = 1; > else > elf->e_flags = 0; > > I think this only works for fadump, but it will not work for normal Maybe "not". I did not try it, but I noticed that there is a similar checking in kernel module, please refer to this one: arch/powerpc/kernel/module_64.c bool module_elf_check_arch(Elf_Ehdr *hdr) { unsigned long abi_level = hdr->e_flags & 0x3; if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2)) return abi_level == 2; else return abi_level < 2; } > kdump. Also for kcore, the e_flags are set as: > > read_kcore_iter(): > #ifndef ELF_CORE_EFLAGS > #define ELF_CORE_EFLAGS 0 > #endif > .e_flags = ELF_CORE_EFLAGS, > > So it seems the e_flags is not a suitable approach for me... The ELF_CORE_EFLAGS should be defined in the arch/powerpc/include/asm/elf.h: #define ELF_CORE_EFLAGS (is_elf2_task() ? 2 : 0) And also refer to this one(arch/powerpc/include/asm/thread_info.h): #if defined(CONFIG_PPC64) #define is_elf2_task() (test_thread_flag(TIF_ELF2ABI)) #else #define is_elf2_task() (0) #endif Anyway, can you try it? Or have you already tried it?

Lianbo > > Thanks, > Tao Liu > > > > Thanks > > Lianbo > > > >> > >> > >> In arch/powerpc/include/asm/ppc_asm.h: > >> #ifdef CONFIG_PPC64_ELF_ABI_V2 > >> #define STK_GOT 24 > >> #else > >> #define STK_GOT 40 > >> > >> In arch/powerpc/kernel/tm.S: > >> _GLOBAL(tm_reclaim) > >> mfcr r5 > >> mflr r0 > >> stw r5, 8(r1) > >> std r0, 16(r1) > >> std r2, STK_GOT(r1) > >> ... > >> > >> So a disassemble on tm_reclaim, and extract the STK_GOT value from std > >> instruction is used as the approach. > >> > >> After the patch: > >> crash> bt 1 > >> PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd" > >> #0 [c0000000037737d0] __schedule at c00000000112e4ec > >> #1 [c0000000037738b0] schedule at c00000000112ea80 > >> ... > >> > >> Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > >> --- > >> defs.h | 1 + > >> ppc64.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > >> 2 files changed, 58 insertions(+), 3 deletions(-) > >> > >> diff --git a/defs.h b/defs.h > >> index 2231cb6..d5cb8cc 100644 > >> --- a/defs.h > >> +++ b/defs.h > >> @@ -4643,6 +4643,7 @@ struct efi_memory_desc_t { > >> #define MSR_PR_LG 14 /* Problem State / Privilege Level */ > >> /* Used to find the user or kernel-mode frame*/ > >> > >> +#define STACK_SWITCH_FRAME_REGS 48 > >> #define STACK_FRAME_OVERHEAD 112 > >> #define EXCP_FRAME_MARKER 0x7265677368657265 > >> > >> diff --git a/ppc64.c b/ppc64.c > >> index e8930a1..6e5f155 100644 > >> --- a/ppc64.c > >> +++ b/ppc64.c > >> @@ -72,6 +72,7 @@ static ulong pud_page_vaddr_l4(ulong pud); > >> static ulong pmd_page_vaddr_l4(ulong pmd); > >> static int is_opal_context(ulong sp, ulong nip); > >> void opalmsg(void); > >> +static bool is_ppc64_elf_abi_v2(void); > >> > >> static int is_opal_context(ulong sp, ulong nip) > >> { > >> @@ -2813,6 +2814,51 @@ ppc64_get_sp(ulong task) > >> return sp; > >> } > >> > >> +static bool > >> +is_ppc64_elf_abi_v2(void) > >> +{ > >> + char buf1[BUFSIZE]; > >> + char *pos1, *pos2; > >> + int errflag = 0; > >> + ulong stk_got = 0; > >> + static bool ret = false; > >> + static bool checked = false; > >> + > >> + if (checked == true || !symbol_exists("tm_reclaim")) > >> + return ret; > >> + > >> + sprintf(buf1, "x/16i tm_reclaim"); > >> + open_tmpfile(); > >> + if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR)) > >> + goto out; > >> + checked = true; > >> + rewind(pc->tmpfile); > >> + while (fgets(buf1, BUFSIZE, pc->tmpfile)) { > >> + // "std r2, STK_GOT(r1)" is expected > >> + if (strstr(buf1, "std") && > >> + strstr(buf1, "(r1)") && > >> + (pos1 = strstr(buf1, "r2,"))) { > >> + pos1 += strlen("r2,"); > >> + for (pos2 = pos1; *pos2 != '\0' && *pos2 != '('; pos2++); > >> + *pos2 = '\0'; > >> + stk_got = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag); > >> + break; > >> + } > >> + } > >> + > >> + if (!errflag) { > >> + switch (stk_got) { > >> + case 24: > >> + ret = true; > >> + case 40: > >> + goto out; > >> + } > >> + } > >> + error(WARNING, "Unstable elf_abi v1/v2 detection.\n"); > >> +out: > >> + close_tmpfile(); > >> + return ret; > >> +} > >> > >> /* > >> * get the SP and PC values for idle tasks. > >> @@ -2834,9 +2880,17 @@ get_ppc64_frame(struct bt_info *bt, ulong *getpc, ulong *getsp) > >> sp = ppc64_get_sp(task); > >> if (!INSTACK(sp, bt)) > >> goto out; > >> - readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, &regs, > >> - sizeof(struct ppc64_pt_regs), > >> - "PPC64 pt_regs", FAULT_ON_ERROR); > >> + > >> + if (THIS_KERNEL_VERSION >= LINUX(6,2,0) && is_ppc64_elf_abi_v2()) { > >> + readmem(sp+STACK_SWITCH_FRAME_REGS, KVADDR, &regs, > >> + sizeof(struct ppc64_pt_regs), > >> + "PPC64 pt_regs", FAULT_ON_ERROR); > >> + } else { > >> + readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, &regs, > >> + sizeof(struct ppc64_pt_regs), > >> + "PPC64 pt_regs", FAULT_ON_ERROR); > >> + } > >> + > >> ip = regs.nip; > >> closest = closest_symbol(ip); > >> if (STREQ(closest, ".__switch_to") || STREQ(closest, "__switch_to")) { > >> -- > >> 2.40.1 >