Hi lianbo,
On Thu, Sep 19, 2024 at 2:41 PM lijiang <lijiang@redhat.com> wrote:
>
> Hi, Tao
>
> Thank you for the fix.
>
> On Wed, Sep 18, 2024 at 7:44 AM <devel-request@lists.crash-utility.osci.io> wrote:
>>
>> Date: Wed, 18 Sep 2024 11:42:03 +1200
>> From: Tao Liu <ltao@redhat.com>
>> Subject: [Crash-utility] [PATCH v1 1/3] ppc64: Fix bt printing error
>> stack trace
>> To: devel@lists.crash-utility.osci.io
>> Cc: adityag@linux.ibm.com, Tao Liu <ltao@redhat.com>
>> Message-ID: <20240917234205.7783-2-ltao@redhat.com>
>> 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@redhat.com>
>> ---
>> 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, ®s,
>> - 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, ®s,
>> + sizeof(struct ppc64_pt_regs),
>> + "PPC64 pt_regs", FAULT_ON_ERROR);
>> + } else {
>> + readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, ®s,
>> + 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