On 2024/01/05 16:30, Aditya Gupta wrote:
> Currently the output is like this ppc64le:
>
> crash> info threads
> Id Target Id Frame
> 1 CPU 0 <unavailable> in ?? ()
> 2 CPU 1 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> 3 CPU 2 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> 4 CPU 3 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> * 5 CPU 4 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> 6 CPU 5 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> 7 CPU 6 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
> 8 CPU 7 0xc000000000025d00 in ppc_panic_event (this=<optimized out>, event=<optimized out>, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712
>
> There are two issues here:
>
> 1. CPU 0 always shows '<unavailable>' frame:
>
> This is due to 'thread 0' being initialised in 'crash_target_init',
> but it's very early in crash init, so it could not get any registers
> from the vmcore.
>
> And, since GDB caches registers (whether it is there or now), it
> keeps thinking that registers for this thread are unavailable, so all
> future usage of thread 0 also shows unavailable registers & frame
>
> Fix this by refreshing the register cache of all threads in
> 'gdb_refresh_regcache', at task_init
>
> 2. All other CPUs show the same frame:
>
> For each thread, GDB depends on crash to give it registers, ie.
> crash_target::fetch_registers, but this in turn depends on crash's
> current context
>
> GDB internally switches it's thread context internally to each thread
> one by one, while printing the thread's frame, similarly switch
> crash's context when the thread context is changed, so that it
> matches the thread context in gdb (such as, CPU 3 in crash for thread
> 2(CPU 3) in gdb).
>
> Also, since we are switching context multiple times, add an argument
> to 'set_cpu' to not print the context everytime
>
> With these fixed, it will show all threads, as well as correct registers
> and frame. It might still show similar frame for many threads, due to
> all of them being on same NMI exception crash path, but if checked with
> 'info registers', they will have different registers.
>
> Signed-off-by: Aditya Gupta <adityag@xxxxxxxxxxxxx>
> ---
> crash_target.c | 37 +++++++++++++++++++++++++++++++++++--
> defs.h | 5 +++--
> gdb-10.2.patch | 40 +++++++++++++++++++++++++++++++++++++---
> kernel.c | 8 +++++---
> task.c | 30 ++++++++++++++++++++++--------
> tools.c | 10 +++++-----
> 6 files changed, 107 insertions(+), 23 deletions(-)
>
> diff --git a/crash_target.c b/crash_target.c
> index a9d7eea8a80e..d06383f594aa 100644
> --- a/crash_target.c
> +++ b/crash_target.c
> @@ -30,7 +30,8 @@ extern "C" int crash_get_nr_cpus(void);
> extern "C" int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> int regsize, void *val);
> extern "C" int gdb_change_cpu_context (unsigned int cpu);
> -extern "C" int set_cpu (int cpu);
> +extern "C" void gdb_refresh_regcache(unsigned int cpu);
> +extern "C" int set_cpu(int cpu, int print_context);
>
>
> /* The crash target. */
> @@ -150,10 +151,42 @@ gdb_change_cpu_context(unsigned int cpu)
> return FALSE;
>
> /* Making sure that crash's context is same */
> - set_cpu(cpu);
> + set_cpu(cpu, FALSE);
>
> /* Switch to the thread */
> switch_to_thread(tp);
> +
> + /* Fetch/Refresh thread's registers */
> + gdb_refresh_regcache(cpu);
> +
> return TRUE;
> }
>
> +/* Refresh regcache of gdb thread on given CPU
> + *
> + * When gdb threads were initially added by 'crash_target_init', crash was not
> + * yet initialised, and hence crash_target::fetch_registers didn't give any
> + * registers to gdb.
> + *
> + * This is meant to be called after tasks in crash have been initialised, and
> + * possible machdep->get_cpu_reg is also set so architecture can give registers
> + */
> +extern "C" void
> +gdb_refresh_regcache(unsigned int cpu)
> +{
> + int saved_cpu = inferior_thread()->ptid.tid();
> + ptid_t ptid = ptid_t(CRASH_INFERIOR_PID, 0, cpu);
> + inferior *inf = current_inferior();
> + thread_info *tp = find_thread_ptid (inf, ptid);
> +
> + if (tp == NULL) {
> + warning("gdb thread for cpu %d not found\n", cpu);
> + return;
> + }
> +
> + /* temporarily switch to the cpu so we get correct registers */
> + set_cpu(cpu, FALSE);
> + target_fetch_registers(get_thread_regcache(tp), -1);
> +
> + set_cpu(saved_cpu, FALSE);
> +}
> diff --git a/defs.h b/defs.h
> index 7f7a12753658..ba4a6a606f66 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -5940,7 +5940,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);
> @@ -6044,7 +6044,7 @@ void parse_kernel_version(char *);
> #define SHOW_LOG_AUDIT (0x8)
> #define SHOW_LOG_CTIME (0x10)
> #define SHOW_LOG_SAFE (0x20)
> -void set_cpu(int);
> +void set_cpu(int cpu, int print_context);
> void clear_machdep_cache(void);
> struct stack_hook *gather_text_list(struct bt_info *);
> int get_cpus_online(void);
> @@ -7978,5 +7978,6 @@ enum ppc64_regnum {
>
> /* crash_target.c */
> extern int gdb_change_cpu_context (unsigned int cpu);
> +extern void gdb_refresh_regcache (unsigned int cpu);
>
> #endif /* !GDB_COMMON */
> diff --git a/gdb-10.2.patch b/gdb-10.2.patch
> index a4e45fa0e6fa..9051c83ac784 100644
> --- a/gdb-10.2.patch
> +++ b/gdb-10.2.patch
> @@ -494,13 +494,47 @@ exit 0
>
> +#ifdef CRASH_MERGE
> +/* Function to set cpu, defined by crash-utility */
> -+extern "C" void set_cpu (int);
> ++extern "C" void set_cpu(int cpu, int print_context);
> +#endif
> +
> /* RAII type used to increase / decrease the refcount of each thread
> in a given list of threads. */
>
> -@@ -1896,7 +1901,13 @@ thread_command (const char *tidstr, int from_tty)
> +@@ -1093,6 +1098,9 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
> + uiout->table_body ();
> + }
> +
> ++#ifdef CRASH_MERGE
> ++ int current_cpu = current_thread ? current_thread->ptid.tid() : 0;
> ++#endif
> + for (inferior *inf : all_inferiors ())
> + for (thread_info *tp : inf->threads ())
> + {
> +@@ -1108,6 +1116,11 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
> +
> + ui_out_emit_tuple tuple_emitter (uiout, NULL);
> +
> ++ /* Switch to the thread's CPU in crash */
> ++#ifdef CRASH_MERGE
> ++ set_cpu(tp->ptid.tid(), FALSE);
> ++#endif
> ++
> + if (!uiout->is_mi_like_p ())
> + {
> + if (tp == current_thread)
> +@@ -1180,6 +1193,11 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
> + /* This end scope restores the current thread and the frame
> + selected before the "info threads" command, and it finishes the
> + ui-out list or table. */
> ++
> ++#ifdef CRASH_MERGE
> ++ /* Restore crash's context */
> ++ set_cpu(current_cpu, FALSE);
> ++#endif
> + }
> +
> + if (pid == -1 && requested_threads == NULL)
> +@@ -1896,7 +1914,13 @@ thread_command (const char *tidstr, int from_tty)
> {
> ptid_t previous_ptid = inferior_ptid;
>
> @@ -508,7 +542,7 @@ exit 0
> + struct thread_info* thread_id = parse_thread_id (tidstr, NULL);
> +
> +#ifdef CRASH_MERGE
> -+ set_cpu(thread_id->ptid.tid());
> ++ set_cpu(thread_id->ptid.tid(), FALSE);
> +#endif
> +
> + thread_select (tidstr, thread_id);
Ditto.
Thanks,
Kazu
> diff --git a/kernel.c b/kernel.c
> index a8d60507dd95..505cffde1cf8 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -6491,9 +6491,10 @@ no_cpu_flags:
>
> /*
> * Set the context to the active task on a given cpu -- dumpfiles only.
> + * Optionally show the context if print_context is TRUE
> */
> void
> -set_cpu(int cpu)
> +set_cpu(int cpu, int print_context)
> {
> ulong task;
>
> @@ -6511,11 +6512,12 @@ set_cpu(int cpu)
> return;
>
> if (task)
> - set_context(task, NO_PID);
> + set_context(task, NO_PID, TRUE);
> else
> error(FATAL, "cannot determine active task on cpu %ld\n", cpu);
>
> - show_context(CURRENT_CONTEXT());
> + if (print_context)
> + show_context(CURRENT_CONTEXT());
> }
>
>
> diff --git a/task.c b/task.c
> index 3a190cafbacb..a405b05a47d1 100644
> --- a/task.c
> +++ b/task.c
> @@ -672,7 +672,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 {
> @@ -684,7 +684,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();
> }
>
> @@ -706,6 +706,17 @@ task_init(void)
> kt->boot_date.tv_nsec = 0;
> }
>
> + /*
> + * Refresh CPU 0's thread's regcache
> + *
> + * This is required since, it's registers were initialised in
> + * crash_target_init when crash was not initialised yet and hence could
> + * not pass registers to gdb when gdb requests via
> + * crash_target::fetch_registers, so CPU 0's registers are shown as
> + * <unavailable> in gdb mode
> + * */
> + gdb_refresh_regcache(0);
> +
> tt->flags |= TASK_INIT_DONE;
> }
>
> @@ -2985,9 +2996,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) &&
> @@ -3053,7 +3064,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, FALSE);
>
> sort_context_by_task();
> }
> @@ -3100,7 +3111,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, FALSE);
>
> sort_context_by_task();
> }
> @@ -5281,7 +5292,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;
> @@ -5303,7 +5314,10 @@ set_context(ulong task, ulong pid)
> CURRENT_CONTEXT() = tc;
>
> /* change the selected thread in gdb, according to current context */
> - return gdb_change_cpu_context(tc->processor);
> + if (update_gdb_thread)
> + return gdb_change_cpu_context(tc->processor);
> + 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 0f2db108838a..9b149fb27fcb 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -1860,7 +1860,7 @@ cmd_set(void)
> break;
> }
> cpu = dtoi(optarg, FAULT_ON_ERROR, NULL);
> - set_cpu(cpu);
> + set_cpu(cpu, TRUE);
> return;
>
> case 'p':
> @@ -1871,7 +1871,7 @@ cmd_set(void)
> return;
>
> if (ACTIVE()) {
> - set_context(tt->this_task, NO_PID);
> + set_context(tt->this_task, NO_PID, FALSE);
> 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, FALSE);
> 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, FALSE))
> show_context(CURRENT_CONTEXT());
> break;
>
> case STR_TASK:
> task = value;
> pid = NO_PID;
> - if (set_context(task, pid))
> + if (set_context(task, pid, FALSE))
> show_context(CURRENT_CONTEXT());
> break;
>
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
