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 <vanekt@fbl.cz>
Reviewed-on: http://openocd.zylin.com/4162
Tested-by: jenkins
Reviewed-by: Christopher Head <chead@zaber.com>
Reviewed-by: Andreas Fritiofson <andreas.fritiofson@gmail.com>

diff --git a/src/target/adi_v5_swd.c b/src/target/adi_v5_swd.c
index a6aada32611ba26884ebf4e739f113e26ead3e3c..c503f090b8dd4e0e105a725658eec6297c301941 100644 (file)
--- 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 2006290237eda467361c12a66bc1253a65135e94..a0d0f4ef042f7ed6d3297ee4405b6605762a76ef 100644 (file)
--- 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 bf9cb5ccee51a208fdda3ce8f9a4ef8fc77209fa..657427b25f368d5b61eea8dcadd98a23ed4124ef 100644 (file)
--- 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);