Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ OBJS := \
main.o \
aclint.o \
coro.o \
gdbctrl.o \
$(OBJS_EXTRA)

deps := $(OBJS:%.o=.%.o.d)
Expand Down
72 changes: 70 additions & 2 deletions coro.c
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
/* Lightweight coroutine for multi-hart execution */

#include "coro.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "coro.h"

/* Platform detection */

#if !defined(CORO_USE_UCONTEXT) && !defined(CORO_USE_ASM)
Expand All @@ -22,7 +23,8 @@
/* Coroutine state */

typedef enum {
CORO_STATE_SUSPENDED,
CORO_STATE_SUSPENDED, /* WFI suspension */
CORO_STATE_DEBUG_SUSPENDED, /* Debug breakpoint suspension */
CORO_STATE_RUNNING,
CORO_STATE_DEAD
} coro_state_t;
Expand Down Expand Up @@ -613,3 +615,69 @@ uint32_t coro_current_hart_id(void)

return coro_state.current_hart;
}

/* Debug-related coroutine functions for GDB integration */

void coro_suspend_hart_debug(uint32_t hart_id)
{
if (hart_id >= coro_state.hart_slots || !coro_state.initialized)
return;

/* Mark the hart as suspended for debugging.
* The scheduler should skip this hart until resumed.
*/
coro_t *co = coro_state.coroutines[hart_id];
if (co && co->state != CORO_STATE_DEAD)
co->state = CORO_STATE_DEBUG_SUSPENDED;
}

void coro_resume_hart_debug(uint32_t hart_id)
{
if (hart_id >= coro_state.hart_slots || !coro_state.initialized)
return;

/* Resume the hart from debug suspension.
* Transition from DEBUG_SUSPENDED to SUSPENDED, then delegate to
* coro_resume_hart which handles the actual context switch.
*/
coro_t *co = coro_state.coroutines[hart_id];
if (co && co->state == CORO_STATE_DEBUG_SUSPENDED) {
co->state = CORO_STATE_SUSPENDED;
coro_resume_hart(hart_id);
Copy link
Contributor

@RinHizakura RinHizakura Nov 2, 2025

Choose a reason for hiding this comment

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

It looks like we can remove the if condition here, as the same thing will be checked again in coro_resume_hart

}
}

bool coro_is_debug_suspended(uint32_t hart_id)
{
if (hart_id >= coro_state.hart_slots || !coro_state.initialized)
return false;

coro_t *co = coro_state.coroutines[hart_id];
return (co && co->state == CORO_STATE_DEBUG_SUSPENDED);
}

bool coro_step_hart(uint32_t hart_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Logically, this is the same as calling coro_resume_hart. Isn't it?

If so, I suggest we just let coro_resume_hart return the error value, and call coro_resume_hart under coro_step_hart directly, instead of doing so many duplicate checks.

{
if (hart_id >= coro_state.hart_slots || !coro_state.initialized)
return false;

coro_t *co = coro_state.coroutines[hart_id];
if (!co)
return false;

/* Only allow stepping if hart is currently debug-suspended */
if (co->state != CORO_STATE_DEBUG_SUSPENDED)
return false;

/* Transition to SUSPENDED state so coro_resume_hart can proceed */
co->state = CORO_STATE_SUSPENDED;

/* Resume the coroutine for one instruction.
* The hart coroutine should check single_step_mode and yield after one
* instruction, transitioning back to DEBUG_SUSPENDED state.
*/
coro_resume_hart(hart_id);

/* Verify that the hart actually yielded back after single-step */
return (co->state == CORO_STATE_DEBUG_SUSPENDED);
}
20 changes: 20 additions & 0 deletions coro.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,23 @@ bool coro_is_suspended(uint32_t slot_id);

/* Get currently executing hart ID */
uint32_t coro_current_hart_id(void);

/* Debug-related coroutine functions for GDB integration */

/* Suspend a hart for debugging (e.g., hit breakpoint)
* The hart will not be scheduled until explicitly resumed.
* This is different from WFI suspension.
*/
void coro_suspend_hart_debug(uint32_t hart_id);
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 2, 2025

Choose a reason for hiding this comment

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

Debug suspend should not reuse the same CORO_STATE_SUSPENDED flag that WFI uses; otherwise coro_is_suspended() cannot distinguish WFI from debugger freezes and the scheduler will treat breakpoints as WFI sleeps.

Prompt for AI agents
Address the following comment on coro.h at line 47:

<comment>Debug suspend should not reuse the same CORO_STATE_SUSPENDED flag that WFI uses; otherwise coro_is_suspended() cannot distinguish WFI from debugger freezes and the scheduler will treat breakpoints as WFI sleeps.</comment>

<file context>
@@ -37,3 +37,23 @@ bool coro_is_suspended(uint32_t slot_id);
+ * The hart will not be scheduled until explicitly resumed.
+ * This is different from WFI suspension.
+ */
+void coro_suspend_hart_debug(uint32_t hart_id);
+
+/* Resume a hart from debug suspension */
</file context>
Fix with Cubic


/* Resume a hart from debug suspension */
void coro_resume_hart_debug(uint32_t hart_id);

/* Check if a hart is suspended for debugging */
bool coro_is_debug_suspended(uint32_t hart_id);

/* Execute exactly one instruction on a hart (for single-step debugging)
* This will resume the hart, execute one instruction, then suspend again.
* Returns: true on success, false if hart is not in valid state
*/
bool coro_step_hart(uint32_t hart_id);
198 changes: 198 additions & 0 deletions gdbctrl.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/* GDB control layer for coroutine-based multi-hart execution */

#include <string.h>

#include "coro.h"
#include "gdbctrl.h"

bool gdb_debug_init(vm_t *vm)
{
if (!vm)
return false;

/* Initialize debug context */
memset(&vm->debug_ctx, 0, sizeof(vm_debug_ctx_t));

/* Initialize debug info for all harts */
for (uint32_t i = 0; i < vm->n_hart; i++) {
hart_t *hart = vm->hart[i];
if (!hart)
continue;

memset(&hart->debug_info, 0, sizeof(hart_debug_info_t));
hart->debug_info.state = HART_STATE_RUNNING;
hart->debug_info.single_step_mode = false;
hart->debug_info.breakpoint_pending = false;
}

return true;
}

void gdb_debug_cleanup(vm_t *vm)
{
if (!vm)
return;

/* Clear all breakpoints */
gdb_clear_all_breakpoints(vm);

/* Reset all hart debug states */
for (uint32_t i = 0; i < vm->n_hart; i++) {
hart_t *hart = vm->hart[i];
if (!hart)
continue;

hart->debug_info.state = HART_STATE_RUNNING;
hart->debug_info.single_step_mode = false;
hart->debug_info.breakpoint_pending = false;
}
}

bool gdb_check_breakpoint(hart_t *hart)
{
if (!hart || !hart->vm)
return false;

vm_t *vm = hart->vm;
uint32_t pc = hart->pc;

/* Check if current PC matches any enabled breakpoint */
for (uint32_t i = 0; i < vm->debug_ctx.bp_count; i++) {
breakpoint_t *bp = &vm->debug_ctx.breakpoints[i];
if (bp->enabled && bp->addr == pc) {
/* Breakpoint hit */
hart->debug_info.breakpoint_pending = true;
return true;
}
}

return false;
}

bool gdb_set_breakpoint(vm_t *vm, uint32_t addr)
{
if (!vm)
return false;

/* Check if breakpoint already exists at this address */
for (uint32_t i = 0; i < vm->debug_ctx.bp_count; i++) {
if (vm->debug_ctx.breakpoints[i].addr == addr) {
/* Already exists, just ensure it's enabled */
vm->debug_ctx.breakpoints[i].enabled = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just came up with the idea that we may combine enabled and addr together by using the bit0.

But such optimization can be minor, and it could be fine in debug mode.

return true;
}
}

/* Check if we have room for a new breakpoint */
if (vm->debug_ctx.bp_count >= MAX_BREAKPOINTS)
return false;

/* Add new breakpoint */
breakpoint_t *bp = &vm->debug_ctx.breakpoints[vm->debug_ctx.bp_count];
bp->addr = addr;
bp->enabled = true;
vm->debug_ctx.bp_count++;

return true;
}

bool gdb_del_breakpoint(vm_t *vm, uint32_t addr)
{
if (!vm)
return false;

/* Find the breakpoint */
for (uint32_t i = 0; i < vm->debug_ctx.bp_count; i++) {
if (vm->debug_ctx.breakpoints[i].addr == addr) {
/* Found it - remove by shifting remaining breakpoints down */
uint32_t remaining = vm->debug_ctx.bp_count - i - 1;
if (remaining > 0) {
memmove(&vm->debug_ctx.breakpoints[i],
&vm->debug_ctx.breakpoints[i + 1],
remaining * sizeof(breakpoint_t));
}
vm->debug_ctx.bp_count--;
return true;
}
}

return false;
}

void gdb_clear_all_breakpoints(vm_t *vm)
{
if (!vm)
return;

memset(&vm->debug_ctx.breakpoints, 0, sizeof(vm->debug_ctx.breakpoints));
vm->debug_ctx.bp_count = 0;
}

uint32_t gdb_get_breakpoint_count(vm_t *vm)
{
if (!vm)
return 0;

return vm->debug_ctx.bp_count;
}

void gdb_suspend_hart(hart_t *hart)
{
if (!hart)
return;

/* Mark hart as suspended for debugging */
hart->debug_info.state = HART_STATE_DEBUG_BREAK;

/* Suspend the coroutine (only in SMP mode) */
if (hart->vm && hart->vm->n_hart > 1)
coro_suspend_hart_debug(hart->mhartid);
}

void gdb_resume_hart(hart_t *hart)
{
if (!hart)
return;

/* Clear breakpoint pending flag */
hart->debug_info.breakpoint_pending = false;

/* If single-step mode is enabled, mark as DEBUG_STEP instead of RUNNING */
if (hart->debug_info.single_step_mode) {
hart->debug_info.state = HART_STATE_DEBUG_STEP;
} else {
hart->debug_info.state = HART_STATE_RUNNING;
}

/* Resume the coroutine (only in SMP mode) */
if (hart->vm && hart->vm->n_hart > 1) {
coro_resume_hart_debug(hart->mhartid);
}
}

void gdb_enable_single_step(hart_t *hart)
{
if (!hart)
return;

hart->debug_info.single_step_mode = true;
hart->debug_info.state = HART_STATE_DEBUG_STEP;
}

void gdb_disable_single_step(hart_t *hart)
{
if (!hart)
return;

hart->debug_info.single_step_mode = false;
if (hart->debug_info.state == HART_STATE_DEBUG_STEP)
hart->debug_info.state = HART_STATE_RUNNING;
}

bool gdb_is_single_stepping(hart_t *hart)
{
if (!hart)
return false;

return hart->debug_info.single_step_mode;
}
Loading
Loading