From 4553abf9064fe3c0e4ea2ed29a1d2217df74ff5f Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Thu, 15 Jun 2017 08:59:01 +0200 Subject: [PATCH] arm_adi_v5: fix wrong addressing after change of CSW_ADDRINC Problem: If the same memory location is accessed alternatively by MEM-AP banked data registers without autoincrement and by standard autoincremented read/write, TAR register is not updated correctly. How to replicate: On a Cortex-M issue mdw 0xe000edf0 multiple times. When poll is on (poll reads the same memory location) only the first read is correct. 0xe000edf0: 01000000 0xe000edf0: 00000000 0xe000edf0: 20002640 0xe000edf0: 01000000 0xe000edf0: 00000000 0xe000edf0: 00000000 No problems with poll off. 0xe000edf0: 01000000 0xe000edf0: 01000000 0xe000edf0: 01000000 mem_ap_setup_tar() writes to MEM_AP_REG_TAR if requested TAR value changed or CSW_ADDRINC_... is currently active. However if an autoincremented access has been issued and autoinc switched off in CSW afterwards, TAR does not get updated. The change introduces mem_ap_update_tar_cache() which is called after queuing of any access to MEM_AP_REG_DRW. It simulates TAR increment to keep tar_value in sync with MEM_AP. Crossing tar autoincrement block boundary invalidates cached value. mem_ap_write() and mem_ap_read() do not check tar autoincrement block boundary, mem_ap_setup_tar() is called before each transfer instead. dap_invalidate_cache() is introduced to ensure invalidation of all cached values during dap_dp_init() and swd_connect() Change-Id: I815c2283d2989cffd6ea9a4100ce2f29dc3fb7b4 Signed-off-by: Tomas Vanek Reviewed-on: http://openocd.zylin.com/4162 Tested-by: jenkins Reviewed-by: Christopher Head Reviewed-by: Andreas Fritiofson --- src/target/adi_v5_swd.c | 2 +- src/target/arm_adi_v5.c | 115 +++++++++++++++++++++++++++++----------- src/target/arm_adi_v5.h | 7 +++ 3 files changed, 93 insertions(+), 31 deletions(-) diff --git a/src/target/adi_v5_swd.c b/src/target/adi_v5_swd.c index a6aada326..c503f090b 100644 --- a/src/target/adi_v5_swd.c +++ b/src/target/adi_v5_swd.c @@ -124,7 +124,7 @@ static int swd_connect(struct adiv5_dap *dap) /* Clear link state, including the SELECT cache. */ dap->do_reconnect = false; - dap->select = DP_SELECT_INVALID; + dap_invalidate_cache(dap); swd_queue_dp_read(dap, DP_DPIDR, &dpidr); diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c index 200629023..a0d0f4ef0 100644 --- a/src/target/arm_adi_v5.c +++ b/src/target/arm_adi_v5.c @@ -111,17 +111,68 @@ static int mem_ap_setup_csw(struct adiv5_ap *ap, uint32_t csw) static int mem_ap_setup_tar(struct adiv5_ap *ap, uint32_t tar) { - if (tar != ap->tar_value || - (ap->csw_value & CSW_ADDRINC_MASK)) { + if (!ap->tar_valid || tar != ap->tar_value) { /* LOG_DEBUG("DAP: Set TAR %x",tar); */ int retval = dap_queue_ap_write(ap, MEM_AP_REG_TAR, tar); if (retval != ERROR_OK) return retval; ap->tar_value = tar; + ap->tar_valid = true; } return ERROR_OK; } +static int mem_ap_read_tar(struct adiv5_ap *ap, uint32_t *tar) +{ + int retval = dap_queue_ap_read(ap, MEM_AP_REG_TAR, tar); + if (retval != ERROR_OK) { + ap->tar_valid = false; + return retval; + } + + retval = dap_run(ap->dap); + if (retval != ERROR_OK) { + ap->tar_valid = false; + return retval; + } + + ap->tar_value = *tar; + ap->tar_valid = true; + return ERROR_OK; +} + +static uint32_t mem_ap_get_tar_increment(struct adiv5_ap *ap) +{ + switch (ap->csw_value & CSW_ADDRINC_MASK) { + case CSW_ADDRINC_SINGLE: + switch (ap->csw_value & CSW_SIZE_MASK) { + case CSW_8BIT: + return 1; + case CSW_16BIT: + return 2; + case CSW_32BIT: + return 4; + } + case CSW_ADDRINC_PACKED: + return 4; + } + return 0; +} + +/* mem_ap_update_tar_cache is called after an access to MEM_AP_REG_DRW + */ +static void mem_ap_update_tar_cache(struct adiv5_ap *ap) +{ + if (!ap->tar_valid) + return; + + uint32_t inc = mem_ap_get_tar_increment(ap); + if (inc >= max_tar_block_size(ap->tar_autoincr_block, ap->tar_value)) + ap->tar_valid = false; + else + ap->tar_value += inc; +} + /** * Queue transactions setting up transfer parameters for the * currently selected MEM-AP. @@ -303,10 +354,6 @@ static int mem_ap_write(struct adiv5_ap *ap, const uint8_t *buffer, uint32_t siz if (ap->unaligned_access_bad && (address % size != 0)) return ERROR_TARGET_UNALIGNED_ACCESS; - retval = mem_ap_setup_tar(ap, address ^ addr_xor); - if (retval != ERROR_OK) - return retval; - while (nbytes > 0) { uint32_t this_size = size; @@ -322,6 +369,10 @@ static int mem_ap_write(struct adiv5_ap *ap, const uint8_t *buffer, uint32_t siz if (retval != ERROR_OK) break; + retval = mem_ap_setup_tar(ap, address ^ addr_xor); + if (retval != ERROR_OK) + return retval; + /* How many source bytes each transfer will consume, and their location in the DRW, * depends on the type of transfer and alignment. See ARM document IHI0031C. */ uint32_t outvalue = 0; @@ -361,12 +412,7 @@ static int mem_ap_write(struct adiv5_ap *ap, const uint8_t *buffer, uint32_t siz if (retval != ERROR_OK) break; - /* Rewrite TAR if it wrapped or we're xoring addresses */ - if (addrinc && (addr_xor || (address % ap->tar_autoincr_block < size && nbytes > 0))) { - retval = mem_ap_setup_tar(ap, address ^ addr_xor); - if (retval != ERROR_OK) - break; - } + mem_ap_update_tar_cache(ap); } /* REVISIT: Might want to have a queued version of this function that does not run. */ @@ -375,8 +421,7 @@ static int mem_ap_write(struct adiv5_ap *ap, const uint8_t *buffer, uint32_t siz if (retval != ERROR_OK) { uint32_t tar; - if (dap_queue_ap_read(ap, MEM_AP_REG_TAR, &tar) == ERROR_OK - && dap_run(dap) == ERROR_OK) + if (mem_ap_read_tar(ap, &tar) == ERROR_OK) LOG_ERROR("Failed to write memory at 0x%08"PRIx32, tar); else LOG_ERROR("Failed to write memory and, additionally, failed to find out where"); @@ -436,12 +481,6 @@ static int mem_ap_read(struct adiv5_ap *ap, uint8_t *buffer, uint32_t size, uint return ERROR_FAIL; } - retval = mem_ap_setup_tar(ap, address); - if (retval != ERROR_OK) { - free(read_buf); - return retval; - } - /* Queue up all reads. Each read will store the entire DRW word in the read buffer. How many * useful bytes it contains, and their location in the word, depends on the type of transfer * and alignment. */ @@ -459,6 +498,10 @@ static int mem_ap_read(struct adiv5_ap *ap, uint8_t *buffer, uint32_t size, uint if (retval != ERROR_OK) break; + retval = mem_ap_setup_tar(ap, address); + if (retval != ERROR_OK) + break; + retval = dap_queue_ap_read(ap, MEM_AP_REG_DRW, read_ptr++); if (retval != ERROR_OK) break; @@ -466,12 +509,7 @@ static int mem_ap_read(struct adiv5_ap *ap, uint8_t *buffer, uint32_t size, uint nbytes -= this_size; address += this_size; - /* Rewrite TAR if it wrapped */ - if (addrinc && address % ap->tar_autoincr_block < size && nbytes > 0) { - retval = mem_ap_setup_tar(ap, address); - if (retval != ERROR_OK) - break; - } + mem_ap_update_tar_cache(ap); } if (retval == ERROR_OK) @@ -486,8 +524,8 @@ static int mem_ap_read(struct adiv5_ap *ap, uint8_t *buffer, uint32_t size, uint * at least give the caller what we have. */ if (retval != ERROR_OK) { uint32_t tar; - if (dap_queue_ap_read(ap, MEM_AP_REG_TAR, &tar) == ERROR_OK - && dap_run(dap) == ERROR_OK) { + if (mem_ap_read_tar(ap, &tar) == ERROR_OK) { + /* TAR is incremented after failed transfer on some devices (eg Cortex-M4) */ LOG_ERROR("Failed to read memory at 0x%08"PRIx32, tar); if (nbytes > tar - address) nbytes = tar - address; @@ -596,6 +634,22 @@ struct adiv5_dap *dap_init(void) return dap; } +/** + * Invalidate cached DP select and cached TAR and CSW of all APs + */ +void dap_invalidate_cache(struct adiv5_dap *dap) +{ + dap->select = DP_SELECT_INVALID; + dap->last_read = NULL; + + int i; + for (i = 0; i <= 255; i++) { + /* force csw and tar write on the next mem-ap access */ + dap->ap[i].tar_valid = false; + dap->ap[i].csw_value = 0; + } +} + /** * Initialize a DAP. This sets up the power domains, prepares the DP * for further use and activates overrun checking. @@ -615,8 +669,7 @@ int dap_dp_init(struct adiv5_dap *dap) if (!dap->ops) dap->ops = &jtag_dp_ops; - dap->select = DP_SELECT_INVALID; - dap->last_read = NULL; + dap_invalidate_cache(dap); for (size_t i = 0; i < 30; i++) { /* DP initialization */ @@ -688,6 +741,8 @@ int mem_ap_init(struct adiv5_ap *ap) int retval; struct adiv5_dap *dap = ap->dap; + ap->tar_valid = false; + ap->csw_value = 0; /* force csw and tar write */ retval = mem_ap_setup_transfer(ap, CSW_8BIT | CSW_ADDRINC_PACKED, 0); if (retval != ERROR_OK) return retval; diff --git a/src/target/arm_adi_v5.h b/src/target/arm_adi_v5.h index bf9cb5cce..657427b25 100644 --- a/src/target/arm_adi_v5.h +++ b/src/target/arm_adi_v5.h @@ -102,6 +102,7 @@ #define AP_REG_IDR 0xFC /* RO: Identification Register */ /* Fields of the MEM-AP's CSW register */ +#define CSW_SIZE_MASK 7 #define CSW_8BIT 0 #define CSW_16BIT 1 #define CSW_32BIT 2 @@ -180,6 +181,9 @@ struct adiv5_ap { /* true if unaligned memory access is not supported by the MEM-AP */ bool unaligned_access_bad; + + /* true if tar_value is in sync with TAR register */ + bool tar_valid; }; @@ -476,6 +480,9 @@ struct adiv5_dap *dap_init(void); int dap_dp_init(struct adiv5_dap *dap); int mem_ap_init(struct adiv5_ap *ap); +/* Invalidate cached DP select and cached TAR and CSW of all APs */ +void dap_invalidate_cache(struct adiv5_dap *dap); + /* Probe the AP for ROM Table location */ int dap_get_debugbase(struct adiv5_ap *ap, uint32_t *dbgbase, uint32_t *apid);