On Mon, Jun 3, 2024 at 10:15 AM <devel-requ...@lists.crash-utility.osci.io>
wrote:
> Date: Fri, 31 May 2024 17:19:39 +0800
> From: Tao Liu <l...@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 <mah...@linux.ibm.com>, "Naveen N . Rao"
> <naveen.n....@linux.vnet.ibm.com>, Lianbo Jiang <
> liji...@redhat.com>,
> Alexey Makhalov <alexey.makha...@broadcom.com>
> Message-ID: <20240531091939.97828-17-l...@redhat.com>
> Content-Type: text/plain; charset=UTF-8
>
> Cc: Sourabh Jain <sourabhj...@linux.ibm.com>
> Cc: Hari Bathini <hbath...@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mah...@linux.ibm.com>
> Cc: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
> Cc: Lianbo Jiang <liji...@redhat.com>
> Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com>
> Cc: Tao Liu <l...@redhat.com>
> Cc: Alexey Makhalov <alexey.makha...@broadcom.com>
> Signed-off-by: Tao Liu <l...@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
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
