Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arm: debug: Add GDB stub for aarch32 #58067

Merged
merged 3 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions arch/arm/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ zephyr_library_sources_ifdef(CONFIG_THREAD_LOCAL_STORAGE tls.c)
zephyr_library_sources_ifdef(CONFIG_USERSPACE userspace.S)
zephyr_library_sources_ifdef(CONFIG_ARM_ZIMAGE_HEADER header.S)
zephyr_library_sources_ifdef(CONFIG_LLEXT elf.c)
zephyr_library_sources_ifdef(CONFIG_GDBSTUB gdbstub.c)

add_subdirectory_ifdef(CONFIG_CPU_CORTEX_M cortex_m)
add_subdirectory_ifdef(CONFIG_CPU_CORTEX_M_HAS_CMSE cortex_m/cmse)
Expand Down
9 changes: 9 additions & 0 deletions arch/arm/core/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,18 @@ config CPU_AARCH32_CORTEX_A
select ARCH_HAS_EXTRA_EXCEPTION_INFO if !USE_SWITCH
select ARCH_HAS_NOCACHE_MEMORY_SUPPORT
select USE_SWITCH_SUPPORTED
# GDBSTUB has not yet been tested on Cortex M or R SoCs
select ARCH_HAS_GDBSTUB
# GDB on ARM needs the etxra registers
select EXTRA_EXCEPTION_INFO if GDBSTUB
help
This option signifies the use of a CPU of the Cortex-A family.

config GDBSTUB_BUF_SZ
# GDB for ARM expects up to 18 4-byte plus 8 12-byte
# registers - 336 HEX letters
default 350 if GDBSTUB

config ISA_THUMB2
bool
help
Expand Down
28 changes: 28 additions & 0 deletions arch/arm/core/cortex_a_r/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
#include <kernel_internal.h>
#include <zephyr/arch/common/exc_handle.h>
#include <zephyr/logging/log.h>
#if defined(CONFIG_GDBSTUB)
#include <zephyr/arch/arm/gdbstub.h>
#include <zephyr/debug/gdbstub.h>
#endif

LOG_MODULE_DECLARE(os, CONFIG_KERNEL_LOG_LEVEL);

#define FAULT_DUMP_VERBOSE (CONFIG_FAULT_DUMP == 2)
Expand Down Expand Up @@ -213,6 +218,12 @@ bool z_arm_fault_undef_instruction(z_arch_esf_t *esf)
z_arm_fpu_caller_save(&esf->fpu);
#endif

#if defined(CONFIG_GDBSTUB)
z_gdb_entry(esf, GDB_EXCEPTION_INVALID_INSTRUCTION);
/* Might not be fatal if GDB stub placed it in the code. */
return false;
#endif

/* Print fault information */
LOG_ERR("***** UNDEFINED INSTRUCTION ABORT *****");

Expand Down Expand Up @@ -247,6 +258,17 @@ bool z_arm_fault_prefetch(z_arch_esf_t *esf)
/* Read Instruction Fault Address Register (IFAR) */
uint32_t ifar = __get_IFAR();

#if defined(CONFIG_GDBSTUB)
/* The BKPT instruction could have caused a software breakpoint */
if (fs == IFSR_DEBUG_EVENT) {
/* Debug event, call the gdbstub handler */
z_gdb_entry(esf, GDB_EXCEPTION_BREAKPOINT);
} else {
/* Fatal */
z_gdb_entry(esf, GDB_EXCEPTION_MEMORY_FAULT);
}
return false;
#endif
/* Print fault information*/
LOG_ERR("***** PREFETCH ABORT *****");
if (FAULT_DUMP_VERBOSE) {
Expand Down Expand Up @@ -314,6 +336,12 @@ bool z_arm_fault_data(z_arch_esf_t *esf)
/* Read Data Fault Address Register (DFAR) */
uint32_t dfar = __get_DFAR();

#if defined(CONFIG_GDBSTUB)
z_gdb_entry(esf, GDB_EXCEPTION_MEMORY_FAULT);
/* return false - non-fatal error */
return false;
#endif

#if defined(CONFIG_USERSPACE)
if ((fs == COND_CODE_1(CONFIG_AARCH32_ARMV8_R,
(FSR_FS_TRANSLATION_FAULT),
Expand Down
225 changes: 225 additions & 0 deletions arch/arm/core/gdbstub.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
/*
* Copyright (c) 2023 Marek Vedral <vedrama5@fel.cvut.cz>
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/kernel.h>
#include <kernel_internal.h>
#include <zephyr/arch/arm/gdbstub.h>
#include <zephyr/debug/gdbstub.h>

/* Position of each register in the packet - n-th register in the ctx.registers array needs to be
* the packet_pos[n]-th byte of the g (read all registers) packet. See struct arm_register_names in
* GDB file gdb/arm-tdep.c, which defines these positions.
*/
static const int packet_pos[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 41};

/* Required struct */
static struct gdb_ctx ctx;

/* Return true if BKPT instruction caused the current entry */
static int is_bkpt(unsigned int exc_cause)
{
int ret = 0;

if (exc_cause == GDB_EXCEPTION_BREAKPOINT) {
/* Get the instruction */
unsigned int instr = sys_read32(ctx.registers[PC]);
/* Try to check the instruction encoding */
int ist = ((ctx.registers[SPSR] & BIT(SPSR_J)) >> (SPSR_J - 1)) |
((ctx.registers[SPSR] & BIT(SPSR_T)) >> SPSR_T);

if (ist == SPSR_ISETSTATE_ARM) {
/* ARM instruction set state */
ret = ((instr & 0xFF00000) == 0x1200000) && ((instr & 0xF0) == 0x70);
} else if (ist != SPSR_ISETSTATE_JAZELLE) {
/* Thumb or ThumbEE encoding */
ret = ((instr & 0xFF00) == 0xBE00);
}
}
return ret;
}

/* Wrapper function to save and restore execution c */
void z_gdb_entry(z_arch_esf_t *esf, unsigned int exc_cause)
{
/* Disable the hardware breakpoint in case it was set */
__asm__ volatile("mcr p14, 0, %0, c0, c0, 5" ::"r"(0x0) :);

ctx.exception = exc_cause;
/* save the registers */
ctx.registers[R0] = esf->basic.r0;
ctx.registers[R1] = esf->basic.r1;
ctx.registers[R2] = esf->basic.r2;
ctx.registers[R3] = esf->basic.r3;
/* The EXTRA_EXCEPTION_INFO kernel option ensures these regs are set */
ctx.registers[R4] = esf->extra_info.callee->v1;
ctx.registers[R5] = esf->extra_info.callee->v2;
ctx.registers[R6] = esf->extra_info.callee->v3;
ctx.registers[R7] = esf->extra_info.callee->v4;
ctx.registers[R8] = esf->extra_info.callee->v5;
ctx.registers[R9] = esf->extra_info.callee->v6;
ctx.registers[R10] = esf->extra_info.callee->v7;
ctx.registers[R11] = esf->extra_info.callee->v8;
ctx.registers[R13] = esf->extra_info.callee->psp;

ctx.registers[R12] = esf->basic.r12;
ctx.registers[LR] = esf->basic.lr;
ctx.registers[PC] = esf->basic.pc;
ctx.registers[SPSR] = esf->basic.xpsr;

/* True if entering after a BKPT instruction */
const int bkpt_entry = is_bkpt(exc_cause);

z_gdb_main_loop(&ctx);

/* The registers part of EXTRA_EXCEPTION_INFO are read-only - the excpetion return code
* does not restore them, thus we don't need to do so here
*/
esf->basic.r0 = ctx.registers[R0];
esf->basic.r1 = ctx.registers[R1];
esf->basic.r2 = ctx.registers[R2];
esf->basic.r3 = ctx.registers[R3];
esf->basic.r12 = ctx.registers[R12];
esf->basic.lr = ctx.registers[LR];
esf->basic.pc = ctx.registers[PC];
esf->basic.xpsr = ctx.registers[SPSR];
/* TODO: restore regs from extra exc. info */

if (bkpt_entry) {
/* Apply this offset, so that the process won't be affected by the
* BKPT instruction
*/
esf->basic.pc += 0x4;
}
esf->basic.xpsr = ctx.registers[SPSR];
}

void arch_gdb_init(void)
{
uint32_t reg_val;
/* Enable the monitor debug mode */
__asm__ volatile("mrc p14, 0, %0, c0, c2, 2" : "=r"(reg_val)::);
reg_val |= DBGDSCR_MONITOR_MODE_EN;
__asm__ volatile("mcr p14, 0, %0, c0, c2, 2" ::"r"(reg_val) :);

/* Generate the Prefetch abort exception */
__asm__ volatile("BKPT");
}

void arch_gdb_continue(void)
{
/* No need to do anything, return to the code. */
}

void arch_gdb_step(void)
{
/* Set the hardware breakpoint */
uint32_t reg_val = ctx.registers[PC];
/* set BVR (Breakpoint value register) to PC, make sure it is word aligned */
reg_val &= ~(0x3);
__asm__ volatile("mcr p14, 0, %0, c0, c0, 4" ::"r"(reg_val) :);

reg_val = 0;
/* Address mismatch */
reg_val |= (DBGDBCR_MEANING_ADDR_MISMATCH & DBGDBCR_MEANING_MASK) << DBGDBCR_MEANING_SHIFT;
/* Match any other instruction */
reg_val |= (0xF & DBGDBCR_BYTE_ADDR_MASK) << DBGDBCR_BYTE_ADDR_SHIFT;
/* Breakpoint enable */
reg_val |= DBGDBCR_BRK_EN_MASK;
__asm__ volatile("mcr p14, 0, %0, c0, c0, 5" ::"r"(reg_val) :);
}

size_t arch_gdb_reg_readall(struct gdb_ctx *c, uint8_t *buf, size_t buflen)
{
int ret = 0;
/* All other registers are not supported */
memset(buf, 'x', buflen);
for (int i = 0; i < GDB_NUM_REGS; i++) {
/* offset inside the packet */
int pos = packet_pos[i] * 8;
int r = bin2hex((const uint8_t *)(c->registers + i), 4, buf + pos, buflen - pos);
/* remove the newline character placed by the bin2hex function */
buf[pos + 8] = 'x';
if (r == 0) {
ret = 0;
break;
}
ret += r;
}

if (ret) {
/* Since we don't support some floating point registers, set the packet size
* manually
*/
ret = GDB_READALL_PACKET_SIZE;
}
return ret;
}

size_t arch_gdb_reg_writeall(struct gdb_ctx *c, uint8_t *hex, size_t hexlen)
{
int ret = 0;

for (unsigned int i = 0; i < hexlen; i += 8) {
if (hex[i] != 'x') {
/* check if the stub supports this register */
for (unsigned int j = 0; j < GDB_NUM_REGS; j++) {
if (packet_pos[j] != i) {
continue;
}
int r = hex2bin(hex + i * 8, 8, (uint8_t *)(c->registers + j), 4);

if (r == 0) {
return 0;
}
ret += r;
}
}
}
return ret;
}

size_t arch_gdb_reg_readone(struct gdb_ctx *c, uint8_t *buf, size_t buflen, uint32_t regno)
{
/* Reading four bytes (could be any return value except 0, which would indicate an error) */
int ret = 4;
/* Fill the buffer with 'x' in case the stub does not support the required register */
memset(buf, 'x', 8);
if (regno == SPSR_REG_IDX) {
/* The SPSR register is at the end, we have to check separately */
ret = bin2hex((uint8_t *)(c->registers + GDB_NUM_REGS - 1), 4, buf, buflen);
} else {
/* Check which of our registers corresponds to regnum */
for (int i = 0; i < GDB_NUM_REGS; i++) {
if (packet_pos[i] == regno) {
ret = bin2hex((uint8_t *)(c->registers + i), 4, buf, buflen);
break;
}
}
}
return ret;
}

size_t arch_gdb_reg_writeone(struct gdb_ctx *c, uint8_t *hex, size_t hexlen, uint32_t regno)
{
int ret = 0;
/* Set the value of a register */
if (hexlen != 8) {
return ret;
}

if (regno < (GDB_NUM_REGS - 1)) {
/* Again, check the corresponding register index */
for (int i = 0; i < GDB_NUM_REGS; i++) {
if (packet_pos[i] == regno) {
ret = hex2bin(hex, hexlen, (uint8_t *)(c->registers + i), 4);
break;
}
}
} else if (regno == SPSR_REG_IDX) {
ret = hex2bin(hex, hexlen, (uint8_t *)(c->registers + GDB_NUM_REGS - 1), 4);
}
return ret;
}
7 changes: 7 additions & 0 deletions arch/arm/core/mmu/arm_mmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,14 @@ static const struct arm_mmu_flat_range mmu_zephyr_ranges[] = {
.start = (uint32_t)__text_region_start,
.end = (uint32_t)__text_region_end,
.attrs = MT_NORMAL | MATTR_SHARED |
/* The code needs to have write permission in order for
* software breakpoints (which modify instructions) to work
*/
#if defined(CONFIG_GDBSTUB)
MPERM_R | MPERM_X | MPERM_W |
#else
MPERM_R | MPERM_X |
#endif
MATTR_CACHE_OUTER_WB_nWA | MATTR_CACHE_INNER_WB_nWA |
MATTR_MAY_MAP_L1_SECTION},

Expand Down
10 changes: 9 additions & 1 deletion drivers/timer/arm_arch_timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ static uint32_t cyc_per_tick;
/ CONFIG_SYS_CLOCK_TICKS_PER_SEC)
#endif

#if defined(CONFIG_GDBSTUB)
/* When interactively debugging, the cycle diff can overflow 32-bit variable */
#define TO_CYCLE_DIFF(x) (x)
#else
/* Convert to 32-bit for fast division */
#define TO_CYCLE_DIFF(x) ((cycle_diff_t)(x))
#endif

/* the unsigned long cast limits divisors to native CPU register width */
#define cycle_diff_t unsigned long

Expand Down Expand Up @@ -58,7 +66,7 @@ static void arm_arch_timer_compare_isr(const void *arg)

uint64_t curr_cycle = arm_arch_timer_count();
uint64_t delta_cycles = curr_cycle - last_cycle;
uint32_t delta_ticks = (cycle_diff_t)delta_cycles / CYC_PER_TICK;
uint32_t delta_ticks = TO_CYCLE_DIFF(delta_cycles) / CYC_PER_TICK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about removing the TO_CYCLE_DIFF, just using uint32_t delta_ticks = (cycle_diff_t) (delta_cycles / CYC_PER_TICK);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the description of the macro. The result of division delta_cycles / CYC_PER_TICK could be very large when debugging and casting the result to cycle_diff_t (unsigned long, 32-bit wide) could cause an overflow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, any differences between uint32_t delta_ticks = delta_cycles / CYC_PER_TICK and uint32_t delta_ticks = (uint32_t) (delta_cycles / CYC_PER_TICK) as delta_ticks is a uint32_t

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, you are right. I didn't notice that the parenthesis encloses the whole division in your previous comment.

I would still keep the macro, because we want to cast the delta_cycles variable to uint32_t if possible to improve performance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, fair enough


last_cycle += (cycle_diff_t)delta_ticks * CYC_PER_TICK;
last_tick += delta_ticks;
Expand Down
3 changes: 3 additions & 0 deletions include/zephyr/arch/arm/arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#include <zephyr/arch/arm/nmi.h>
#include <zephyr/arch/arm/asm_inline.h>
#include <zephyr/arch/common/sys_bitops.h>
#if defined(CONFIG_GDBSTUB)
#include <zephyr/arch/arm/gdbstub.h>
#endif

#ifdef CONFIG_CPU_CORTEX_M
#include <zephyr/arch/arm/cortex_m/cpu.h>
Expand Down