breakpoints: use 64-bit type for watchpoint mask and value

This patch changes data types of watchpoint value and mask to allow for
64-bit values match that some architectures (like RISCV) allow.

In addition this patch fixes the behavior of watchpoint command to
zero-out mask if only data value is provided.

Change-Id: I3c7ec1630f03ea9534ec34c0ebe99e08ea56e7f0
Signed-off-by: Parshintsev Anatoly <anatoly.parshintsev@syntacore.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7840
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-by: Marek Vrbka <marek.vrbka@codasip.com>
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 702dbef49922087b6d4fb18851ffb77697e3696d..4a4ea53dc325162c0de89f48b8a55454f355c18a 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -1779,7 +1779,7 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
                case 4:
                {
                        if (packet[0] == 'Z') {
-                               retval = watchpoint_add(target, address, size, wp_type, 0, 0xffffffffu);
+                               retval = watchpoint_add(target, address, size, wp_type, 0, WATCHPOINT_IGNORE_DATA_VALUE_MASK);
                                if (retval == ERROR_NOT_IMPLEMENTED) {
                                        /* Send empty reply to report that watchpoints of this type are not supported */
                                        gdb_put_packet(connection, "", 0);

diff --git a/src/target/arm7_9_common.c b/src/target/arm7_9_common.c
index bbdbc4981cc5ddca6bf34c385a53d7206dcc92cd..ad814e05413423567948edc3be2d5469de0c0e44 100644 (file)
--- a/src/target/arm7_9_common.c
+++ b/src/target/arm7_9_common.c
@@ -451,6 +451,7 @@ static int arm7_9_set_watchpoint(struct target *target, struct watchpoint *watch
        struct arm7_9_common *arm7_9 = target_to_arm7_9(target);
        int rw_mask = 1;
        uint32_t mask;
+       const uint32_t wp_data_mask = watchpoint->mask;
 
        mask = watchpoint->length - 1;
 
@@ -469,8 +470,8 @@ static int arm7_9_set_watchpoint(struct target *target, struct watchpoint *watch
                        watchpoint->address);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W0_ADDR_MASK], mask);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W0_DATA_MASK],
-                       watchpoint->mask);
-               if (watchpoint->mask != 0xffffffffu)
+                       wp_data_mask);
+               if (wp_data_mask != (uint32_t)WATCHPOINT_IGNORE_DATA_VALUE_MASK)
                        embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W0_DATA_VALUE],
                                watchpoint->value);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W0_CONTROL_MASK],
@@ -488,8 +489,8 @@ static int arm7_9_set_watchpoint(struct target *target, struct watchpoint *watch
                        watchpoint->address);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W1_ADDR_MASK], mask);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W1_DATA_MASK],
-                       watchpoint->mask);
-               if (watchpoint->mask != 0xffffffffu)
+                       wp_data_mask);
+               if (wp_data_mask != (uint32_t)WATCHPOINT_IGNORE_DATA_VALUE_MASK)
                        embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W1_DATA_VALUE],
                                watchpoint->value);
                embeddedice_set_reg(&arm7_9->eice_cache->reg_list[EICE_W1_CONTROL_MASK],

diff --git a/src/target/arm_dpm.c b/src/target/arm_dpm.c
index fd6fb263fc005e5456211dec1f3a79a84d6de0e2..ab9b50e2302a89fce24aed831dcec48c687a20db 100644 (file)
--- a/src/target/arm_dpm.c
+++ b/src/target/arm_dpm.c
@@ -918,7 +918,7 @@ static int dpm_watchpoint_setup(struct arm_dpm *dpm, unsigned index_t,
        uint32_t control;
 
        /* this hardware doesn't support data value matching or masking */
-       if (wp->value || wp->mask != ~(uint32_t)0) {
+       if (wp->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK) {
                LOG_DEBUG("watchpoint values and masking not supported");
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }

diff --git a/src/target/armv8_dpm.c b/src/target/armv8_dpm.c
index d1eefe5b32e25f1d46e4083b726e1ae68046efc1..9ba6b5453b4ab731099c06e9496f91fa2aaf8dcd 100644 (file)
--- a/src/target/armv8_dpm.c
+++ b/src/target/armv8_dpm.c
@@ -1210,7 +1210,7 @@ static int dpmv8_watchpoint_setup(struct arm_dpm *dpm, unsigned index_t,
        uint32_t control;
 
        /* this hardware doesn't support data value matching or masking */
-       if (wp->value || wp->mask != ~(uint32_t)0) {
+       if (wp->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK) {
                LOG_DEBUG("watchpoint values and masking not supported");
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }

diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c
index bbaff4e7565067c5136c9ed53637b396b48e3e11..4d268cebb3f460f3768eb1c256207bb35c4cef9a 100644 (file)
--- a/src/target/breakpoints.c
+++ b/src/target/breakpoints.c
@@ -393,7 +393,7 @@ struct breakpoint *breakpoint_find(struct target *target, target_addr_t address)
 }
 
 static int watchpoint_add_internal(struct target *target, target_addr_t address,
-               uint32_t length, enum watchpoint_rw rw, uint32_t value, uint32_t mask)
+               uint32_t length, enum watchpoint_rw rw, uint64_t value, uint64_t mask)
 {
        struct watchpoint *watchpoint = target->watchpoints;
        struct watchpoint **watchpoint_p = &target->watchpoints;
@@ -460,7 +460,7 @@ bye:
 }
 
 int watchpoint_add(struct target *target, target_addr_t address,
-               uint32_t length, enum watchpoint_rw rw, uint32_t value, uint32_t mask)
+               uint32_t length, enum watchpoint_rw rw, uint64_t value, uint64_t mask)
 {
        if (target->smp) {
                struct target_list *head;

diff --git a/src/target/breakpoints.h b/src/target/breakpoints.h
index a9ae484357178f7ed8caca0107a94f05952471a0..d447515bff32bb6675dd1c24b56fcd58b905a5ca 100644 (file)
--- a/src/target/breakpoints.h
+++ b/src/target/breakpoints.h
@@ -36,11 +36,13 @@ struct breakpoint {
        int linked_brp;
 };
 
+#define WATCHPOINT_IGNORE_DATA_VALUE_MASK (~(uint64_t)0)
+
 struct watchpoint {
        target_addr_t address;
        uint32_t length;
-       uint32_t mask;
-       uint32_t value;
+       uint64_t mask;
+       uint64_t value;
        enum watchpoint_rw rw;
        bool is_set;
        unsigned int number;
@@ -69,7 +71,7 @@ static inline void breakpoint_hw_set(struct breakpoint *breakpoint, unsigned int
 void watchpoint_clear_target(struct target *target);
 int watchpoint_add(struct target *target,
                target_addr_t address, uint32_t length,
-               enum watchpoint_rw rw, uint32_t value, uint32_t mask);
+               enum watchpoint_rw rw, uint64_t value, uint64_t mask);
 void watchpoint_remove(struct target *target, target_addr_t address);
 
 /* report type and address of just hit watchpoint */

diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c
index 9541caa7921122f7a4bc152660052646daaff472..987dc9b245164dd84c5934b7b42b4b868e43854e 100644 (file)
--- a/src/target/cortex_m.c
+++ b/src/target/cortex_m.c
@@ -2046,8 +2046,14 @@ int cortex_m_add_watchpoint(struct target *target, struct watchpoint *watchpoint
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }
 
-       /* hardware doesn't support data value masking */
-       if (watchpoint->mask != ~(uint32_t)0) {
+       /* REVISIT This DWT may well be able to watch for specific data
+        * values.  Requires comparator #1 to set DATAVMATCH and match
+        * the data, and another comparator (DATAVADDR0) matching addr.
+        *
+        * NOTE: hardware doesn't support data value masking, so we'll need
+        * to check that mask is zero
+        */
+       if (watchpoint->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK) {
                LOG_TARGET_DEBUG(target, "watchpoint value masks not supported");
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }
@@ -2068,18 +2074,6 @@ int cortex_m_add_watchpoint(struct target *target, struct watchpoint *watchpoint
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }
 
-       /* Caller doesn't seem to be able to describe watching for data
-        * values of zero; that flags "no value".
-        *
-        * REVISIT This DWT may well be able to watch for specific data
-        * values.  Requires comparator #1 to set DATAVMATCH and match
-        * the data, and another comparator (DATAVADDR0) matching addr.
-        */
-       if (watchpoint->value) {
-               LOG_TARGET_DEBUG(target, "data value watchpoint not YET supported");
-               return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
-       }
-
        cortex_m->dwt_comp_available--;
        LOG_TARGET_DEBUG(target, "dwt_comp_available: %d", cortex_m->dwt_comp_available);
 

diff --git a/src/target/target.c b/src/target/target.c
index 89eaa23d846a2a2103d069399c93e501fd9d870b..96f4ae7d3ee82d959191e36e6b11f839acec7412 100644 (file)
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -4061,8 +4061,8 @@ COMMAND_HANDLER(handle_wp_command)
                while (watchpoint) {
                        command_print(CMD, "address: " TARGET_ADDR_FMT
                                        ", len: 0x%8.8" PRIx32
-                                       ", r/w/a: %i, value: 0x%8.8" PRIx32
-                                       ", mask: 0x%8.8" PRIx32,
+                                       ", r/w/a: %i, value: 0x%8.8" PRIx64
+                                       ", mask: 0x%8.8" PRIx64,
                                        watchpoint->address,
                                        watchpoint->length,
                                        (int)watchpoint->rw,
@@ -4076,15 +4076,20 @@ COMMAND_HANDLER(handle_wp_command)
        enum watchpoint_rw type = WPT_ACCESS;
        target_addr_t addr = 0;
        uint32_t length = 0;
-       uint32_t data_value = 0x0;
-       uint32_t data_mask = 0xffffffff;
+       uint64_t data_value = 0x0;
+       uint64_t data_mask = WATCHPOINT_IGNORE_DATA_VALUE_MASK;
+       bool mask_specified = false;
 
        switch (CMD_ARGC) {
        case 5:
-               COMMAND_PARSE_NUMBER(u32, CMD_ARGV[4], data_mask);
+               COMMAND_PARSE_NUMBER(u64, CMD_ARGV[4], data_mask);
+               mask_specified = true;
                /* fall through */
        case 4:
-               COMMAND_PARSE_NUMBER(u32, CMD_ARGV[3], data_value);
+               COMMAND_PARSE_NUMBER(u64, CMD_ARGV[3], data_value);
+               // if user specified only data value without mask - the mask should be 0
+               if (!mask_specified)
+                       data_mask = 0;
                /* fall through */
        case 3:
                switch (CMD_ARGV[2][0]) {

diff --git a/src/target/xscale.c b/src/target/xscale.c
index 03aa5166bab5c6f327897a8373d6100b984456d1..fbf43516d02fcbb446287ee7939c51c18974630e 100644 (file)
--- a/src/target/xscale.c
+++ b/src/target/xscale.c
@@ -2296,7 +2296,7 @@ static int xscale_add_watchpoint(struct target *target,
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }
 
-       if (watchpoint->value)
+       if (watchpoint->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK)
                LOG_WARNING("xscale does not support value, mask arguments; ignoring");
 
        /* check that length is a power of two */

diff --git a/src/target/xtensa/xtensa.c b/src/target/xtensa/xtensa.c
index 431be894b91dbe43bef5f9c68d08866531dfa926..c575b534e04df75a642632ed24b23d38e5a0e50d 100644 (file)
--- a/src/target/xtensa/xtensa.c
+++ b/src/target/xtensa/xtensa.c
@@ -2570,7 +2570,7 @@ int xtensa_watchpoint_add(struct target *target, struct watchpoint *watchpoint)
                return ERROR_TARGET_NOT_HALTED;
        }
 
-       if (watchpoint->mask != ~(uint32_t)0) {
+       if (watchpoint->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK) {
                LOG_TARGET_ERROR(target, "watchpoint value masks not supported");
                return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
        }