From feb83b78b7d66248217b14cda4a5d1b8306a4a4a Mon Sep 17 00:00:00 2001 From: Jan Matyas Date: Fri, 11 Feb 2022 08:16:10 +0100 Subject: [PATCH 1/2] fix progbuf cache: invalidate it when needed This commit relates to progbuf cache, implemented in https://github.com/riscv/riscv-openocd/pull/381 Make sure the cache gets invalidated when the progbuf contents change via other means. I've identified two such cases where the invalidation is required: 1) When the user manually tinkers with the progbuf registers (TCL command "riscv dmi_write") 2) When program buffer is used as a scratch memory (scratch_write64()) Change-Id: Ie7ffb0fccda63297de894ab919d09082ea21cfae Signed-off-by: Jan Matyas --- src/target/riscv/riscv-013.c | 15 +++++++++++++++ src/target/riscv/riscv.c | 18 ++++++++++++++---- src/target/riscv/riscv.h | 1 + 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index ec6821d14..9ed2d2f53 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -52,6 +52,7 @@ static int riscv013_write_debug_buffer(struct target *target, unsigned index, riscv_insn_t d); static riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned index); +static int riscv013_invalidate_cached_debug_buffer(struct target *target); static int riscv013_execute_debug_buffer(struct target *target); static void riscv013_fill_dmi_write_u64(struct target *target, char *buf, int a, uint64_t d); static void riscv013_fill_dmi_read_u64(struct target *target, char *buf, int a); @@ -1248,6 +1249,7 @@ static int scratch_write64(struct target *target, scratch_mem_t *scratch, case SPACE_DMI_PROGBUF: dmi_write(target, DM_PROGBUF0 + scratch->debug_address, value); dmi_write(target, DM_PROGBUF1 + scratch->debug_address, value >> 32); + riscv013_invalidate_cached_debug_buffer(target); break; case SPACE_DMI_RAM: { @@ -2278,6 +2280,7 @@ static int init_target(struct command_context *cmd_ctx, generic_info->read_debug_buffer = &riscv013_read_debug_buffer; generic_info->write_debug_buffer = &riscv013_write_debug_buffer; generic_info->execute_debug_buffer = &riscv013_execute_debug_buffer; + generic_info->invalidate_cached_debug_buffer = &riscv013_invalidate_cached_debug_buffer; generic_info->fill_dmi_write_u64 = &riscv013_fill_dmi_write_u64; generic_info->fill_dmi_read_u64 = &riscv013_fill_dmi_read_u64; generic_info->fill_dmi_nop_u64 = &riscv013_fill_dmi_nop_u64; @@ -4337,6 +4340,18 @@ riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned index) return value; } +int riscv013_invalidate_cached_debug_buffer(struct target *target) +{ + dm013_info_t *dm = get_dm(target); + if (!dm) + return ERROR_FAIL; + + LOG_TARGET_DEBUG(target, "Invalidating progbuf cache"); + for (unsigned int i = 0; i < 15; i++) + dm->progbuf_cache[i] = 0; + return ERROR_OK; +} + int riscv013_execute_debug_buffer(struct target *target) { uint32_t run_program = 0; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 09c1939fe..783d937f7 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -2795,11 +2795,21 @@ COMMAND_HANDLER(riscv_dmi_write) COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], value); if (r->dmi_write) { - return r->dmi_write(target, address, value); - } else { - LOG_ERROR("dmi_write is not implemented for this target."); - return ERROR_FAIL; + /* Perform the DMI write */ + int retval = r->dmi_write(target, address, value); + + /* If the user tinkered with progbuf registers, we need to + drop our cached copy of the progbuf */ + if (address >= DM_PROGBUF0 && address <= DM_PROGBUF15) { + if (r->invalidate_cached_debug_buffer) + r->invalidate_cached_debug_buffer(target); + } + + return retval; } + + LOG_ERROR("dmi_write is not implemented for this target."); + return ERROR_FAIL; } COMMAND_HANDLER(riscv_test_sba_config_reg) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 531c896b9..f1104e35b 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -161,6 +161,7 @@ typedef struct { riscv_insn_t d); riscv_insn_t (*read_debug_buffer)(struct target *target, unsigned index); int (*execute_debug_buffer)(struct target *target); + int (*invalidate_cached_debug_buffer)(struct target *target); int (*dmi_write_u64_bits)(struct target *target); void (*fill_dmi_write_u64)(struct target *target, char *buf, int a, uint64_t d); void (*fill_dmi_read_u64)(struct target *target, char *buf, int a); From 8274cc58c1a1b44ac23fd542f136d9265c6c3252 Mon Sep 17 00:00:00 2001 From: Jan Matyas Date: Mon, 14 Feb 2022 14:10:17 +0100 Subject: [PATCH 2/2] fix progbuf cache: another two cases for invalidation Continuation of the previous patch. There are two more cases when progbuf cache in OpenOCD shall be invalidated: - When OpenOCD resets the debug module undergoes reset (dmactive=0), e.g. during target examination - When the user manually performs that very same operation (via riscv dmi_write) Change-Id: I53f8f08250eeedcbd55ab4361d5665370b063680 Signed-off-by: Jan Matyas --- src/target/riscv/riscv-013.c | 5 ++++- src/target/riscv/riscv.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 9ed2d2f53..62866dcfd 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1568,6 +1568,9 @@ static int examine(struct target *target) dmi_write(target, DM_DMCONTROL, 0); dmi_write(target, DM_DMCONTROL, DM_DMCONTROL_DMACTIVE); dm->was_reset = true; + + /* The DM gets reset, so forget any cached progbuf entries. */ + riscv013_invalidate_cached_debug_buffer(target); } dmi_write(target, DM_DMCONTROL, DM_DMCONTROL_HARTSELLO | @@ -2371,7 +2374,7 @@ static int assert_reset(struct target *target) /* The DM might have gotten reset if OpenOCD called us in some reset that * involves SRST being toggled. So clear our cache which may be out of * date. */ - memset(dm->progbuf_cache, 0, sizeof(dm->progbuf_cache)); + riscv013_invalidate_cached_debug_buffer(target); return ERROR_OK; } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 783d937f7..7be42ef5e 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -2798,9 +2798,14 @@ COMMAND_HANDLER(riscv_dmi_write) /* Perform the DMI write */ int retval = r->dmi_write(target, address, value); - /* If the user tinkered with progbuf registers, we need to - drop our cached copy of the progbuf */ - if (address >= DM_PROGBUF0 && address <= DM_PROGBUF15) { + /* Invalidate our cached progbuf copy: + - if the user tinkered directly with a progbuf register + - if debug module was reset, in which case progbuf registers + may not retain their value. + */ + bool progbufTouched = (address >= DM_PROGBUF0 && address <= DM_PROGBUF15); + bool dmDeactivated = (address == DM_DMCONTROL && (value & DM_DMCONTROL_DMACTIVE) == 0); + if (progbufTouched || dmDeactivated) { if (r->invalidate_cached_debug_buffer) r->invalidate_cached_debug_buffer(target); }