Skip to content

Commit

Permalink
target/riscv: cache requests to trigger configuration
Browse files Browse the repository at this point in the history 
Depending on configuration, the existing implementation of watchpoints is
rather inefficient for certain scenarios. Consider HW that:

1. triggers 0-3 can be used as instruction breakpoints
2. triggers 4-7 can be used as data breakpoints (watchpoints)
3.  NAPOT triggers are not supported.

Now, consider that we have a pending watchpoint. And we perform a "step"
operation. According to the current implementation:

* OpenOCD will disable watchpoints
* Perform a single-step
* Will try to restore the original watchpoints. It will need 12 attempts
to find a suitable trigger: (8 attempts to try NAPOT, and another 4 to try
GE+LE).

This patch introduces a dedicated cache for requests to triggers. It
significantly speeds things up, since we cache failed attempts and no
additional interactions with HW is necessary.

Change-Id: Ic272895eaa763a7ae84d14f7633790afd015ca9d
Signed-off-by: Anastasiya Chernikova <anastasiya.chernikova@syntacore.com>
  • Loading branch information
@AnastasiyaChernikova
AnastasiyaChernikova committed Nov 2, 2023
1 parent aad90d8 commit 6c43689
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 9 deletions.
150 changes: 141 additions & 9 deletions src/target/riscv/riscv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ struct trigger {
int unique_id;
};

struct tdata2_cache {
struct list_head elem_tdata2;
riscv_reg_t tdata2;
};

struct tdata1_cache {
riscv_reg_t tdata1;
struct list_head tdata2_cache_head;
struct list_head elem_tdata1;
};

/* Wall-clock timeout for a command/access. Settable via RISC-V Target commands.*/
int riscv_command_timeout_sec = DEFAULT_COMMAND_TIMEOUT_SEC;

Expand Down Expand Up @@ -488,6 +499,26 @@ static void riscv_free_registers(struct target *target)
}
}


static void free_wp_triggers_cache(struct target *target)
{
RISCV_INFO(r);

for (unsigned int i = 0; i < r->trigger_count; ++i) {
struct tdata1_cache *elem_1, *tmp_1;
list_for_each_entry_safe(elem_1, tmp_1, &r->wp_triggers_negative_cache[i], elem_tdata1) {
struct tdata2_cache *elem_2, *tmp_2;
list_for_each_entry_safe(elem_2, tmp_2, &elem_1->tdata2_cache_head, elem_tdata2) {
list_del(&elem_2->elem_tdata2);
free(elem_2);
}
list_del(&elem_1->elem_tdata1);
free(elem_1);
}
}
free(r->wp_triggers_negative_cache);
}

static void riscv_deinit_target(struct target *target)
{
LOG_TARGET_DEBUG(target, "riscv_deinit_target()");
Expand All @@ -504,6 +535,7 @@ static void riscv_deinit_target(struct target *target)
tt->deinit_target(target);

riscv_free_registers(target);
free_wp_triggers_cache(target);

if (!info)
return;
Expand Down Expand Up @@ -638,8 +670,8 @@ static int set_trigger(struct target *target, unsigned int idx, riscv_reg_t tdat
LOG_TARGET_DEBUG(target,
"wrote 0x%" PRIx64 " to tdata2 but read back 0x%" PRIx64,
tdata2, tdata2_rb);

riscv_set_register(target, GDB_REGNO_TDATA1, 0);
if (riscv_set_register(target, GDB_REGNO_TDATA1, 0) != ERROR_OK)
return ERROR_FAIL;
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
}

Expand Down Expand Up @@ -705,6 +737,99 @@ static void log_trigger_request_info(struct trigger_request_info trig_info)
trig_info.tdata1, trig_info.tdata2, trig_info.tdata1_ignore_mask);
};

static struct tdata1_cache *tdata1_cache_alloc(struct list_head *tdata1_cache_head, riscv_reg_t tdata1)
{
struct tdata1_cache *elem = (struct tdata1_cache *)calloc(1, sizeof(struct tdata1_cache));
elem->tdata1 = tdata1;
INIT_LIST_HEAD(&elem->tdata2_cache_head);
list_add_tail(&elem->elem_tdata1, tdata1_cache_head);
return elem;
}

static void tdata2_cache_alloc(struct list_head *tdata2_cache_head, riscv_reg_t tdata2)
{
struct tdata2_cache * const elem = calloc(1, sizeof(struct tdata2_cache));
elem->tdata2 = tdata2;
list_add(&elem->elem_tdata2, tdata2_cache_head);
}

struct tdata2_cache *tdata2_cache_search(struct list_head *tdata2_cache_head, riscv_reg_t find_tdata2)
{
struct tdata2_cache *elem_2;
list_for_each_entry(elem_2, tdata2_cache_head, elem_tdata2) {
if (elem_2->tdata2 == find_tdata2)
return elem_2;
}
return NULL;
}

struct tdata1_cache *tdata1_cache_search(struct list_head *tdata1_cache_head, riscv_reg_t find_tdata1)
{
struct tdata1_cache *elem_1;
list_for_each_entry(elem_1, tdata1_cache_head, elem_tdata1) {
if (elem_1->tdata1 == find_tdata1)
return elem_1;
}
return NULL;
}

static void create_wp_trigger_cache(struct target *target)
{
RISCV_INFO(r);

r->wp_triggers_negative_cache = (struct list_head *)calloc(r->trigger_count,
sizeof(struct list_head));
for (unsigned int i = 0; i < r->trigger_count; ++i)
INIT_LIST_HEAD(&r->wp_triggers_negative_cache[i]);
}

static void wp_triggers_cache_add(struct target *target, unsigned int idx, riscv_reg_t tdata1,
riscv_reg_t tdata2, int error_code)
{
RISCV_INFO(r);

struct tdata1_cache *tdata1_cache = tdata1_cache_search(&r->wp_triggers_negative_cache[idx], tdata1);
if (!tdata1_cache) {
tdata1_cache = tdata1_cache_alloc(&r->wp_triggers_negative_cache[idx], tdata1);
} else {
struct tdata2_cache *tdata2_cache = tdata2_cache_search(&tdata1_cache->tdata2_cache_head, tdata2);
if (tdata2_cache) {
list_move(&tdata2_cache->elem_tdata2, &tdata1_cache->tdata2_cache_head);
return;
}
}
tdata2_cache_alloc(&tdata1_cache->tdata2_cache_head, tdata2);
}

static bool wp_triggers_cache_search(struct target *target, unsigned int idx,
riscv_reg_t tdata1, riscv_reg_t tdata2)
{
RISCV_INFO(r);

struct tdata1_cache *tdata1_cache = tdata1_cache_search(&r->wp_triggers_negative_cache[idx], tdata1);
if (!tdata1_cache)
return false;
struct tdata2_cache *tdata2_cache = tdata2_cache_search(&tdata1_cache->tdata2_cache_head, tdata2);
if (!tdata2_cache)
return false;
assert(tdata1_cache->tdata1 == tdata1 && tdata2_cache->tdata2 == tdata2);
return true;
}

static int try_use_trigger_and_cache_result(struct target *target, unsigned int idx, riscv_reg_t tdata1,
riscv_reg_t tdata2, riscv_reg_t tdata1_ignore_mask)
{
if (wp_triggers_cache_search(target, idx, tdata1, tdata2))
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;

int ret = set_trigger(target, idx, tdata1, tdata2, tdata1_ignore_mask);

/* Add these values to the cache to remember that they are not supported. */
if (ret == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
wp_triggers_cache_add(target, idx, tdata1, tdata2, ret);
return ret;
}

static int try_setup_single_match_trigger(struct target *target,
struct trigger *trigger, struct trigger_request_info trig_info)
{
Expand All @@ -720,8 +845,9 @@ static int try_setup_single_match_trigger(struct target *target,
for (unsigned int idx = 0;
find_next_free_trigger(target, trigger_type, false, &idx) == ERROR_OK;
++idx) {
ret = set_trigger(target, idx, trig_info.tdata1, trig_info.tdata2,
trig_info.tdata1_ignore_mask);
ret = try_use_trigger_and_cache_result(target, idx, trig_info.tdata1, trig_info.tdata2,
trig_info.tdata1_ignore_mask);

if (ret == ERROR_OK) {
r->trigger_unique_id[idx] = trigger->unique_id;
return ERROR_OK;
Expand All @@ -748,12 +874,17 @@ static int try_setup_chained_match_triggers(struct target *target,
for (unsigned int idx = 0;
find_next_free_trigger(target, trigger_type, true, &idx) == ERROR_OK;
++idx) {
ret = set_trigger(target, idx, t1.tdata1, t1.tdata2,
t1.tdata1_ignore_mask);
if (ret != ERROR_OK)
ret = try_use_trigger_and_cache_result(target, idx, t1.tdata1, t1.tdata2,
t1.tdata1_ignore_mask);

if (ret == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
continue;
ret = set_trigger(target, idx + 1, t2.tdata1, t2.tdata2,
t2.tdata1_ignore_mask);
else if (ret != ERROR_OK)
return ret;

ret = try_use_trigger_and_cache_result(target, idx + 1, t2.tdata1, t2.tdata2,
t2.tdata1_ignore_mask);

if (ret == ERROR_OK) {
r->trigger_unique_id[idx] = trigger->unique_id;
r->trigger_unique_id[idx + 1] = trigger->unique_id;
Expand Down Expand Up @@ -5378,6 +5509,7 @@ int riscv_enumerate_triggers(struct target *target)
r->triggers_enumerated = true;
r->trigger_count = t;
LOG_TARGET_INFO(target, "Found %d triggers", r->trigger_count);
create_wp_trigger_cache(target);
return ERROR_OK;
}

Expand Down
6 changes: 6 additions & 0 deletions src/target/riscv/riscv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ struct riscv_info {
/* The number of triggers per hart. */
unsigned int trigger_count;

/* Data structure to record known unsupported tdata1+tdata2 trigger CSR values.
* This is to avoid repetitive attempts to set trigger configurations that are already
* known to be unsupported in the HW.
* A separate data structure is created for each trigger. */
struct list_head *wp_triggers_negative_cache;

/* record the tinfo of each trigger */
unsigned int trigger_tinfo[RISCV_MAX_TRIGGERS];

Expand Down

0 comments on commit 6c43689

Please sign in to comment.