From: Aditya Gupta <adityag(a)linux.ibm.com&gt; Currently, 'frame' is prohibited on every architecture, even in gdb mode. But with gdb stack unwinding working correctly on some architectures (eg. ppc64, x86_64, arm64), so remove it from prohibited list, so that 'frame' command can be used This has an implication on all architectures, that 'frame' now outputs this in default mode: ``` crash> frame #0 <unavailable> in ?? () ``` instead of the before prohibited message ``` crash> frame crash: prohibited gdb command: frame ``` Though, 'frame' should work in gdb mode for architectures that implement 'machdep->get_cpu_reg' Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; --- gdb_interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb_interface.c b/gdb_interface.c index b14319c..8f99a0d 100644 --- a/gdb_interface.c +++ b/gdb_interface.c @@ -711,7 +711,7 @@ static char *prohibited_list[] = { "watch", "rwatch", "awatch", "attach", "continue", "c", "fg", "detach", "finish", "handle", "interrupt", "jump", "kill", "next", "nexti", "signal", "step", "s", "stepi", "target", "until", "delete", - "clear", "disable", "enable", "condition", "ignore", "frame", "catch", + "clear", "disable", "enable", "condition", "ignore", "catch", "tcatch", "return", "file", "exec-file", "core-file", "symbol-file", "load", "si", "ni", "shell", "sy", NULL /* must be last */ -- 2.40.1

This patch is a preparation of gdb stack unwinding support. "set" command is extended with gdb context change: crash> set <pid> or crash> set <task> Then the task context of crash and gdb will both be switched to pid/task, and the following command: "bt" "gdb bt" will output the same task context. Co-developed-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; Co-developed-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Co-developed-by: Tao Liu <ltao(a)redhat.com&gt; Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- crash_target.c | 46 ++++++++++++++++++++++++++++++---------------- defs.h | 5 ++++- kernel.c | 11 ++++++++--- task.c | 21 +++++++++++++-------- tools.c | 8 ++++---- 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/crash_target.c b/crash_target.c index 1f62bf6..41f0f3a 100644 --- a/crash_target.c +++ b/crash_target.c @@ -28,7 +28,7 @@ void crash_target_init (void); extern "C" int gdb_readmem_callback(unsigned long, void *, int, int); extern "C" int crash_get_cpu_reg (int cpu, int regno, const char *regname, int regsize, void *val); - +extern "C" int gdb_change_thread_context (void); /* The crash target. */ @@ -63,26 +63,32 @@ public: }; -/* We just get all the registers, so we don't use regno. */ -void -crash_target::fetch_registers (struct regcache *regcache, int regno) +static void supply_registers(struct regcache *regcache, int regno) { gdb_byte regval[16]; int cpu = inferior_ptid.tid(); struct gdbarch *arch = regcache->arch (); + const char *regname = gdbarch_register_name(arch, regno); + int regsize = register_size(arch, regno); - for (int r = 0; r < gdbarch_num_regs (arch); r++) - { - const char *regname = gdbarch_register_name(arch, r); - int regsize = register_size (arch, r); - if (regsize > sizeof (regval)) - error (_("fatal error: buffer size is not enough to fit register value")); - - if (crash_get_cpu_reg (cpu, r, regname, regsize, (void *)&regval)) - regcache->raw_supply (r, regval); - else - regcache->raw_supply (r, NULL); - } + if (regsize > sizeof (regval)) + error (_("fatal error: buffer size is not enough to fit register value")); + + if (crash_get_cpu_reg (cpu, regno, regname, regsize, (void *)&regval)) + regcache->raw_supply (regno, regval); + else + regcache->raw_supply (regno, NULL); +} + +void +crash_target::fetch_registers (struct regcache *regcache, int regno) +{ + if (regno >= 0) { + supply_registers(regcache, regno); + } else if (regno == -1) { + for (int r = 0; r < gdbarch_num_regs (regcache->arch ()); r++) + supply_registers(regcache, r); + } } @@ -129,3 +135,11 @@ crash_target_init (void) /* Now, set up the frame cache. */ reinit_frame_cache (); } + +extern "C" int +gdb_change_thread_context (void) +{ + target_fetch_registers(get_current_regcache(), -1); + reinit_frame_cache(); + return TRUE; +} diff --git a/defs.h b/defs.h index 49e6923..9ffe7c9 100644 --- a/defs.h +++ b/defs.h @@ -6019,7 +6019,7 @@ extern char *help_map[]; * task.c */ void task_init(void); -int set_context(ulong, ulong); +int set_context(ulong, ulong, uint); void show_context(struct task_context *); ulong pid_to_task(ulong); ulong task_to_pid(ulong); @@ -8055,4 +8055,7 @@ enum x86_64_regnum { LAST_REGNUM }; +/* crash_target.c */ +extern int gdb_change_thread_context (void); + #endif /* !GDB_COMMON */ diff --git a/kernel.c b/kernel.c index 8a9d498..5d25b88 100644 --- a/kernel.c +++ b/kernel.c @@ -6507,15 +6507,20 @@ set_cpu(int cpu) if (hide_offline_cpu(cpu)) error(FATAL, "invalid cpu number: cpu %d is OFFLINE\n", cpu); - if ((task = get_active_task(cpu))) - set_context(task, NO_PID); + task = get_active_task(cpu); + + /* Check if context is already set to given cpu */ + if (task == CURRENT_TASK()) + return; + + if (task) + set_context(task, NO_PID, TRUE); else error(FATAL, "cannot determine active task on cpu %ld\n", cpu); show_context(CURRENT_CONTEXT()); } - /* * Collect the irq_desc[] entry along with its associated handler and * action structures. diff --git a/task.c b/task.c index d52ce0b..697a692 100644 --- a/task.c +++ b/task.c @@ -679,7 +679,7 @@ task_init(void) if (ACTIVE()) { active_pid = REMOTE() ? pc->server_pid : LOCAL_ACTIVE() ? pc->program_pid : 1; - set_context(NO_TASK, active_pid); + set_context(NO_TASK, active_pid, FALSE); tt->this_task = pid_to_task(active_pid); } else { @@ -691,7 +691,7 @@ task_init(void) else if (ELF_NOTES_VALID() && DISKDUMP_DUMPFILE()) map_cpus_to_prstatus_kdump_cmprs(); please_wait("determining panic task"); - set_context(get_panic_context(), NO_PID); + set_context(get_panic_context(), NO_PID, TRUE); please_wait_done(); } @@ -2992,9 +2992,9 @@ refresh_context(ulong curtask, ulong curpid) struct task_context *tc; if (task_exists(curtask) && pid_exists(curpid)) { - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, FALSE); } else { - set_context(tt->this_task, NO_PID); + set_context(tt->this_task, NO_PID, FALSE); complain = TRUE; if (STREQ(args[0], "set") && (argcnt == 2) && @@ -3060,7 +3060,7 @@ sort_context_array(void) curtask = CURRENT_TASK(); qsort((void *)tt->context_array, (size_t)tt->running_tasks, sizeof(struct task_context), sort_by_pid); - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, TRUE); sort_context_by_task(); } @@ -3107,7 +3107,7 @@ sort_context_array_by_last_run(void) curtask = CURRENT_TASK(); qsort((void *)tt->context_array, (size_t)tt->running_tasks, sizeof(struct task_context), sort_by_last_run); - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, TRUE); sort_context_by_task(); } @@ -5288,7 +5288,7 @@ comm_exists(char *s) * that pid is selected. */ int -set_context(ulong task, ulong pid) +set_context(ulong task, ulong pid, uint update_gdb_thread) { int i; struct task_context *tc; @@ -5308,7 +5308,12 @@ set_context(ulong task, ulong pid) if (found) { CURRENT_CONTEXT() = tc; - return TRUE; + + /* change the selected thread in gdb, according to current context */ + if (update_gdb_thread) + return gdb_change_thread_context(); + else + return TRUE; } else { if (task) error(INFO, "cannot set context for task: %lx\n", task); diff --git a/tools.c b/tools.c index 1022d57..a8f9785 100644 --- a/tools.c +++ b/tools.c @@ -1871,7 +1871,7 @@ cmd_set(void) return; if (ACTIVE()) { - set_context(tt->this_task, NO_PID); + set_context(tt->this_task, NO_PID, TRUE); show_context(CURRENT_CONTEXT()); return; } @@ -1880,7 +1880,7 @@ cmd_set(void) error(INFO, "no panic task found!\n"); return; } - set_context(tt->panic_task, NO_PID); + set_context(tt->panic_task, NO_PID, TRUE); show_context(CURRENT_CONTEXT()); return; @@ -2559,14 +2559,14 @@ cmd_set(void) case STR_PID: pid = value; task = NO_TASK; - if (set_context(task, pid)) + if (set_context(task, pid, TRUE)) show_context(CURRENT_CONTEXT()); break; case STR_TASK: task = value; pid = NO_PID; - if (set_context(task, pid)) + if (set_context(task, pid, TRUE)) show_context(CURRENT_CONTEXT()); break; -- 2.40.1

This patch is a preparation of gdb stack unwinding support. "info threads" will output useful info for current task, such as pid, command, stack frame. Before the patch: crash> info threads Id Target Id Frame * 1 CPU 0 crash_setup_regs (oldregs=0x0, newregs=0xffffbd1040c57c70) ... After the patch: crash> info threads Id Target Id Frame * 1 1231 bash crash_setup_regs (oldregs=0x0, newregs=0xffffbd1040c57c70) ... Co-developed-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; Co-developed-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Co-developed-by: Tao Liu <ltao(a)redhat.com&gt; Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- crash_target.c | 8 +++++++- task.c | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/crash_target.c b/crash_target.c index 6ee12d2..1080976 100644 --- a/crash_target.c +++ b/crash_target.c @@ -29,6 +29,7 @@ extern "C" int gdb_readmem_callback(unsigned long, void *, int, int); extern "C" int crash_get_current_task_reg (int regno, const char *regname, int regsize, void *val); extern "C" int gdb_change_thread_context (void); +extern "C" void crash_get_current_task_info(unsigned long *pid, char **comm); /* The crash target. */ @@ -59,7 +60,12 @@ public: bool has_registers () override { return true; } bool thread_alive (ptid_t ptid) override { return true; } std::string pid_to_str (ptid_t ptid) override - { return string_printf ("CPU %ld", ptid.tid ()); } + { + unsigned long pid; + char *comm; + crash_get_current_task_info(&pid, &comm); + return string_printf ("%ld %s", pid, comm); + } }; diff --git a/task.c b/task.c index 697a692..b91acb2 100644 --- a/task.c +++ b/task.c @@ -120,6 +120,7 @@ static int has_sched_policy(ulong, ulong); static ulong task_policy(ulong); static ulong sched_policy_bit_from_str(const char *); static ulong make_sched_policy(const char *); +void crash_get_current_task_info(unsigned long *, char **); static struct sched_policy_info { ulong value; @@ -11292,3 +11293,11 @@ check_stack_end_magic: if (!total) fprintf(fp, "No stack overflows detected\n"); } + +void crash_get_current_task_info(unsigned long *pid, char **comm) +{ + struct task_context *tc = CURRENT_CONTEXT(); + + *pid = tc->pid; + *comm = tc->comm; +} -- 2.40.1

Hi Tao, Sorry to be a bother, but the author of this patch changed again. Maybe while rebasing and editing the patch series, this might have happened, thanks for sending out this series again though. Thanks, Aditya Gupta On 29/07/24 16:14, Tao Liu wrote: > Currently for most gdb_interface call, in which a non-null file pointer > is passed, GDB's output stream is replaced with the passed file pointer > > Due to this, 'info threads', which is a gdb passthrough, doesn't print any > thread, after support was added to get registers from crash_target: > > crash> info threads > Id Target Id Frame > > This empty output of 'info threads' was due to a subtle bug in > gdb_interface. > > After this gdb passthrough is run, 'datatype_info' is called, with file > pointer set to null_fp (pointing to /dev/null). And after > 'datatype_info' returns, any output by gdb goes to /dev/null, and hence > the output is lost. > > Fix this by restoring the original output streams, after gdb_interface > has handled the output > > After this patch: > > crash> info threads > Id Target Id Frame > * 1 2131 bash 0xc000000000051e40 in crash_fadump (regs=0x0, str=0xc000000002c60510 <buf> "sysrq triggered crash") at arch/powerpc/kernel/fadump.c:735 > > Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; > Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; > Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; > Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; > Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; > Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; > Cc: Tao Liu <ltao(a)redhat.com&gt; > Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; > Signed-off-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > gdb-10.2.patch | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > index 3694b13..0bed96a 100644 > --- a/gdb-10.2.patch > +++ b/gdb-10.2.patch > @@ -16118,3 +16118,56 @@ exit 0 > subclass (SYMBOL_NONE) > { > /* We can't use an initializer list for members of a base class, and > +--- gdb-10.2/gdb/ui-file.h.orig > ++++ gdb-10.2/gdb/ui-file.h > +@@ -195,6 +195,7 @@ public: > + > + bool can_emit_style_escape () override; > + > ++ FILE *get_stream(void); > + /* Sets the internal stream to FILE, and saves the FILE's file > + descriptor in M_FD. */ > + void set_stream (FILE *file); > +--- gdb-10.2/gdb/ui-file.c.orig > ++++ gdb-10.2/gdb/ui-file.c > +@@ -161,6 +161,12 @@ stdio_file::~stdio_file () > + fclose (m_file); > + } > + > ++FILE* > ++stdio_file::get_stream(void) > ++{ > ++ return m_file; > ++} > ++ > + void > + stdio_file::set_stream (FILE *file) > + { > +--- gdb-10.2/gdb/symtab.c.orig > ++++ gdb-10.2/gdb/symtab.c > +@@ -6964,8 +6964,12 @@ void > + gdb_command_funnel_1(struct gnu_request *req) > + { > + struct symbol *sym; > ++ FILE *original_stdout_stream = nullptr; > ++ FILE *original_stderr_stream = nullptr; > + > + if (req->command != GNU_VERSION && req->command != GNU_USER_PRINT_OPTION) { > ++ original_stdout_stream = (dynamic_cast< stdio_file * >gdb_stdout)->get_stream(); > ++ original_stderr_stream = (dynamic_cast< stdio_file * >gdb_stderr)->get_stream(); > + (dynamic_cast<stdio_file *>gdb_stdout)->set_stream(req->fp); > + (dynamic_cast<stdio_file *>gdb_stderr)->set_stream(req->fp); > + } > +@@ -7068,6 +7072,12 @@ gdb_command_funnel_1(struct gnu_request *req) > + req->flags |= GNU_COMMAND_FAILED; > + break; > + } > ++ > ++ /* Restore the streams gdb output was using */ > ++ if (original_stdout_stream) > ++ (dynamic_cast<stdio_file *>gdb_stdout)->set_stream(original_stdout_stream); > ++ if (original_stderr_stream) > ++ (dynamic_cast<stdio_file *>gdb_stderr)->set_stream(original_stderr_stream); > + } > + > + /*

The stack unwinding is for kernel addresses only. If non-kernel address encountered, it is usually a user space address, or non-address value like a function call parameter. So stopping stack unwinding at non-kernel address will decrease the invalid unwind results. Before: crash> gdb bt #0 0xffffffff816a8f65 in context_switch ... #1 __schedule () ... #2 0xffffffff816a94e9 in schedule ... #3 0xffffffff816a86fd in schedule_hrtimeout_range_clock ... #4 0xffffffff816a8733 in schedule_hrtimeout_range ... #5 0xffffffff8124bb7e in ep_poll ... #6 0xffffffff8124d00d in SYSC_epoll_wait ... #7 SyS_epoll_wait ... #8 <signal handler called> #9 0x00007f0449407923 in ?? () #10 0xffff880100000001 in ?? () #11 0xffff880169b3c010 in ?? () #12 0x0000000000000040 in irq_stack_union () #13 0xffff880169b3c058 in ?? () #14 0xffff880169b3c048 in ?? () #15 0xffff880169b3c050 in ?? () #16 0x0000000000000000 in ?? () After: crash> gdb bt #0 0xffffffff816a8f65 in context_switch ... #1 __schedule () ... #2 0xffffffff816a94e9 in schedule () ... #3 0xffffffff816a86fd in schedule_hrtimeout_range_clock ... #4 0xffffffff816a8733 in schedule_hrtimeout_range ... #5 0xffffffff8124bb7e in ep_poll ... #6 0xffffffff8124d00d in SYSC_epoll_wait ... #7 SyS_epoll_wait ... #8 <signal handler called> Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- defs.h | 1 + gdb-10.2.patch | 26 ++++++++++++++++++++++++++ gdb_interface.c | 6 ++++++ 3 files changed, 33 insertions(+) diff --git a/defs.h b/defs.h index 82c2af8..f18d781 100644 --- a/defs.h +++ b/defs.h @@ -7906,6 +7906,7 @@ extern unsigned char *gdb_prettyprint_arrays; extern unsigned int *gdb_repeat_count_threshold; extern unsigned char *gdb_stop_print_at_null; extern unsigned int *gdb_output_radix; +int is_kvaddr(ulong); /* * gdb/top.c diff --git a/gdb-10.2.patch b/gdb-10.2.patch index 0bed96a..2a52a8b 100644 --- a/gdb-10.2.patch +++ b/gdb-10.2.patch @@ -16171,3 +16171,29 @@ exit 0 } /* +--- gdb-10.2/gdb/stack.c.orig ++++ gdb-10.2/gdb/stack.c +@@ -1990,6 +1990,10 @@ + /* Print briefly all stack frames or just the innermost COUNT_EXP + frames. */ + ++#ifdef CRASH_MERGE ++extern "C" int is_kvaddr(ulong); ++#endif ++ + static void + backtrace_command_1 (const frame_print_options &fp_opts, + const backtrace_cmd_options &bt_opts, +@@ -2082,6 +2086,12 @@ + hand, perhaps the code does or could be fixed to make sure + the frame->prev field gets set to NULL in that case). */ + ++ CORE_ADDR pc = 0; ++ get_frame_pc_if_available (fi, &pc); ++ if (!is_kvaddr(pc)) { ++ fi = NULL; ++ break; ++ } + print_frame_info (fp_opts, fi, 1, LOCATION, 1, 0); + if ((flags & PRINT_LOCALS) != 0) + { diff --git a/gdb_interface.c b/gdb_interface.c index 7399fd0..2965a10 100644 --- a/gdb_interface.c +++ b/gdb_interface.c @@ -947,6 +947,12 @@ gdb_lookup_module_symbol(ulong addr, ulong *offset) } } +int +is_kvaddr(ulong addr) +{ + return IS_KVADDR(addr); +} + /* * Used by gdb_interface() to catch gdb-related errors, if desired. */ -- 2.40.1

This patch will add stack unwinding support for x86_64 CPU arch. It follows the tech path of ppc64_get_stack_frame() & ppc64_get_cpu_reg() in ppc64.c, to get and consume the cpu regs. One different case is, we need to handle inactive_task_frame structure specifically in x86_64. If inactive_task_frame structure enabled, for inactive tasks, 7 regs can be get from inactive_task_frame in stack, and sp need to rewind back to skip inactive_task_frame structure (See code comments in x86_64.c:x86_64_get_stack_frame()); for active tasks, we get regs by the original way. If inactive_task_frame structure is not enabled, for inactive tasks, the stack frame is organized as linked list, and sp/bp can be get from stack; for active tasks, we get regs by the original way. vmware_vmss_get_cpu_reg() whould be called only for active tasks to get their registers from corresponding CPUs. Otherwise, the standard path of fetching pt_regs from the memory (inactive_task_frame) should be used. Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; Signed-off-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; --- defs.h | 19 +++- symbols.c | 14 +++ x86_64.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 320 insertions(+), 30 deletions(-) diff --git a/defs.h b/defs.h index 49ace67..d79f5bc 100644 --- a/defs.h +++ b/defs.h @@ -2244,6 +2244,20 @@ struct offset_table { /* stash of commonly-used offsets */ long vmap_node_busy; long rb_list_head; long file_f_inode; + long inactive_task_frame_r15; + long inactive_task_frame_r14; + long inactive_task_frame_r13; + long inactive_task_frame_r12; + long inactive_task_frame_flags; + long inactive_task_frame_si; + long inactive_task_frame_di; + long inactive_task_frame_bx; + long thread_struct_es; + long thread_struct_ds; + long thread_struct_fsbase; + long thread_struct_gsbase; + long thread_struct_fs; + long thread_struct_gs; }; struct size_table { /* stash of commonly-used sizes */ @@ -8019,7 +8033,7 @@ extern int have_full_symbols(void); /* * Register numbers must be in sync with gdb/features/i386/64bit-core.c - * to make crash_target->fetch_registers() ---> machdep->get_cpu_reg() + * to make crash_target->fetch_registers() ---> machdep->get_current_task_reg() * working properly. */ enum x86_64_regnum { @@ -8063,6 +8077,9 @@ enum x86_64_regnum { FOSEG_REGNUM, FOOFF_REGNUM, FOP_REGNUM, + FS_BASE_REGNUM = 152, + GS_BASE_REGNUM, + ORIG_RAX_REGNUM, LAST_REGNUM }; diff --git a/symbols.c b/symbols.c index 69a1fbb..927c5ad 100644 --- a/symbols.c +++ b/symbols.c @@ -11813,6 +11813,20 @@ dump_offset_table(char *spec, ulong makestruct) fprintf(fp, " zs_pool_size_class: %ld\n", OFFSET(zs_pool_size_class)); fprintf(fp, " size_class_size: %ld\n", OFFSET(size_class_size)); fprintf(fp, " zspage_huge: %ld\n", OFFSET(zspage_huge)); + fprintf(fp, " inactive_task_frame_r15: %ld\n", OFFSET(inactive_task_frame_r15)); + fprintf(fp, " inactive_task_frame_r14: %ld\n", OFFSET(inactive_task_frame_r14)); + fprintf(fp, " inactive_task_frame_r13: %ld\n", OFFSET(inactive_task_frame_r13)); + fprintf(fp, " inactive_task_frame_r12: %ld\n", OFFSET(inactive_task_frame_r12)); + fprintf(fp, " inactive_task_frame_flags: %ld\n", OFFSET(inactive_task_frame_flags)); + fprintf(fp, " inactive_task_frame_si: %ld\n", OFFSET(inactive_task_frame_si)); + fprintf(fp, " inactive_task_frame_di: %ld\n", OFFSET(inactive_task_frame_di)); + fprintf(fp, " inactive_task_frame_bx: %ld\n", OFFSET(inactive_task_frame_bx)); + fprintf(fp, " thread_struct_es: %ld\n", OFFSET(thread_struct_es)); + fprintf(fp, " thread_struct_ds: %ld\n", OFFSET(thread_struct_ds)); + fprintf(fp, " thread_struct_fsbase: %ld\n", OFFSET(thread_struct_fsbase)); + fprintf(fp, " thread_struct_gsbase: %ld\n", OFFSET(thread_struct_gsbase)); + fprintf(fp, " thread_struct_fs: %ld\n", OFFSET(thread_struct_fs)); + fprintf(fp, " thread_struct_gs: %ld\n", OFFSET(thread_struct_gs)); fprintf(fp, "\n size_table:\n"); fprintf(fp, " page: %ld\n", SIZE(page)); diff --git a/x86_64.c b/x86_64.c index 24a7040..145436f 100644 --- a/x86_64.c +++ b/x86_64.c @@ -126,7 +126,7 @@ static int x86_64_get_framesize(struct bt_info *, ulong, ulong, char *); static void x86_64_framesize_debug(struct bt_info *); static void x86_64_get_active_set(void); static int x86_64_get_kvaddr_ranges(struct vaddr_range *); -static int x86_64_get_cpu_reg(int, int, const char *, int, void *); +static int x86_64_get_current_task_reg(int, const char *, int, void *); static int x86_64_verify_paddr(uint64_t); static void GART_init(void); static void x86_64_exception_stacks_init(void); @@ -143,6 +143,23 @@ struct machine_specific x86_64_machine_specific = { 0 }; static const char *exception_functions_orig[]; static const char *exception_functions_5_8[]; +/* Use this hardwired version -- sometimes the + * debuginfo doesn't pick this up even though + * it exists in the kernel; it shouldn't change. + */ +struct x86_64_user_regs_struct { + unsigned long r15, r14, r13, r12, bp, bx; + unsigned long r11, r10, r9, r8, ax, cx, dx; + unsigned long si, di, orig_ax, ip, cs; + unsigned long flags, sp, ss, fs_base; + unsigned long gs_base, ds, es, fs, gs; +}; + +struct user_regs_bitmap_struct { + struct x86_64_user_regs_struct ur; + ulong bitmap[32]; +}; + /* * Do all necessary machine-specific setup here. This is called several * times during initialization. @@ -195,7 +212,7 @@ x86_64_init(int when) machdep->machspec->irq_eframe_link = UNINITIALIZED; machdep->machspec->irq_stack_gap = UNINITIALIZED; machdep->get_kvaddr_ranges = x86_64_get_kvaddr_ranges; - machdep->get_current_task_reg = x86_64_get_cpu_reg; + machdep->get_current_task_reg = x86_64_get_current_task_reg; if (machdep->cmdline_args[0]) parse_cmdline_args(); if ((string = pc->read_vmcoreinfo("relocate"))) { @@ -500,6 +517,12 @@ x86_64_init(int when) MEMBER_OFFSET_INIT(thread_struct_rsp, "thread_struct", "sp"); if (INVALID_MEMBER(thread_struct_rsp0)) MEMBER_OFFSET_INIT(thread_struct_rsp0, "thread_struct", "sp0"); + MEMBER_OFFSET_INIT(thread_struct_es, "thread_struct", "es"); + MEMBER_OFFSET_INIT(thread_struct_ds, "thread_struct", "ds"); + MEMBER_OFFSET_INIT(thread_struct_fsbase, "thread_struct", "fsbase"); + MEMBER_OFFSET_INIT(thread_struct_gsbase, "thread_struct", "gsbase"); + MEMBER_OFFSET_INIT(thread_struct_fs, "thread_struct", "fs"); + MEMBER_OFFSET_INIT(thread_struct_gs, "thread_struct", "gs"); STRUCT_SIZE_INIT(tss_struct, "tss_struct"); MEMBER_OFFSET_INIT(tss_struct_ist, "tss_struct", "ist"); if (INVALID_MEMBER(tss_struct_ist)) { @@ -583,17 +606,6 @@ x86_64_init(int when) "user_regs_struct", "r15"); STRUCT_SIZE_INIT(user_regs_struct, "user_regs_struct"); if (!VALID_STRUCT(user_regs_struct)) { - /* Use this hardwired version -- sometimes the - * debuginfo doesn't pick this up even though - * it exists in the kernel; it shouldn't change. - */ - struct x86_64_user_regs_struct { - unsigned long r15, r14, r13, r12, bp, bx; - unsigned long r11, r10, r9, r8, ax, cx, dx; - unsigned long si, di, orig_ax, ip, cs; - unsigned long flags, sp, ss, fs_base; - unsigned long gs_base, ds, es, fs, gs; - }; ASSIGN_SIZE(user_regs_struct) = sizeof(struct x86_64_user_regs_struct); ASSIGN_OFFSET(user_regs_struct_rip) = @@ -891,7 +903,7 @@ x86_64_dump_machdep_table(ulong arg) fprintf(fp, " is_page_ptr: x86_64_is_page_ptr()\n"); fprintf(fp, " verify_paddr: x86_64_verify_paddr()\n"); fprintf(fp, " get_kvaddr_ranges: x86_64_get_kvaddr_ranges()\n"); - fprintf(fp, " get_cpu_reg: x86_64_get_cpu_reg()\n"); + fprintf(fp, "get_current_task_reg: x86_64_get_current_task_reg()\n"); fprintf(fp, " init_kernel_pgd: x86_64_init_kernel_pgd()\n"); fprintf(fp, "clear_machdep_cache: x86_64_clear_machdep_cache()\n"); fprintf(fp, " xendump_p2m_create: %s\n", PVOPS_XEN() ? @@ -4973,22 +4985,143 @@ x86_64_eframe_verify(struct bt_info *bt, long kvaddr, long cs, long ss, return FALSE; } +static ulong +get_reg_from_inactive_task_frame(struct bt_info *bt, char *reg_name, + ulong reg_offset) +{ + ulong reg_value = 0, rsp = 0; + char reg_info[64]; + + snprintf(reg_info, sizeof(reg_info), "inactive_task_frame.%s", reg_name); + + readmem(bt->task + OFFSET(task_struct_thread) + + OFFSET(thread_struct_rsp), KVADDR, &rsp, + sizeof(ulong), "thread_struct.rsp", FAULT_ON_ERROR); + readmem(rsp + reg_offset, KVADDR, &reg_value, + sizeof(ulong), reg_info, FAULT_ON_ERROR); + return reg_value; +} + +#define SET_REG_BITMAP(REGMAP, TYPE, MEMBER) \ + SET_BIT(REGMAP, REG_SEQ(TYPE, MEMBER)) + /* * Get a stack frame combination of pc and ra from the most relevent spot. */ static void x86_64_get_stack_frame(struct bt_info *bt, ulong *pcp, ulong *spp) { - if (bt->flags & BT_DUMPFILE_SEARCH) - return x86_64_get_dumpfile_stack_frame(bt, pcp, spp); + struct user_regs_bitmap_struct *ur_bitmap; + ulong pcp_save = *pcp; + ulong spp_save = *spp; + ulong sp; if (bt->flags & BT_SKIP_IDLE) bt->flags &= ~BT_SKIP_IDLE; + if (pcp) + *pcp = x86_64_get_pc(bt); + if (spp) + sp = *spp = x86_64_get_sp(bt); + ur_bitmap = (struct user_regs_bitmap_struct *)GETBUF(sizeof(*ur_bitmap)); + memset(ur_bitmap, 0, sizeof(*ur_bitmap)); + + if (VALID_MEMBER(inactive_task_frame_bp)) { + if (!is_task_active(bt->task)) { + /* + * For inactive tasks in live and dumpfile, regs can be + * get from inactive_task_frame struct. + */ + ur_bitmap->ur.r15 = get_reg_from_inactive_task_frame(bt, + "r15", OFFSET(inactive_task_frame_r15)); + ur_bitmap->ur.r14 = get_reg_from_inactive_task_frame(bt, + "r14", OFFSET(inactive_task_frame_r14)); + ur_bitmap->ur.r13 = get_reg_from_inactive_task_frame(bt, + "r13", OFFSET(inactive_task_frame_r13)); + ur_bitmap->ur.r12 = get_reg_from_inactive_task_frame(bt, + "r12", OFFSET(inactive_task_frame_r12)); + ur_bitmap->ur.bx = get_reg_from_inactive_task_frame(bt, + "bx", OFFSET(inactive_task_frame_bx)); + ur_bitmap->ur.bp = get_reg_from_inactive_task_frame(bt, + "bp", OFFSET(inactive_task_frame_bp)); + /* + For inactive tasks: + crash> task -x 1|grep sp + sp = 0xffffc90000013d00 + crash> rd ffffc90000013d00 32 + ffffc90000013d00: ffff888104dad4a8 0000000000000000 r15,r14 + ffffc90000013d10: ffff888100280000 ffff888100216500 r13,r12 + ffffc90000013d20: ffff888100217018 ffff88817fd2c800 rbx,rbp + ffffc90000013d30: ffffffff81a6a1b3 ffffc90000013de0 saved_rip,... + ffffc90000013d40: ffff888100000004 99ccbf53ea493000 + ffffc90000013d50: ffff888100216500 ffff888100216500 + + crash> dis __schedule + ... + 0xffffffff81a6a1ab <__schedule+507>: mov %r13,%rsi + 0xffffffff81a6a1ae <__schedule+510>: call 0xffffffff81003490 <__switch_to_asm> + 0xffffffff81a6a1b3 <__schedule+515>: mov %rax,%rdi <<=== saved_rip + ... + crash> dis __switch_to_asm + 0xffffffff81003490 <__switch_to_asm>: push %rbp + 0xffffffff81003491 <__switch_to_asm+1>: push %rbx + 0xffffffff81003492 <__switch_to_asm+2>: push %r12 + 0xffffffff81003494 <__switch_to_asm+4>: push %r13 + 0xffffffff81003496 <__switch_to_asm+6>: push %r14 + 0xffffffff81003498 <__switch_to_asm+8>: push %r15 + 0xffffffff8100349a <__switch_to_asm+10>: mov %rsp,0x14d8(%rdi) + ... + Now saved_rip = ffffffff81a6a1b3, and we are starting + the stack unwind at saved_rip, which is function __schedule() + instead of function __switch_to_asm(), so the stack pointer should + be rewind from ffffc90000013d00 back to ffffc90000013d38, + aka *spp += 7 * reg_len. Otherwise we are unwinding function + __schedule() but with __switch_to_asm()'s stack frame, which + will fail. + */ + sp += 7 * sizeof(unsigned long); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, r15); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, r14); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, r13); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, r12); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, bx); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, bp); + } else { + /* + * For active tasks in dumpfile, we get regs through the + * original way. For active tasks in live, we only get + * ip and sp in the end of the function. + */ + if (bt->flags & BT_DUMPFILE_SEARCH) { + FREEBUF(ur_bitmap); + bt->need_free = FALSE; + *pcp = pcp_save; + *spp = spp_save; + return x86_64_get_dumpfile_stack_frame(bt, pcp, spp); + } + } + } else { + if (!is_task_active(bt->task)) { + readmem(*spp, KVADDR, &(ur_bitmap->ur.bp), + sizeof(ulong), "ur_bitmap->ur.bp", FAULT_ON_ERROR); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, bp); + } else { + if (bt->flags & BT_DUMPFILE_SEARCH) { + FREEBUF(ur_bitmap); + bt->need_free = FALSE; + *pcp = pcp_save; + *spp = spp_save; + return x86_64_get_dumpfile_stack_frame(bt, pcp, spp); + } + } + } - if (pcp) - *pcp = x86_64_get_pc(bt); - if (spp) - *spp = x86_64_get_sp(bt); + ur_bitmap->ur.ip = *pcp; + ur_bitmap->ur.sp = sp; + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, ip); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, sp); + + bt->machdep = ur_bitmap; + bt->need_free = TRUE; } /* @@ -6401,6 +6534,17 @@ x86_64_ORC_init(void) }; struct ORC_data *orc; + MEMBER_OFFSET_INIT(inactive_task_frame_bp, "inactive_task_frame", "bp"); + MEMBER_OFFSET_INIT(inactive_task_frame_ret_addr, "inactive_task_frame", "ret_addr"); + MEMBER_OFFSET_INIT(inactive_task_frame_r15, "inactive_task_frame", "r15"); + MEMBER_OFFSET_INIT(inactive_task_frame_r14, "inactive_task_frame", "r14"); + MEMBER_OFFSET_INIT(inactive_task_frame_r13, "inactive_task_frame", "r13"); + MEMBER_OFFSET_INIT(inactive_task_frame_r12, "inactive_task_frame", "r12"); + MEMBER_OFFSET_INIT(inactive_task_frame_flags, "inactive_task_frame", "flags"); + MEMBER_OFFSET_INIT(inactive_task_frame_si, "inactive_task_frame", "si"); + MEMBER_OFFSET_INIT(inactive_task_frame_di, "inactive_task_frame", "di"); + MEMBER_OFFSET_INIT(inactive_task_frame_bx, "inactive_task_frame", "bx"); + if (machdep->flags & FRAMEPOINTER) return; @@ -6458,9 +6602,6 @@ x86_64_ORC_init(void) orc->__stop_orc_unwind = symbol_value("__stop_orc_unwind"); orc->orc_lookup = symbol_value("orc_lookup"); - MEMBER_OFFSET_INIT(inactive_task_frame_bp, "inactive_task_frame", "bp"); - MEMBER_OFFSET_INIT(inactive_task_frame_ret_addr, "inactive_task_frame", "ret_addr"); - orc->has_signal = MEMBER_EXISTS("orc_entry", "signal"); /* added at 6.3 */ orc->has_end = MEMBER_EXISTS("orc_entry", "end"); /* removed at 6.4 */ @@ -9073,17 +9214,135 @@ x86_64_get_kvaddr_ranges(struct vaddr_range *vrp) return cnt; } +#define CHECK_REG_CASE(R, r) \ + case R##_REGNUM: \ + if (!NUM_IN_BITMAP(ur_bitmap->bitmap, \ + REG_SEQ(x86_64_user_regs_struct, r))) { \ + FREEBUF(ur_bitmap); \ + return FALSE; \ + } \ + break; + +#define COPY_REG_CASE(R, r) \ + case R##_REGNUM: \ + if (size != sizeof(ur_bitmap->ur.r)) \ + break; \ + memcpy(value, &ur_bitmap->ur.r, size); \ + ret = TRUE; \ + break; + static int -x86_64_get_cpu_reg(int cpu, int regno, const char *name, +x86_64_get_current_task_reg(int regno, const char *name, int size, void *value) { - if (regno >= LAST_REGNUM) - return FALSE; + 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 RAX_REGNUM ... GS_REGNUM: + case FS_BASE_REGNUM ... ORIG_RAX_REGNUM: + break; + default: + return FALSE; + } - if (VMSS_DUMPFILE()) - return vmware_vmss_get_cpu_reg(cpu, regno, name, size, value); + tc = CURRENT_CONTEXT(); + if (!tc) + return FALSE; - return FALSE; + /* + * Task is active, grab CPU's registers + */ + if (is_task_active(tc->task) && VMSS_DUMPFILE()) + return vmware_vmss_get_cpu_reg(tc->processor, regno, name, size, value); + + BZERO(&bt_setup, sizeof(struct bt_info)); + clone_bt_info(&bt_setup, &bt_info, tc); + fill_stackbuf(&bt_info); + + // reusing the get_dumpfile_regs function to get pt regs structure + 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; + + /* Get all registers from elf notes*/ + if (!bt_info.need_free) { + goto get_all; + } + + /* Get subset registers from stack frame*/ + switch (regno) { + CHECK_REG_CASE(RAX, ax); + CHECK_REG_CASE(RBX, bx); + CHECK_REG_CASE(RCX, cx); + CHECK_REG_CASE(RDX, dx); + CHECK_REG_CASE(RSI, si); + CHECK_REG_CASE(RDI, di); + CHECK_REG_CASE(RBP, bp); + CHECK_REG_CASE(RSP, sp); + CHECK_REG_CASE(R8, r8); + CHECK_REG_CASE(R9, r9); + CHECK_REG_CASE(R10, r10); + CHECK_REG_CASE(R11, r11); + CHECK_REG_CASE(R12, r12); + CHECK_REG_CASE(R13, r13); + CHECK_REG_CASE(R14, r14); + CHECK_REG_CASE(R15, r15); + CHECK_REG_CASE(RIP, ip); + CHECK_REG_CASE(EFLAGS, flags); + CHECK_REG_CASE(CS, cs); + CHECK_REG_CASE(SS, ss); + CHECK_REG_CASE(DS, ds); + CHECK_REG_CASE(ES, es); + CHECK_REG_CASE(FS, fs); + CHECK_REG_CASE(GS, gs); + CHECK_REG_CASE(FS_BASE, fs_base); + CHECK_REG_CASE(GS_BASE, gs_base); + CHECK_REG_CASE(ORIG_RAX, orig_ax); + } + +get_all: + switch (regno) { + COPY_REG_CASE(RAX, ax); + COPY_REG_CASE(RBX, bx); + COPY_REG_CASE(RCX, cx); + COPY_REG_CASE(RDX, dx); + COPY_REG_CASE(RSI, si); + COPY_REG_CASE(RDI, di); + COPY_REG_CASE(RBP, bp); + COPY_REG_CASE(RSP, sp); + COPY_REG_CASE(R8, r8); + COPY_REG_CASE(R9, r9); + COPY_REG_CASE(R10, r10); + COPY_REG_CASE(R11, r11); + COPY_REG_CASE(R12, r12); + COPY_REG_CASE(R13, r13); + COPY_REG_CASE(R14, r14); + COPY_REG_CASE(R15, r15); + COPY_REG_CASE(RIP, ip); + COPY_REG_CASE(EFLAGS, flags); + COPY_REG_CASE(CS, cs); + COPY_REG_CASE(SS, ss); + COPY_REG_CASE(DS, ds); + COPY_REG_CASE(ES, es); + COPY_REG_CASE(FS, fs); + COPY_REG_CASE(GS, gs); + COPY_REG_CASE(FS_BASE, fs_base); + COPY_REG_CASE(GS_BASE, gs_base); + COPY_REG_CASE(ORIG_RAX, orig_ax); + } + + if (bt_info.need_free) { + FREEBUF(ur_bitmap); + bt_info.need_free = FALSE; + } + return ret; } /* -- 2.40.1

There may be extra "=>" prefix before gdb disassembly, as a result, parse_line() will return string "=>" as arglist[0], which will be converted to number by htol() and fails. E.g.: crash> gdb x/40i __list_del_entry ... 0xffffffff8133c384 <__list_del_entry+36>: cmp %rcx,%rax 0xffffffff8133c387 <__list_del_entry+39>: je 0xffffffff8133c403 <__list_del_entry+163> => 0xffffffff8133c389 <__list_del_entry+41>: mov (%rax),%r8 0xffffffff8133c38c <__list_del_entry+44>: cmp %r8,%rdi 0xffffffff8133c38f <__list_del_entry+47>: jne 0xffffffff8133c3e4 <__list_del_entry+132> 0xffffffff8133c391 <__list_del_entry+49>: mov 0x8(%rdx),%r8 Before the patch: crash> bt ... #10 [ffff880095647c00] async_page_fault at ffffffff816a8638 [exception RIP: __list_del_entry+41] RIP: ffffffff8133c389 RSP: ffff880095647cb0 RFLAGS: 00010207 RAX: 0000000000000000 RBX: ffffea0400408020 RCX: dead000000000200 RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffffea0400408020 RBP: ffff880095647cb0 R8: 0000000080000431 R9: ffffffff81e835c0 R10: 0000000000000000 R11: 0000000000000400 R12: ffff880138795b58 R13: 0000000010010201 R14: ffff880095647d70 R15: 0000000400408040 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 bt: invalid input: "=>" #11 [ffff880095647cb8] list_del at ffffffff8133c43d #12 [ffff880095647cd0] devm_memremap_pages at ffffffff81180c53 After the patch: No string as 'bt: invalid input: "=>"' of output. Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- x86_64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x86_64.c b/x86_64.c index 145436f..9db10a5 100644 --- a/x86_64.c +++ b/x86_64.c @@ -8844,6 +8844,8 @@ x86_64_get_framesize(struct bt_info *bt, ulong textaddr, ulong rsp, char *stack_ rewind(pc->tmpfile2); while (fgets(buf, BUFSIZE, pc->tmpfile2)) { + if (STRNEQ(buf, "=>")) + shift_string_left(buf, 2); strcpy(buf2, buf); if (CRASHDEBUG(3)) -- 2.40.1

From: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; There are several versions of debug.guest format. Current version of the code is able to parse only version 4. Improve parser to support other known versions. Split data structures on sub-structures and introduce a helper functions to calculate a gap between them based on the version number. Implement additional data structure (struct mainmeminfo_old) and logic specifically for original (version 1) format support. Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; Cc: Tao Liu <ltao(a)redhat.com&gt; Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; Signed-off-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; --- vmware_guestdump.c | 316 ++++++++++++++++++++++++++++++++------------- 1 file changed, 229 insertions(+), 87 deletions(-) diff --git a/vmware_guestdump.c b/vmware_guestdump.c index 5be26c8..5c7ee4d 100644 --- a/vmware_guestdump.c +++ b/vmware_guestdump.c @@ -2,6 +2,8 @@ * vmware_guestdump.c * * Copyright (c) 2020 VMware, Inc. + * Copyright (c) 2024 Broadcom. All Rights Reserved. The term "Broadcom" + * refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -13,7 +15,7 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * - * Author: Alexey Makhalov <amakhalov(a)vmware.com&gt; + * Author: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; */ #include "defs.h" @@ -21,20 +23,31 @@ #define LOGPRX "vmw: " -#define GUESTDUMP_VERSION 4 -#define GUESTDUMP_MAGIC1 1 -#define GUESTDUMP_MAGIC2 0 - +/* + * debug.guest file layout + * 00000000: guest dump header, it includes: + * 1. Version (4 bytes) \ + * 2. Number of Virtual CPUs (4 bytes) } - struct guestdumpheader + * 3. Reserved gap + * 4. Main Memory information - struct mainmeminfo{,_old} + * (use get_vcpus_offset() to get total size of guestdumpheader) + * vcpus_offset: ---------\ + * 1. struct vcpu_state1 \ + * 2. reserved gap } num_vcpus times + * 3. struct vcpu_state2 / + * 4. 4KB of reserved data / + * --------/ + * + */ struct guestdumpheader { uint32_t version; uint32_t num_vcpus; - uint8_t magic1; - uint8_t reserved1; - uint32_t cpu_vendor; - uint64_t magic2; +} __attribute__((packed)) hdr; + +struct mainmeminfo { uint64_t last_addr; uint64_t memsize_in_pages; - uint32_t reserved2; + uint32_t reserved1; uint32_t mem_holes; struct memhole { uint64_t ppn; @@ -42,14 +55,36 @@ struct guestdumpheader { } holes[2]; } __attribute__((packed)); -struct vcpu_state { +/* Used by version 1 only */ +struct mainmeminfo_old { + uint64_t last_addr; + uint32_t memsize_in_pages; + uint32_t reserved1; + uint32_t mem_holes; + struct memhole1 { + uint32_t ppn; + uint32_t pages; + } holes[2]; + /* There are additional fields, see get_vcpus_offset() calculation. */ +} __attribute__((packed)); + +/* First half of vcpu_state */ +struct vcpu_state1 { uint32_t cr0; uint64_t cr2; uint64_t cr3; uint64_t cr4; uint64_t reserved1[10]; uint64_t idt_base; - uint16_t reserved2[21]; +} __attribute__((packed)); + +/* + * Unused fields between vcpu_state1 and vcpu_state2 swill be skipped. + * See get_vcpu_gapsize() calculation. + */ + +/* Second half of vcpu_state */ +struct vcpu_state2 { struct x86_64_pt_regs { uint64_t r15; uint64_t r14; @@ -76,9 +111,41 @@ struct vcpu_state { uint8_t reserved3[65]; } __attribute__((packed)); +/* + * Returns the size of the guest dump header. + */ +static inline long +get_vcpus_offset(uint32_t version, int mem_holes) +{ + switch (version) { + case 1: /* ESXi 6.7 and older */ + return sizeof(struct guestdumpheader) + 13 + sizeof(struct mainmeminfo_old) + + (mem_holes == -1 ? 0 : 8 * mem_holes + 4); + case 3: /* ESXi 6.8 */ + return sizeof(struct guestdumpheader) + 14 + sizeof(struct mainmeminfo); + case 4: /* ESXi 7.0 */ + case 5: /* ESXi 8.0 */ + return sizeof(struct guestdumpheader) + 14 + sizeof(struct mainmeminfo); + case 6: /* ESXi 8.0u2 */ + return sizeof(struct guestdumpheader) + 15 + sizeof(struct mainmeminfo); + + } + return 0; +} + +/* + * Returns the size of reserved (unused) fields in the middle of vcpu_state structure. + */ +static inline long +get_vcpu_gapsize(uint32_t version) +{ + if (version < 4) + return 45; + return 42; +} /* - * vmware_guestdump is extension to vmware_vmss with ability to debug + * vmware_guestdump is an extension to the vmware_vmss with ability to debug * debug.guest and debug.vmem files. * * debug.guest.gz and debug.vmem.gz can be obtained using following @@ -86,73 +153,136 @@ struct vcpu_state { * monitor.mini-suspend_on_panic = TRUE * monitor.suspend_on_triplefault = TRUE * - * guestdump (debug.guest) is simplified version of *.vmss which does - * not contain full VM state, but minimal guest state, such as memory + * guestdump (debug.guest) is a simplified version of the *.vmss which does + * not contain a full VM state, but minimal guest state, such as a memory * layout and CPUs state, needed for debugger. is_vmware_guestdump() * and vmware_guestdump_init() functions parse guestdump header and - * populate vmss data structure (from vmware_vmss.c). As result, all + * populate vmss data structure (from vmware_vmss.c). In result, all * handlers (except mempry_dump) from vmware_vmss.c can be reused. * - * debug.guest does not have dedicated header magic or signature for - * its format. To probe debug.guest we need to perform header fields - * and file size validity. In addition, check for the filename - * extension, which must be ".guest". + * debug.guest does not have a dedicated header magic or file format signature + * To probe debug.guest we need to perform series of validations. In addition, + * we check for the filename extension, which must be ".guest". */ - int is_vmware_guestdump(char *filename) { - struct guestdumpheader hdr; + struct mainmeminfo mmi; + long vcpus_offset; FILE *fp; - uint64_t filesize, holes_sum = 0; + uint64_t filesize, expected_filesize, holes_sum = 0; int i; if (strcmp(filename + strlen(filename) - 6, ".guest")) return FALSE; - if ((fp = fopen(filename, "r")) == NULL) { + if ((fp = fopen(filename, "r")) == NULL) { error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n", - filename, errno, strerror(errno)); + filename, errno, strerror(errno)); return FALSE; - } + } if (fread(&hdr, sizeof(struct guestdumpheader), 1, fp) != 1) { error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", - "guestdumpheader", filename, errno, strerror(errno)); + "guestdumpheader", filename, errno, strerror(errno)); + fclose(fp); + return FALSE; + } + + vcpus_offset = get_vcpus_offset(hdr.version, -1 /* Unknown yet, adjust it later */); + + if (!vcpus_offset) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Not supported version %d\n", hdr.version); fclose(fp); return FALSE; } + if (hdr.version == 1) { + struct mainmeminfo_old tmp; + if (fseek(fp, vcpus_offset - sizeof(struct mainmeminfo_old), SEEK_SET) == -1) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", + filename, errno, strerror(errno)); + fclose(fp); + return FALSE; + } + + if (fread(&tmp, sizeof(struct mainmeminfo_old), 1, fp) != 1) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", + "mainmeminfo_old", filename, errno, strerror(errno)); + fclose(fp); + return FALSE; + } + mmi.last_addr = tmp.last_addr; + mmi.memsize_in_pages = tmp.memsize_in_pages; + mmi.mem_holes = tmp.mem_holes; + mmi.holes[0].ppn = tmp.holes[0].ppn; + mmi.holes[0].pages = tmp.holes[0].pages; + mmi.holes[1].ppn = tmp.holes[1].ppn; + mmi.holes[1].pages = tmp.holes[1].pages; + /* vcpu_offset adjustment for mem_holes is required only for version 1. */ + vcpus_offset = get_vcpus_offset(hdr.version, mmi.mem_holes); + } else { + if (fseek(fp, vcpus_offset - sizeof(struct mainmeminfo), SEEK_SET) == -1) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", + filename, errno, strerror(errno)); + fclose(fp); + return FALSE; + } + + if (fread(&mmi, sizeof(struct mainmeminfo), 1, fp) != 1) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", + "mainmeminfo", filename, errno, strerror(errno)); + fclose(fp); + return FALSE; + } + } if (fseek(fp, 0L, SEEK_END) == -1) { - error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", - filename, errno, strerror(errno)); + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", + filename, errno, strerror(errno)); fclose(fp); return FALSE; } filesize = ftell(fp); fclose(fp); - if (hdr.mem_holes > 2) - goto unrecognized; + if (mmi.mem_holes > 2) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Unexpected mmi.mem_holes value %d\n", + mmi.mem_holes); + return FALSE; + } - for (i = 0; i < hdr.mem_holes; i++) { + for (i = 0; i < mmi.mem_holes; i++) { /* hole start page */ - vmss.regions[i].startpagenum = hdr.holes[i].ppn; + vmss.regions[i].startpagenum = mmi.holes[i].ppn; /* hole end page */ - vmss.regions[i].startppn = hdr.holes[i].ppn + hdr.holes[i].pages; - holes_sum += hdr.holes[i].pages; + vmss.regions[i].startppn = mmi.holes[i].ppn + mmi.holes[i].pages; + holes_sum += mmi.holes[i].pages; + } + + if ((mmi.last_addr + 1) != ((mmi.memsize_in_pages + holes_sum) << VMW_PAGE_SHIFT)) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Memory size check failed\n"); + return FALSE; } - if (hdr.version != GUESTDUMP_VERSION || - hdr.magic1 != GUESTDUMP_MAGIC1 || - hdr.magic2 != GUESTDUMP_MAGIC2 || - (hdr.last_addr + 1) != ((hdr.memsize_in_pages + holes_sum) << VMW_PAGE_SHIFT) || - filesize != sizeof(struct guestdumpheader) + - hdr.num_vcpus * (sizeof (struct vcpu_state) + VMW_PAGE_SIZE)) - goto unrecognized; + expected_filesize = vcpus_offset + hdr.num_vcpus * (sizeof(struct vcpu_state1) + + get_vcpu_gapsize(hdr.version) + sizeof(struct vcpu_state2) + VMW_PAGE_SIZE); + if (filesize != expected_filesize) { + if (CRASHDEBUG(1)) + error(INFO, LOGPRX"Incorrect file size: %d != %d\n", + filesize, expected_filesize); + return FALSE; + } - vmss.memsize = hdr.memsize_in_pages << VMW_PAGE_SHIFT; - vmss.regionscount = hdr.mem_holes + 1; + vmss.memsize = mmi.memsize_in_pages << VMW_PAGE_SHIFT; + vmss.regionscount = mmi.mem_holes + 1; vmss.memoffset = 0; vmss.num_vcpus = hdr.num_vcpus; return TRUE; @@ -169,7 +299,8 @@ vmware_guestdump_init(char *filename, FILE *ofp) FILE *fp = NULL; int i, result = TRUE; char *vmem_filename = NULL; - struct vcpu_state vs; + struct vcpu_state1 vs1; + struct vcpu_state2 vs2; char *p; if (!machine_type("X86") && !machine_type("X86_64")) { @@ -180,14 +311,14 @@ vmware_guestdump_init(char *filename, FILE *ofp) goto exit; } - if ((fp = fopen(filename, "r")) == NULL) { + if ((fp = fopen(filename, "r")) == NULL) { error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n", filename, errno, strerror(errno)); result = FALSE; goto exit; - } + } - if (fseek(fp, sizeof(struct guestdumpheader), SEEK_SET) == -1) { + if (fseek(fp, get_vcpus_offset(hdr.version, vmss.regionscount - 1), SEEK_SET) == -1) { error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", filename, errno, strerror(errno)); result = FALSE; @@ -203,7 +334,19 @@ vmware_guestdump_init(char *filename, FILE *ofp) } for (i = 0; i < vmss.num_vcpus; i++) { - if (fread(&vs, sizeof(struct vcpu_state), 1, fp) != 1) { + if (fread(&vs1, sizeof(struct vcpu_state1), 1, fp) != 1) { + error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", + "vcpu_state", filename, errno, strerror(errno)); + result = FALSE; + goto exit; + } + if (fseek(fp, get_vcpu_gapsize(hdr.version), SEEK_CUR) == -1) { + error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", + "vcpu_state", filename, errno, strerror(errno)); + result = FALSE; + goto exit; + } + if (fread(&vs2, sizeof(struct vcpu_state2), 1, fp) != 1) { error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n", "vcpu_state", filename, errno, strerror(errno)); result = FALSE; @@ -217,29 +360,29 @@ vmware_guestdump_init(char *filename, FILE *ofp) } vmss.vcpu_regs[i] = 0; - vmss.regs64[i]->rax = vs.regs64.rax; - vmss.regs64[i]->rcx = vs.regs64.rcx; - vmss.regs64[i]->rdx = vs.regs64.rdx; - vmss.regs64[i]->rbx = vs.regs64.rbx; - vmss.regs64[i]->rbp = vs.regs64.rbp; - vmss.regs64[i]->rsp = vs.regs64.rsp; - vmss.regs64[i]->rsi = vs.regs64.rsi; - vmss.regs64[i]->rdi = vs.regs64.rdi; - vmss.regs64[i]->r8 = vs.regs64.r8; - vmss.regs64[i]->r9 = vs.regs64.r9; - vmss.regs64[i]->r10 = vs.regs64.r10; - vmss.regs64[i]->r11 = vs.regs64.r11; - vmss.regs64[i]->r12 = vs.regs64.r12; - vmss.regs64[i]->r13 = vs.regs64.r13; - vmss.regs64[i]->r14 = vs.regs64.r14; - vmss.regs64[i]->r15 = vs.regs64.r15; - vmss.regs64[i]->idtr = vs.idt_base; - vmss.regs64[i]->cr[0] = vs.cr0; - vmss.regs64[i]->cr[2] = vs.cr2; - vmss.regs64[i]->cr[3] = vs.cr3; - vmss.regs64[i]->cr[4] = vs.cr4; - vmss.regs64[i]->rip = vs.regs64.rip; - vmss.regs64[i]->rflags = vs.regs64.eflags; + vmss.regs64[i]->rax = vs2.regs64.rax; + vmss.regs64[i]->rcx = vs2.regs64.rcx; + vmss.regs64[i]->rdx = vs2.regs64.rdx; + vmss.regs64[i]->rbx = vs2.regs64.rbx; + vmss.regs64[i]->rbp = vs2.regs64.rbp; + vmss.regs64[i]->rsp = vs2.regs64.rsp; + vmss.regs64[i]->rsi = vs2.regs64.rsi; + vmss.regs64[i]->rdi = vs2.regs64.rdi; + vmss.regs64[i]->r8 = vs2.regs64.r8; + vmss.regs64[i]->r9 = vs2.regs64.r9; + vmss.regs64[i]->r10 = vs2.regs64.r10; + vmss.regs64[i]->r11 = vs2.regs64.r11; + vmss.regs64[i]->r12 = vs2.regs64.r12; + vmss.regs64[i]->r13 = vs2.regs64.r13; + vmss.regs64[i]->r14 = vs2.regs64.r14; + vmss.regs64[i]->r15 = vs2.regs64.r15; + vmss.regs64[i]->idtr = vs1.idt_base; + vmss.regs64[i]->cr[0] = vs1.cr0; + vmss.regs64[i]->cr[2] = vs1.cr2; + vmss.regs64[i]->cr[3] = vs1.cr3; + vmss.regs64[i]->cr[4] = vs1.cr4; + vmss.regs64[i]->rip = vs2.regs64.rip; + vmss.regs64[i]->rflags = vs2.regs64.eflags; vmss.vcpu_regs[i] = REGS_PRESENT_ALL; } @@ -268,9 +411,9 @@ vmware_guestdump_init(char *filename, FILE *ofp) fprintf(ofp, LOGPRX"vmem file: %s\n\n", vmem_filename); if (CRASHDEBUG(1)) { - vmware_guestdump_memory_dump(ofp); - dump_registers_for_vmss_dump(); - } + vmware_guestdump_memory_dump(ofp); + dump_registers_for_vmss_dump(); + } exit: if (fp) @@ -296,24 +439,23 @@ exit: int vmware_guestdump_memory_dump(FILE *ofp) { + uint64_t holes_sum = 0; + unsigned i; + fprintf(ofp, "vmware_guestdump:\n"); fprintf(ofp, " Header: version=%d num_vcpus=%llu\n", - GUESTDUMP_VERSION, (ulonglong)vmss.num_vcpus); + hdr.version, (ulonglong)vmss.num_vcpus); fprintf(ofp, "Total memory: %llu\n", (ulonglong)vmss.memsize); - if (vmss.regionscount > 1) { - uint64_t holes_sum = 0; - unsigned i; - fprintf(ofp, "Memory regions[%d]:\n", vmss.regionscount); - fprintf(ofp, " [0x%016x-", 0); - for (i = 0; i < vmss.regionscount - 1; i++) { - fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.regions[i].startpagenum << VMW_PAGE_SHIFT); - fprintf(ofp, " [0x%016llx-", (ulonglong)vmss.regions[i].startppn << VMW_PAGE_SHIFT); - holes_sum += vmss.regions[i].startppn - vmss.regions[i].startpagenum; - } - fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.memsize + (holes_sum << VMW_PAGE_SHIFT)); + fprintf(ofp, "Memory regions[%d]:\n", vmss.regionscount); + fprintf(ofp, " [0x%016x-", 0); + for (i = 0; i < vmss.regionscount - 1; i++) { + fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.regions[i].startpagenum << VMW_PAGE_SHIFT); + fprintf(ofp, " [0x%016llx-", (ulonglong)vmss.regions[i].startppn << VMW_PAGE_SHIFT); + holes_sum += vmss.regions[i].startppn - vmss.regions[i].startpagenum; } + fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.memsize + (holes_sum << VMW_PAGE_SHIFT)); return TRUE; } -- 2.40.1