interesting
/
riscv-openocd
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

target/riscv: implement abstract command cache 

Implemented cache of unsupported abstract commands.
It's purpose is to replace set of caching variables to one.
So this commit provides one ac_not_supported_cache
instead of abstract_read_csr_supported,
abstract_write_csr_supported, abstract_read_fpr_supported,
abstract_write_fpr_supported, has_aampostincrement.

Dropped check for buggy aampostincrement

Fixes #1232

Change-Id: I75cae1481841f2cd0393d6cc80f0d913fbe34294
Signed-off-by: Farid Khaydari <f.khaydari@syntacore.com>
This commit is contained in:
Farid Khaydari 2025-02-25 00:21:08 +03:00
parent fa7e2351c8
commit cc7e5e3aad
1 changed files with 111 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

221
src/target/riscv/riscv-013.c
View File

@ -145,6 +145,60 @@ typedef struct {
struct target *target; struct target *target;
} target_list_t; } target_list_t;
struct ac_cache {
uint32_t *commands;
size_t size;
};
static int ac_cache_elem_comparator(const void *lhs, const void *rhs)
{
return *(const uint32_t *)rhs - *(const uint32_t *)lhs;
}
static struct ac_cache ac_cache_construct(void)
{
struct ac_cache cache = {
cache.commands = NULL,
cache.size = 0,
};
return cache;
}
static void ac_cache_free(struct ac_cache *cache)
{
free(cache->commands);
cache->commands = NULL;
cache->size = 0;
}
static void ac_cache_insert(struct ac_cache *cache, uint32_t command)
{
assert(cache);
size_t old_size = cache->size;
size_t new_size = old_size + 1;
size_t entry_size = sizeof(*cache->commands);
uint32_t *commands = realloc(cache->commands, new_size * entry_size);
if (!commands) {
LOG_ERROR("Reallocation to %zu bytes failed", new_size);
return;
}
commands[old_size] = command;
cache->commands = commands;
cache->size = new_size;
qsort(cache->commands, cache->size, entry_size,
ac_cache_elem_comparator);
}
static bool ac_cache_contains(const struct ac_cache *cache, uint32_t command)
{
return bsearch(&command, cache->commands, cache->size,
sizeof(*cache->commands), ac_cache_elem_comparator);
}
typedef struct { typedef struct {
/* The indexed used to address this hart in its DM. */ /* The indexed used to address this hart in its DM. */
unsigned int index; unsigned int index;
@ -175,12 +229,7 @@ typedef struct {
*/ */
struct riscv_scan_delays learned_delays; struct riscv_scan_delays learned_delays;
bool abstract_read_csr_supported; struct ac_cache ac_not_supported_cache;
bool abstract_write_csr_supported;
bool abstract_read_fpr_supported;
bool abstract_write_fpr_supported;
yes_no_maybe_t has_aampostincrement;
/* Some fields from hartinfo. */ /* Some fields from hartinfo. */
uint8_t datasize; uint8_t datasize;
@ -685,6 +734,11 @@ int riscv013_execute_abstract_command(struct target *target, uint32_t command,
res = abstract_cmd_batch_check_and_clear_cmderr(target, batch, res = abstract_cmd_batch_check_and_clear_cmderr(target, batch,
abstractcs_read_key, cmderr); abstractcs_read_key, cmderr);
cleanup: cleanup:
if (res != ERROR_OK && *cmderr == CMDERR_NOT_SUPPORTED) {
LOG_TARGET_DEBUG(target, "Command 0x%" PRIx32
"is cached as not supported, bailing out", command);
ac_cache_insert(&get_info(target)->ac_not_supported_cache, command);
}
riscv_batch_free(batch); riscv_batch_free(batch);
return res; return res;
} }
@ -834,33 +888,22 @@ static int register_read_abstract_with_size(struct target *target,
{ {
RISCV013_INFO(info); RISCV013_INFO(info);
if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31 &&
!info->abstract_read_fpr_supported)
return ERROR_FAIL;
if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095 &&
!info->abstract_read_csr_supported)
return ERROR_FAIL;
/* The spec doesn't define abstract register numbers for vector registers. */ /* The spec doesn't define abstract register numbers for vector registers. */
if (number >= GDB_REGNO_V0 && number <= GDB_REGNO_V31) if (number >= GDB_REGNO_V0 && number <= GDB_REGNO_V31)
return ERROR_FAIL; return ERROR_FAIL;
uint32_t command = riscv013_access_register_command(target, number, size, uint32_t command = riscv013_access_register_command(target, number, size,
AC_ACCESS_REGISTER_TRANSFER); AC_ACCESS_REGISTER_TRANSFER);
if (ac_cache_contains(&info->ac_not_supported_cache, command)) {
LOG_TARGET_DEBUG(target, "Command is cached as not supported, bailing out");
LOG_DEBUG_REG(target, AC_ACCESS_REGISTER, command);
return ERROR_FAIL;
}
uint32_t cmderr; uint32_t cmderr;
int result = riscv013_execute_abstract_command(target, command, &cmderr); int result = riscv013_execute_abstract_command(target, command, &cmderr);
if (result != ERROR_OK) { if (result != ERROR_OK)
if (cmderr == CMDERR_NOT_SUPPORTED) {
if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) {
info->abstract_read_fpr_supported = false;
LOG_TARGET_INFO(target, "Disabling abstract command reads from FPRs.");
} else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) {
info->abstract_read_csr_supported = false;
LOG_TARGET_INFO(target, "Disabling abstract command reads from CSRs.");
}
}
return result; return result;
}
if (value) if (value)
return read_abstract_arg(target, value, 0, size); return read_abstract_arg(target, value, 0, size);
@ -885,17 +928,16 @@ static int register_write_abstract(struct target *target, enum gdb_regno number,
if (!dm) if (!dm)
return ERROR_FAIL; return ERROR_FAIL;
if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31 &&
!info->abstract_write_fpr_supported)
return ERROR_FAIL;
if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095 &&
!info->abstract_write_csr_supported)
return ERROR_FAIL;
const unsigned int size_bits = register_size(target, number); const unsigned int size_bits = register_size(target, number);
const uint32_t command = riscv013_access_register_command(target, number, size_bits, const uint32_t command = riscv013_access_register_command(target, number, size_bits,
AC_ACCESS_REGISTER_TRANSFER | AC_ACCESS_REGISTER_TRANSFER |
AC_ACCESS_REGISTER_WRITE); AC_ACCESS_REGISTER_WRITE);
if (ac_cache_contains(&info->ac_not_supported_cache, command)) {
LOG_TARGET_DEBUG(target, "Command is cached as not supported, bailing out");
LOG_DEBUG_REG(target, AC_ACCESS_REGISTER, command);
return ERROR_FAIL;
}
LOG_DEBUG_REG(target, AC_ACCESS_REGISTER, command); LOG_DEBUG_REG(target, AC_ACCESS_REGISTER, command);
assert(size_bits % 32 == 0); assert(size_bits % 32 == 0);
const unsigned int size_in_words = size_bits / 32; const unsigned int size_in_words = size_bits / 32;
@ -916,16 +958,10 @@ static int register_write_abstract(struct target *target, enum gdb_regno number,
res = abstract_cmd_batch_check_and_clear_cmderr(target, batch, res = abstract_cmd_batch_check_and_clear_cmderr(target, batch,
abstractcs_read_key, &cmderr); abstractcs_read_key, &cmderr);
if (res != ERROR_OK) { if (res != ERROR_OK && cmderr == CMDERR_NOT_SUPPORTED) {
if (cmderr == CMDERR_NOT_SUPPORTED) { LOG_TARGET_DEBUG(target, "Caching the command as not supported");
if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { LOG_DEBUG_REG(target, AC_ACCESS_REGISTER, command);
info->abstract_write_fpr_supported = false; ac_cache_insert(&info->ac_not_supported_cache, command);
LOG_TARGET_INFO(target, "Disabling abstract command writes to FPRs.");
} else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) {
info->abstract_write_csr_supported = false;
LOG_TARGET_INFO(target, "Disabling abstract command writes to CSRs.");
}
}
} }
cleanup: cleanup:
riscv_batch_free(batch); riscv_batch_free(batch);
@ -1709,6 +1745,10 @@ static void deinit_target(struct target *target)
if (!info) if (!info)
return; return;
riscv013_info_t *vsinfo = info->version_specific;
if (vsinfo)
ac_cache_free(&vsinfo->ac_not_supported_cache);
riscv013_dm_free(target); riscv013_dm_free(target);
free(info->version_specific); free(info->version_specific);
@ -2819,17 +2859,7 @@ static int init_target(struct command_context *cmd_ctx,
info->progbufsize = -1; info->progbufsize = -1;
reset_learned_delays(target); reset_learned_delays(target);
/* Assume all these abstract commands are supported until we learn info->ac_not_supported_cache = ac_cache_construct();
* otherwise.
* TODO: The spec allows eg. one CSR to be able to be accessed abstractly
* while another one isn't. We don't track that this closely here, but in
* the future we probably should. */
info->abstract_read_csr_supported = true;
info->abstract_write_csr_supported = true;
info->abstract_read_fpr_supported = true;
info->abstract_write_fpr_supported = true;
info->has_aampostincrement = YNM_MAYBE;
return ERROR_OK; return ERROR_OK;
} }
@ -3720,15 +3750,19 @@ read_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
assert(riscv_mem_access_is_read(args)); assert(riscv_mem_access_is_read(args));
RISCV013_INFO(info); RISCV013_INFO(info);
bool use_aampostincrement = info->has_aampostincrement != YNM_NO;
memset(args.read_buffer, 0, args.count * args.size); memset(args.read_buffer, 0, args.count * args.size);
/* Convert the size (bytes) to width (bits) */ /* Convert the size (bytes) to width (bits) */
unsigned int width = args.size << 3; unsigned int width = args.size << 3;
/* Create the command (physical address, postincrement, read) */ /* Create the command (physical address, postincrement, read) */
uint32_t command = access_memory_command(target, false, width, use_aampostincrement, false); uint32_t command = access_memory_command(target, false, width,
/* postincrement = */ true, /* is_write= */ false);
bool use_aampostincrement =
!ac_cache_contains(&info->ac_not_supported_cache, command);
if (!use_aampostincrement)
command = access_memory_command(target, false, width,
/* postincrement = */ false, /* is_write= */ false);
/* Execute the reads */ /* Execute the reads */
uint8_t *p = args.read_buffer; uint8_t *p = args.read_buffer;
@ -3749,33 +3783,14 @@ read_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
/* Execute the command */ /* Execute the command */
uint32_t cmderr; uint32_t cmderr;
result = riscv013_execute_abstract_command(target, command, &cmderr); result = riscv013_execute_abstract_command(target, command, &cmderr);
if (use_aampostincrement && result != ERROR_OK &&
cmderr == CMDERR_NOT_SUPPORTED) {
LOG_TARGET_DEBUG(target, "aampostincrement is not supported on this target.");
use_aampostincrement = false;
/* TODO: we need to modify error handling here. */ /* Try the same access but with postincrement disabled. */
/* NOTE: in case of timeout cmderr is set to CMDERR_NONE */ command = access_memory_command(target, false, width, false, false);
if (info->has_aampostincrement == YNM_MAYBE) { result = riscv013_execute_abstract_command(target, command, &cmderr);
if (result == ERROR_OK) {
/* Safety: double-check that the address was really auto-incremented */
riscv_reg_t new_address;
result = read_abstract_arg(target, &new_address, 1, riscv_xlen(target));
if (result != ERROR_OK)
return mem_access_result(MEM_ACCESS_FAILED_DM_ACCESS_FAILED);
if (new_address == args.address + args.size) {
LOG_TARGET_DEBUG(target, "aampostincrement is supported on this target.");
info->has_aampostincrement = YNM_YES;
} else {
LOG_TARGET_WARNING(target, "Buggy aampostincrement! Address not incremented correctly.");
info->has_aampostincrement = YNM_NO;
}
} else {
/* Try the same access but with postincrement disabled. */
command = access_memory_command(target, false, width, false, false);
result = riscv013_execute_abstract_command(target, command, &cmderr);
if (result == ERROR_OK) {
LOG_TARGET_DEBUG(target, "aampostincrement is not supported on this target.");
info->has_aampostincrement = YNM_NO;
}
}
} }
/* TODO: /* TODO:
@ -3791,7 +3806,7 @@ read_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
return mem_access_result(MEM_ACCESS_FAILED_DM_ACCESS_FAILED); return mem_access_result(MEM_ACCESS_FAILED_DM_ACCESS_FAILED);
buf_set_u64(p, 0, 8 * args.size, value); buf_set_u64(p, 0, 8 * args.size, value);
if (info->has_aampostincrement == YNM_YES) if (use_aampostincrement)
updateaddr = false; updateaddr = false;
p += args.size; p += args.size;
} }
@ -3811,13 +3826,18 @@ write_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
RISCV013_INFO(info); RISCV013_INFO(info);
int result = ERROR_OK; int result = ERROR_OK;
bool use_aampostincrement = info->has_aampostincrement != YNM_NO;
/* Convert the size (bytes) to width (bits) */ /* Convert the size (bytes) to width (bits) */
unsigned int width = args.size << 3; unsigned int width = args.size << 3;
/* Create the command (physical address, postincrement, write) */ /* Create the command (physical address, postincrement, write) */
uint32_t command = access_memory_command(target, false, width, use_aampostincrement, true); uint32_t command = access_memory_command(target, false, width,
/* postincrement = */ true, /* is_write = */ true);
bool use_aampostincrement =
!ac_cache_contains(&info->ac_not_supported_cache, command);
if (!use_aampostincrement)
command = access_memory_command(target, false, width,
/* postincrement = */ false, /* is_write= */ false);
/* Execute the writes */ /* Execute the writes */
const uint8_t *p = args.write_buffer; const uint8_t *p = args.write_buffer;
@ -3844,33 +3864,14 @@ write_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
/* Execute the command */ /* Execute the command */
uint32_t cmderr; uint32_t cmderr;
result = riscv013_execute_abstract_command(target, command, &cmderr); result = riscv013_execute_abstract_command(target, command, &cmderr);
if (use_aampostincrement && result != ERROR_OK &&
cmderr == CMDERR_NOT_SUPPORTED) {
LOG_TARGET_DEBUG(target, "aampostincrement is not supported on this target.");
use_aampostincrement = false;
/* TODO: we need to modify error handling here. */ /* Try the same access but with postincrement disabled. */
/* NOTE: in case of timeout cmderr is set to CMDERR_NONE */ command = access_memory_command(target, false, width, false, true);
if (info->has_aampostincrement == YNM_MAYBE) { result = riscv013_execute_abstract_command(target, command, &cmderr);
if (result == ERROR_OK) {
/* Safety: double-check that the address was really auto-incremented */
riscv_reg_t new_address;
result = read_abstract_arg(target, &new_address, 1, riscv_xlen(target));
if (result != ERROR_OK)
return mem_access_result(MEM_ACCESS_FAILED_DM_ACCESS_FAILED);
if (new_address == args.address + args.size) {
LOG_TARGET_DEBUG(target, "aampostincrement is supported on this target.");
info->has_aampostincrement = YNM_YES;
} else {
LOG_TARGET_WARNING(target, "Buggy aampostincrement! Address not incremented correctly.");
info->has_aampostincrement = YNM_NO;
}
} else {
/* Try the same access but with postincrement disabled. */
command = access_memory_command(target, false, width, false, true);
result = riscv013_execute_abstract_command(target, command, &cmderr);
if (result == ERROR_OK) {
LOG_TARGET_DEBUG(target, "aampostincrement is not supported on this target.");
info->has_aampostincrement = YNM_NO;
}
}
} }
/* TODO: /* TODO:
@ -3879,7 +3880,7 @@ write_memory_abstract(struct target *target, const riscv_mem_access_args_t args)
if (result != ERROR_OK) if (result != ERROR_OK)
return mem_access_result(MEM_ACCESS_SKIPPED_ABSTRACT_ACCESS_CMDERR); return mem_access_result(MEM_ACCESS_SKIPPED_ABSTRACT_ACCESS_CMDERR);
if (info->has_aampostincrement == YNM_YES) if (use_aampostincrement)
updateaddr = false; updateaddr = false;
p += args.size; p += args.size;
} }