[RFC PATCH v2 2/4] ppc64: fix gdb passthrough by implementing machdep->get_cpu_reg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Currently, gdb passthroughs of 'bt', 'frame', 'up', 'down', 'info
locals' don't work. This is due to gdb not knowing the register values to
unwind the stack frames

Every gdb passthrough goes through `gdb_interface`. And then, gdb expects
`crash_target::fetch_registers` to give it the register values, which is
dependent on `machdep->get_cpu_reg` to read the register values for
specific architecture.

                                      ┌────────────────────────┐
           gdb passthrough (eg. "bt") │                        │
   crash   ─────────────────────────▶│                        │
                                      │      gdb_interface     │
                                      │                        │
                                      │                        │
                                      │ ┌────────────────────┐ │
                 fetch_registers      │ │                    │ │
crash_target◀────────────────────────┼─┤        gdb         │ │
            ─────────────────────────┼▶│                    │ │
              Registers (SP,NIP, etc.)│ │                    │ │
                                      │ │                    │ │
                                      │ └────────────────────┘ │
                                      └────────────────────────┘

Implement `machdep->get_cpu_reg` on PPC64, so that crash provides the
register values to gdb to unwind stack frames properly

Also, with the goal to preserve the behaviour of gdb passthroughs in
default mode, a hook 'machdep->gdb_mode_hook' has been introduced, which
will get executed everytime when "set gdb on", and "set gdb off" are run,
and can be used by any architecture.
This hook has been used on PPC64 such that, `machdep->get_cpu_reg` is only
set on "set gdb on", and set to NULL in default mode (or on "set gdb off"),
so that behaviour of gdb passthroughs in default mode stays the same.

With these changes, on powerpc, 'bt' command output in gdb mode, will look
like this:

    gdb> bt
    #0  0xc0000000002a53e8 in crash_setup_regs (oldregs=<optimized out>, newregs=0xc00000000486f8d8) at ./arch/powerpc/include/asm/kexec.h:69
    #1  __crash_kexec (regs=<optimized out>) at kernel/kexec_core.c:974
    #2  0xc000000000168918 in panic (fmt=<optimized out>) at kernel/panic.c:358
    #3  0xc000000000b735f8 in sysrq_handle_crash (key=<optimized out>) at drivers/tty/sysrq.c:155
    #4  0xc000000000b742cc in __handle_sysrq (key=key@entry=99, check_mask=check_mask@entry=false) at drivers/tty/sysrq.c:602
    #5  0xc000000000b7506c in write_sysrq_trigger (file=<optimized out>, buf=<optimized out>, count=2, ppos=<optimized out>) at drivers/tty/sysrq.c:1163
    #6  0xc00000000069a7bc in pde_write (ppos=<optimized out>, count=<optimized out>, buf=<optimized out>, file=<optimized out>, pde=0xc000000009ed3a80) at fs/proc/inode.c:340
    #7  proc_reg_write (file=<optimized out>, buf=<optimized out>, count=<optimized out>, ppos=<optimized out>) at fs/proc/inode.c:352
    #8  0xc0000000005b3bbc in vfs_write (file=file@entry=0xc00000009dda7d00, buf=buf@entry=0xebcfc7c6040 <error: Cannot access memory at address 0xebcfc7c6040>, count=count@entry=2, pos=pos@entry=0xc00000000486fda0) at fs/read_write.c:582

instead of earlier output without this patch:

    gdb> bt
    #0  <unavailable> in ?? ()
    Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Signed-off-by: Aditya Gupta <adityag@xxxxxxxxxxxxx>
---
 defs.h  | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ppc64.c | 100 +++++++++++++++++++++++++++++++++++++++++++++
 tools.c |  12 ++++--
 3 files changed, 232 insertions(+), 3 deletions(-)

diff --git a/defs.h b/defs.h
index b6f213120975..798a49e7ec41 100644
--- a/defs.h
+++ b/defs.h
@@ -1074,6 +1074,7 @@ struct machdep_table {
         void (*show_interrupts)(int, ulong *);
 	int (*is_page_ptr)(ulong, physaddr_t *);
 	int (*get_cpu_reg)(int, int, const char *, int, void *);
+	void (*gdb_mode_hook)(int gdb_mode_enabled);
 };
 
 /*
@@ -6660,6 +6661,8 @@ struct machine_specific {
 
 void ppc64_init(int);
 void ppc64_dump_machdep_table(ulong);
+int ppc64_get_cpu_reg(int cpu, int regno, const char *name, int size,
+		  void *value);
 #define display_idt_table() \
         error(FATAL, "-d option is not applicable to PowerPC architecture\n")
 #define KSYMS_START     (0x1)
@@ -7839,4 +7842,124 @@ enum x86_64_regnum {
         LAST_REGNUM
 };
 
+/*
+ * Register numbers to make crash_target->fetch_registers()
+ * ---> machdep->get_cpu_reg() work properly.
+ *
+ *  These register numbers and names are given according to output of
+ *  `rs6000_register_name`, because that is what was being used by
+ *  crash_target::fetch_registers in case of PPC64
+ */
+enum ppc64_renum {
+	PPC64_R0_REGNUM = 0,
+	PPC64_R1_REGNUM,
+	PPC64_R2_REGNUM,
+	PPC64_R3_REGNUM,
+	PPC64_R4_REGNUM,
+	PPC64_R5_REGNUM,
+	PPC64_R6_REGNUM,
+	PPC64_R7_REGNUM,
+	PPC64_R8_REGNUM,
+	PPC64_R9_REGNUM,
+	PPC64_R10_REGNUM,
+	PPC64_R11_REGNUM,
+	PPC64_R12_REGNUM,
+	PPC64_R13_REGNUM,
+	PPC64_R14_REGNUM,
+	PPC64_R15_REGNUM,
+	PPC64_R16_REGNUM,
+	PPC64_R17_REGNUM,
+	PPC64_R18_REGNUM,
+	PPC64_R19_REGNUM,
+	PPC64_R20_REGNUM,
+	PPC64_R21_REGNUM,
+	PPC64_R22_REGNUM,
+	PPC64_R23_REGNUM,
+	PPC64_R24_REGNUM,
+	PPC64_R25_REGNUM,
+	PPC64_R26_REGNUM,
+	PPC64_R27_REGNUM,
+	PPC64_R28_REGNUM,
+	PPC64_R29_REGNUM,
+	PPC64_R30_REGNUM,
+	PPC64_R31_REGNUM,
+
+	PPC64_F0_REGNUM = 32,
+	PPC64_F1_REGNUM,
+	PPC64_F2_REGNUM,
+	PPC64_F3_REGNUM,
+	PPC64_F4_REGNUM,
+	PPC64_F5_REGNUM,
+	PPC64_F6_REGNUM,
+	PPC64_F7_REGNUM,
+	PPC64_F8_REGNUM,
+	PPC64_F9_REGNUM,
+	PPC64_F10_REGNUM,
+	PPC64_F11_REGNUM,
+	PPC64_F12_REGNUM,
+	PPC64_F13_REGNUM,
+	PPC64_F14_REGNUM,
+	PPC64_F15_REGNUM,
+	PPC64_F16_REGNUM,
+	PPC64_F17_REGNUM,
+	PPC64_F18_REGNUM,
+	PPC64_F19_REGNUM,
+	PPC64_F20_REGNUM,
+	PPC64_F21_REGNUM,
+	PPC64_F22_REGNUM,
+	PPC64_F23_REGNUM,
+	PPC64_F24_REGNUM,
+	PPC64_F25_REGNUM,
+	PPC64_F26_REGNUM,
+	PPC64_F27_REGNUM,
+	PPC64_F28_REGNUM,
+	PPC64_F29_REGNUM,
+	PPC64_F30_REGNUM,
+	PPC64_F31_REGNUM,
+
+	PPC64_PC_REGNUM = 64,
+	PPC64_MSR_REGNUM = 65,
+	PPC64_CR_REGNUM = 66,
+	PPC64_LR_REGNUM = 67,
+	PPC64_CTR_REGNUM = 68,
+	PPC64_XER_REGNUM = 69,
+	PPC64_FPSCR_REGNUM = 70,
+
+	PPC64_VR0_REGNUM = 106,
+	PPC64_VR1_REGNUM,
+	PPC64_VR2_REGNUM,
+	PPC64_VR3_REGNUM,
+	PPC64_VR4_REGNUM,
+	PPC64_VR5_REGNUM,
+	PPC64_VR6_REGNUM,
+	PPC64_VR7_REGNUM,
+	PPC64_VR8_REGNUM,
+	PPC64_VR9_REGNUM,
+	PPC64_VR10_REGNUM,
+	PPC64_VR11_REGNUM,
+	PPC64_VR12_REGNUM,
+	PPC64_VR13_REGNUM,
+	PPC64_VR14_REGNUM,
+	PPC64_VR15_REGNUM,
+	PPC64_VR16_REGNUM,
+	PPC64_VR17_REGNUM,
+	PPC64_VR18_REGNUM,
+	PPC64_VR19_REGNUM,
+	PPC64_VR20_REGNUM,
+	PPC64_VR21_REGNUM,
+	PPC64_VR22_REGNUM,
+	PPC64_VR23_REGNUM,
+	PPC64_VR24_REGNUM,
+	PPC64_VR25_REGNUM,
+	PPC64_VR26_REGNUM,
+	PPC64_VR27_REGNUM,
+	PPC64_VR28_REGNUM,
+	PPC64_VR29_REGNUM,
+	PPC64_VR30_REGNUM,
+	PPC64_VR31_REGNUM,
+
+	PPC64_VSCR_REGNUM = 138,
+	PPC64_VRSAVE_REGNU = 139
+};
+
 #endif /* !GDB_COMMON */
diff --git a/ppc64.c b/ppc64.c
index 7622f68289e7..61ff47c2c8d9 100644
--- a/ppc64.c
+++ b/ppc64.c
@@ -55,6 +55,8 @@ static void ppc64_set_bt_emergency_stack(enum emergency_stack_type type,
 static char * ppc64_check_eframe(struct ppc64_pt_regs *);
 static void ppc64_print_eframe(char *, struct ppc64_pt_regs *, 
 		struct bt_info *);
+int ppc64_get_cpu_reg(int cpu, int regno, const char *name, int size,
+		  void *value);
 static void parse_cmdline_args(void);
 static int ppc64_paca_percpu_offset_init(int);
 static void ppc64_init_cpu_info(void);
@@ -63,6 +65,7 @@ static void ppc64_clear_machdep_cache(void);
 static void ppc64_init_paca_info(void);
 static void ppc64_vmemmap_init(void);
 static int ppc64_get_kvaddr_ranges(struct vaddr_range *);
+static void ppc64_gdb_mode_hook(int gdb_mode_enabled);
 static uint get_ptetype(ulong pte);
 static int is_hugepage(ulong pte);
 static int is_hugepd(ulong pte);
@@ -140,6 +143,15 @@ static inline int is_hugepd(ulong pte)
 	}
 }
 
+static void ppc64_gdb_mode_hook(int gdb_mode_enabled)
+{
+	if (gdb_mode_enabled) {
+		machdep->get_cpu_reg = ppc64_get_cpu_reg;
+	} else {
+		machdep->get_cpu_reg = NULL;
+	}
+}
+
 static inline uint get_ptetype(ulong pte)
 {
 	uint pte_type = 0; /* 0: regular entry; 1: huge pte; 2: huge pd */
@@ -694,6 +706,8 @@ ppc64_init(int when)
 				error(FATAL, "cannot malloc hwirqstack buffer space.");
 		}
 
+		machdep->gdb_mode_hook = ppc64_gdb_mode_hook;
+
 		ppc64_init_paca_info();
 
 		if (!machdep->hz) {
@@ -2472,6 +2486,92 @@ ppc64_print_eframe(char *efrm_str, struct ppc64_pt_regs *regs,
 	ppc64_print_nip_lr(regs, 1);
 }
 
+int
+ppc64_get_cpu_reg(int cpu, int regno, const char *name, int size,
+		  void *value)
+{
+	struct bt_info bt_info, bt_setup;
+	struct task_context *tc;
+	struct ppc64_pt_regs *pt_regs;
+	ulong ip, sp;
+
+	/* Currently only handling registers available in pt_regs:
+	 *
+	 * 0-31:   r0-r31
+	 * 64:     pc/nip
+	 * 65:     msr
+	 *
+	 * 67:     lr
+	 * 68:     ctr
+	 */
+	switch (regno) {
+	case PPC64_R0_REGNUM ... PPC64_R31_REGNUM:
+
+	case PPC64_PC_REGNUM:
+	case PPC64_MSR_REGNUM:
+	case PPC64_LR_REGNUM:
+	case PPC64_CTR_REGNUM:
+		break;
+
+	default:
+		// return false if we can't get that register
+		return FALSE;
+	}
+
+	/* FIXME: Always setting the context to CURRENT_CONTEXT irrespective of whicher
+	 * thread we switched to, in gdb
+	 */
+	tc = CURRENT_CONTEXT();
+	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);
+	pt_regs = (struct ppc64_pt_regs *)bt_info.machdep;
+
+	switch (regno) {
+	case PPC64_R0_REGNUM ... PPC64_R31_REGNUM:
+		if (size != sizeof(pt_regs->gpr[regno]))
+			return FALSE;  // size mismatch
+
+		memcpy(value, &pt_regs->gpr[regno], size);
+		return TRUE;
+
+	case PPC64_PC_REGNUM:
+		if (size != sizeof(pt_regs->nip))
+			return FALSE;  // size mismatch
+
+		memcpy(value, &pt_regs->nip, size);
+		return TRUE;
+
+	case PPC64_MSR_REGNUM:
+		if (size != sizeof(pt_regs->msr))
+			return FALSE;  // size mismatch
+
+		memcpy(value, &pt_regs->msr, size);
+		return TRUE;
+
+	case PPC64_LR_REGNUM:
+		if (size != sizeof(pt_regs->link))
+			return FALSE;  // size mismatch
+
+		memcpy(value, &pt_regs->link, size);
+		return TRUE;
+
+	case PPC64_CTR_REGNUM:
+		if (size != sizeof(pt_regs->ctr))
+			return FALSE;  // size mismatch
+
+		memcpy(value, &pt_regs->ctr, size);
+		return TRUE;
+	}
+
+	printf("error: %s: statement after switch case should be unreachable"
+			, __func__);
+	return FALSE;
+}
+
 /*
  * For vmcore typically saved with KDump or FADump, get SP and IP values
  * from the saved ptregs.
diff --git a/tools.c b/tools.c
index 0f2db108838a..b8081f350ae9 100644
--- a/tools.c
+++ b/tools.c
@@ -2075,12 +2075,18 @@ cmd_set(void)
 				if (!runtime)
 					defer();
                                 else if (STREQ(args[optind], "on")) {
-					if (pc->flags & MINIMAL_MODE)
+					if (pc->flags & MINIMAL_MODE) {
 						goto invalid_set_command;
-					else
+					} else {
                                         	pc->flags2 |= GDB_CMD_MODE;
-                                } else if (STREQ(args[optind], "off"))
+						if (machdep->gdb_mode_hook)
+							machdep->gdb_mode_hook(1);
+					}
+				} else if (STREQ(args[optind], "off")) {
                                         pc->flags2 &= ~GDB_CMD_MODE;
+					if (machdep->gdb_mode_hook)
+						machdep->gdb_mode_hook(0);
+				}
                                 else if (IS_A_NUMBER(args[optind])) {
                                         value = stol(args[optind],
                                                 FAULT_ON_ERROR, NULL);
-- 
2.41.0

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux