On Mon, Jun 3, 2024 at 10:15 AM <devel-request@lists.crash-utility.osci.io> wrote:
Date: Fri, 31 May 2024 17:19:39 +0800
From: Tao Liu <ltao@redhat.com>
Subject: [Crash-utility] [PATCH v4 16/16] arm64: Add gdb stack
        unwinding support
To: devel@lists.crash-utility.osci.io
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>, "Naveen N . Rao"
        <naveen.n.rao@linux.vnet.ibm.com>, Lianbo Jiang <lijiang@redhat.com>,
        Alexey Makhalov <alexey.makhalov@broadcom.com>
Message-ID: <20240531091939.97828-17-ltao@redhat.com>
Content-Type: text/plain; charset=UTF-8

Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Lianbo Jiang <lijiang@redhat.com>
Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com>
Cc: Tao Liu <ltao@redhat.com>
Cc: Alexey Makhalov <alexey.makhalov@broadcom.com>
Signed-off-by: Tao Liu <ltao@redhat.com>
---
 arm64.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 defs.h  |  36 ++++++++++++++++++
 2 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/arm64.c b/arm64.c
index af0e0d7..5f9be82 100644
--- a/arm64.c
+++ b/arm64.c
@@ -116,6 +116,23 @@ static void arm64_get_struct_page_size(struct machine_specific *ms);
 #define mte_tag_reset(addr)    (((ulong)addr & ~mte_tag_shifted(KASAN_TAG_KERNEL)) | \
                                        mte_tag_shifted(KASAN_TAG_KERNEL))

+struct user_regs_bitmap_struct {
+       struct {
+               union {
+                       struct arm64_user_pt_regs user_regs;
+                       struct {
+                               u64 regs[31];
+                               u64 sp;
+                               u64 pc;
+                               u64 pstate;
+                       };
+               };
+               u64 orig_x0;
+               u64 syscallno;
+       };
+       ulong bitmap;
+};
+
 static inline bool is_mte_kvaddr(ulong addr)
 {
        /* check for ARM64_MTE enabled */
@@ -174,6 +191,85 @@ arm64_vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
        return FALSE;
 }

+static int
+arm64_get_current_task_reg(int regno, const char *name,
+                   int size, void *value)
+{
+       struct bt_info bt_info, bt_setup;
+       struct task_context *tc;
+       struct user_regs_bitmap_struct *ur_bitmap;
+       ulong ip, sp;
+       bool ret = FALSE;
+
+       switch (regno) {
+               case X0_REGNUM ... PC_REGNUM:
+               break;
+       default:
+               return FALSE;
+       }
+

The above switch-case code block can be defined as a separate static function to do the sanity check, as mentioned in another thread.
Please refer to the comment here:

Link: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00816.html

Hmm, basically the current patch is similar to the code design of [patch v4 08/16], so I won't comment on the following code one by one, can you try to improve this one based on the previous comments? (See the above link).

Thanks.
Lianbo


+       tc = CURRENT_CONTEXT();
+       if (!tc)
+               return FALSE;
+       BZERO(&bt_setup, sizeof(struct bt_info));
+       clone_bt_info(&bt_setup, &bt_info, tc);
+       fill_stackbuf(&bt_info);
+
+       get_dumpfile_regs(&bt_info, &sp, &ip);
+       if (bt_info.stackbuf)
+               FREEBUF(bt_info.stackbuf);
+       ur_bitmap = (struct user_regs_bitmap_struct *)bt_info.machdep;
+       if (!ur_bitmap)
+               return FALSE;
+
+       if (!bt_info.need_free) {
+               goto get_all;
+       }
+       switch (ur_bitmap->bitmap) {
+       case 0x1a0000000:
+               switch (regno) {
+               case X29_REGNUM:
+               case SP_REGNUM:
+               case PC_REGNUM:
+                       break;
+               default:
+                       return FALSE;
+               }
+               break;
+       }
+
+get_all:
+       switch (regno) {
+       case X0_REGNUM ... X30_REGNUM:
+               if (size != sizeof(ur_bitmap->regs[regno])) {
+                       ret = FALSE; break;
+               } else {
+                       memcpy(value, &ur_bitmap->regs[regno], size);
+                       ret = TRUE; break;
+               }
+       case SP_REGNUM:
+                if (size != sizeof(ur_bitmap->sp)) {
+                        ret = FALSE; break;
+                } else {
+                        memcpy(value, &ur_bitmap->sp, size);
+                        ret = TRUE; break;
+                }
+       case PC_REGNUM:
+                if (size != sizeof(ur_bitmap->pc)) {
+                        ret = FALSE; break;
+                } else {
+                        memcpy(value, &ur_bitmap->pc, size);
+                        ret = TRUE; break;
+                }
+       }
+
+       if (bt_info.need_free) {
+               FREEBUF(ur_bitmap);
+               bt_info.need_free = FALSE;
+       }
+       return ret;
+}
+
 /*
  * Do all necessary machine-specific setup here. This is called several times
  * during initialization.
@@ -472,6 +568,7 @@ arm64_init(int when)
                machdep->dumpfile_init = NULL;
                machdep->verify_line_number = NULL;
                machdep->init_kernel_pgd = arm64_init_kernel_pgd;
+               machdep->get_current_task_reg = arm64_get_current_task_reg;

                /* use machdep parameters */
                arm64_calc_phys_offset();
@@ -3785,6 +3882,7 @@ arm64_get_dumpfile_stackframe(struct bt_info *bt, struct arm64_stackframe *frame
 try_kernel:
                frame->sp = ptregs->sp;
                frame->fp = ptregs->regs[29];
+               bt->machdep = ptregs;
        }

        if (arm64_in_kdump_text(bt, frame) ||
@@ -3814,21 +3912,27 @@ static void
 arm64_get_stack_frame(struct bt_info *bt, ulong *pcp, ulong *spp)
 {
        int ret;
+       struct user_regs_bitmap_struct *ur_bitmap;
        struct arm64_stackframe stackframe = { 0 };

        if (DUMPFILE() && is_task_active(bt->task)) {
                ret = arm64_get_dumpfile_stackframe(bt, &stackframe);
+               bt->need_free = FALSE;
        } else {
                if (bt->flags & BT_SKIP_IDLE)
                        bt->flags &= ~BT_SKIP_IDLE;

                ret = arm64_get_stackframe(bt, &stackframe);
-       }

-       if (!ret)
-               error(WARNING,
-                       "cannot determine starting stack frame for task %lx\n",
-                               bt->task);
+                ur_bitmap = (struct user_regs_bitmap_struct *)GETBUF(sizeof(*ur_bitmap));
+                memset(ur_bitmap, 0, sizeof(*ur_bitmap));
+                ur_bitmap->pc = stackframe.pc;
+                ur_bitmap->sp = stackframe.sp;
+                ur_bitmap->regs[29] = stackframe.fp;
+                ur_bitmap->bitmap += 0x1a0000000;
+                bt->machdep = ur_bitmap;
+                bt->need_free = TRUE;
+       }

        bt->frameptr = stackframe.fp;
        if (pcp)
diff --git a/defs.h b/defs.h
index fd00462..05d8e14 100644
--- a/defs.h
+++ b/defs.h
@@ -8073,6 +8073,42 @@ enum x86_64_regnum {
         LAST_REGNUM
 };

+enum arm64_regnum {
+       X0_REGNUM,
+       X1_REGNUM,
+       X2_REGNUM,
+       X3_REGNUM,
+       X4_REGNUM,
+       X5_REGNUM,
+       X6_REGNUM,
+       X7_REGNUM,
+       X8_REGNUM,
+       X9_REGNUM,
+       X10_REGNUM,
+       X11_REGNUM,
+       X12_REGNUM,
+       X13_REGNUM,
+       X14_REGNUM,
+       X15_REGNUM,
+       X16_REGNUM,
+       X17_REGNUM,
+       X18_REGNUM,
+       X19_REGNUM,
+       X20_REGNUM,
+       X21_REGNUM,
+       X22_REGNUM,
+       X23_REGNUM,
+       X24_REGNUM,
+       X25_REGNUM,
+       X26_REGNUM,
+       X27_REGNUM,
+       X28_REGNUM,
+       X29_REGNUM,
+       X30_REGNUM,
+       SP_REGNUM,
+       PC_REGNUM,
+};
+
 /*
  * Register numbers to make crash_target->fetch_registers()
  * ---> machdep->get_current_task_reg() work properly.
--
2.40.1