gdb_server, rtos: Fine-grained RTOS register access

1. Add get_thread_reg() to rtos. It's used in rtos_get_gdb_reg() to read
the value of a single register, instead of reading all register values
by calling get_thread_reg_list().
2. Add set_reg() to rtos. gdb_server uses this to change a single
register value for a specific thread.
3. Add target_get_gdb_reg_list_noread() so it's possible for gdb to get
a list of registers without attempting to read their contents.

The clang static checker doesn't find any new problems with this change.

Change-Id: I77f792d1238cb015b91527ca8cb99593ccc8870e
Signed-off-by: Tim Newsome <tim@sifive.com>
Reviewed-on: http://openocd.zylin.com/5114
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>

diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 20e875d8b850098a7a64037438532b445a774300..002d7b543c1448e0db2b7c638a736d7131941262 100644 (file)
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -462,6 +462,7 @@ static int rtos_put_gdb_reg_list(struct connection *connection,
        return ERROR_OK;
 }
 
+/** Look through all registers to find this register. */
 int rtos_get_gdb_reg(struct connection *connection, int reg_num)
 {
        struct target *target = get_target_from_connection(connection);
@@ -473,19 +474,31 @@ int rtos_get_gdb_reg(struct connection *connection, int reg_num)
                struct rtos_reg *reg_list;
                int num_regs;
 
-               LOG_DEBUG("RTOS: getting register %d for thread 0x%" PRIx64
-                                 ", target->rtos->current_thread=0x%" PRIx64 "\r\n",
+               LOG_DEBUG("getting register %d for thread 0x%" PRIx64
+                                 ", target->rtos->current_thread=0x%" PRIx64,
                                                                                reg_num,
                                                                                current_threadid,
                                                                                target->rtos->current_thread);
 
-               int retval = target->rtos->type->get_thread_reg_list(target->rtos,
-                               current_threadid,
-                               &reg_list,
-                               &num_regs);
-               if (retval != ERROR_OK) {
-                       LOG_ERROR("RTOS: failed to get register list");
-                       return retval;
+               int retval;
+               if (target->rtos->type->get_thread_reg) {
+                       reg_list = calloc(1, sizeof(*reg_list));
+                       num_regs = 1;
+                       retval = target->rtos->type->get_thread_reg(target->rtos,
+                                       current_threadid, reg_num, &reg_list[0]);
+                       if (retval != ERROR_OK) {
+                               LOG_ERROR("RTOS: failed to get register %d", reg_num);
+                               return retval;
+                       }
+               } else {
+                       retval = target->rtos->type->get_thread_reg_list(target->rtos,
+                                       current_threadid,
+                                       &reg_list,
+                                       &num_regs);
+                       if (retval != ERROR_OK) {
+                               LOG_ERROR("RTOS: failed to get register list");
+                               return retval;
+                       }
                }
 
                for (int i = 0; i < num_regs; ++i) {
@@ -501,6 +514,7 @@ int rtos_get_gdb_reg(struct connection *connection, int reg_num)
        return ERROR_FAIL;
 }
 
+/** Return a list of general registers. */
 int rtos_get_gdb_reg_list(struct connection *connection)
 {
        struct target *target = get_target_from_connection(connection);
@@ -534,6 +548,20 @@ int rtos_get_gdb_reg_list(struct connection *connection)
        return ERROR_FAIL;
 }
 
+int rtos_set_reg(struct connection *connection, int reg_num,
+               uint8_t *reg_value)
+{
+       struct target *target = get_target_from_connection(connection);
+       int64_t current_threadid = target->rtos->current_threadid;
+       if ((target->rtos != NULL) &&
+                       (target->rtos->type->set_reg != NULL) &&
+                       (current_threadid != -1) &&
+                       (current_threadid != 0)) {
+               return target->rtos->type->set_reg(target->rtos, reg_num, reg_value);
+       }
+       return ERROR_FAIL;
+}
+
 int rtos_generic_stack_read(struct target *target,
        const struct rtos_register_stacking *stacking,
        int64_t stack_ptr,

diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index 93b1731aad1b0d793b14590d1aafe3ddedc912dc..a649e2449d956a775830889c8b8707ec23491e49 100644 (file)
--- a/src/rtos/rtos.h
+++ b/src/rtos/rtos.h
@@ -20,6 +20,7 @@
 #define OPENOCD_RTOS_RTOS_H
 
 #include "server/server.h"
+#include "target/target.h"
 #include <jim-nvp.h>
 
 typedef int64_t threadid_t;
@@ -49,7 +50,9 @@ struct rtos {
        symbol_table_elem_t *symbols;
        struct target *target;
        /*  add a context variable instead of global variable */
+       /* The thread currently selected by gdb. */
        int64_t current_threadid;
+       /* The currently selected thread according to the target. */
        threadid_t current_thread;
        struct thread_detail *thread_details;
        int thread_count;
@@ -70,11 +73,15 @@ struct rtos_type {
        int (*create)(struct target *target);
        int (*smp_init)(struct target *target);
        int (*update_threads)(struct rtos *rtos);
+       /** Return a list of general registers, with their values filled out. */
        int (*get_thread_reg_list)(struct rtos *rtos, int64_t thread_id,
                        struct rtos_reg **reg_list, int *num_regs);
+       int (*get_thread_reg)(struct rtos *rtos, int64_t thread_id,
+                       uint32_t reg_num, struct rtos_reg *reg);
        int (*get_symbol_list_to_lookup)(symbol_table_elem_t *symbol_list[]);
        int (*clean)(struct target *target);
        char * (*ps_command)(struct target *target);
+       int (*set_reg)(struct rtos *rtos, uint32_t reg_num, uint8_t *reg_value);
 };
 
 struct stack_register_offset {
@@ -104,6 +111,8 @@ struct rtos_register_stacking {
 #define GDB_THREAD_PACKET_NOT_CONSUMED (-40)
 
 int rtos_create(Jim_GetOptInfo *goi, struct target *target);
+int rtos_set_reg(struct connection *connection, int reg_num,
+               uint8_t *reg_value);
 int rtos_generic_stack_read(struct target *target,
                const struct rtos_register_stacking *stacking,
                int64_t stack_ptr,

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index ab3e6bef8d885079aa7c86543d3d7645595f2702..02020425e481e31560a0b0c98c0f7a97cf692a58 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -724,7 +724,7 @@ static int gdb_output(struct command_context *context, const char *line)
 static void gdb_signal_reply(struct target *target, struct connection *connection)
 {
        struct gdb_connection *gdb_connection = connection->priv;
-       char sig_reply[45];
+       char sig_reply[65];
        char stop_reason[20];
        char current_thread[25];
        int sig_reply_len;
@@ -735,17 +735,25 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio
        if (target->debug_reason == DBG_REASON_EXIT) {
                sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "W00");
        } else {
+               struct target *ct;
+               if (target->rtos != NULL) {
+                       target->rtos->current_threadid = target->rtos->current_thread;
+                       target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct);
+               } else {
+                       ct = target;
+               }
+
                if (gdb_connection->ctrl_c) {
                        signal_var = 0x2;
                } else
-                       signal_var = gdb_last_signal(target);
+                       signal_var = gdb_last_signal(ct);
 
                stop_reason[0] = '\0';
-               if (target->debug_reason == DBG_REASON_WATCHPOINT) {
+               if (ct->debug_reason == DBG_REASON_WATCHPOINT) {
                        enum watchpoint_rw hit_wp_type;
                        target_addr_t hit_wp_address;
 
-                       if (watchpoint_hit(target, &hit_wp_type, &hit_wp_address) == ERROR_OK) {
+                       if (watchpoint_hit(ct, &hit_wp_type, &hit_wp_address) == ERROR_OK) {
 
                                switch (hit_wp_type) {
                                        case WPT_WRITE:
@@ -767,15 +775,9 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio
                }
 
                current_thread[0] = '\0';
-               if (target->rtos != NULL) {
-                       struct target *ct;
-                       snprintf(current_thread, sizeof(current_thread), "thread:%016" PRIx64 ";",
+               if (target->rtos != NULL)
+                       snprintf(current_thread, sizeof(current_thread), "thread:%" PRIx64 ";",
                                        target->rtos->current_thread);
-                       target->rtos->current_threadid = target->rtos->current_thread;
-                       target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct);
-                       if (!gdb_connection->ctrl_c)
-                               signal_var = gdb_last_signal(ct);
-               }
 
                sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "T%2.2x%s%s",
                                signal_var, stop_reason, current_thread);
@@ -1300,7 +1302,7 @@ static int gdb_get_register_packet(struct connection *connection,
        if ((target->rtos != NULL) && (ERROR_OK == rtos_get_gdb_reg(connection, reg_num)))
                return ERROR_OK;
 
-       retval = target_get_gdb_reg_list(target, &reg_list, &reg_list_size,
+       retval = target_get_gdb_reg_list_noread(target, &reg_list, &reg_list_size,
                        REG_CLASS_ALL);
        if (retval != ERROR_OK)
                return gdb_error(connection, retval);
@@ -1336,37 +1338,49 @@ static int gdb_set_register_packet(struct connection *connection,
 {
        struct target *target = get_target_from_connection(connection);
        char *separator;
-       uint8_t *bin_buf;
        int reg_num = strtoul(packet + 1, &separator, 16);
        struct reg **reg_list;
        int reg_list_size;
        int retval;
 
+#ifdef _DEBUG_GDB_IO_
        LOG_DEBUG("-");
+#endif
 
-       retval = target_get_gdb_reg_list(target, &reg_list, &reg_list_size,
+       if (*separator != '=') {
+               LOG_ERROR("GDB 'set register packet', but no '=' following the register number");
+               return ERROR_SERVER_REMOTE_CLOSED;
+       }
+       size_t chars = strlen(separator + 1);
+       uint8_t *bin_buf = malloc(chars / 2);
+       gdb_target_to_reg(target, separator + 1, chars, bin_buf);
+
+       if ((target->rtos != NULL) &&
+                       (ERROR_OK == rtos_set_reg(connection, reg_num, bin_buf))) {
+               free(bin_buf);
+               gdb_put_packet(connection, "OK", 2);
+               return ERROR_OK;
+       }
+
+       retval = target_get_gdb_reg_list_noread(target, &reg_list, &reg_list_size,
                        REG_CLASS_ALL);
-       if (retval != ERROR_OK)
+       if (retval != ERROR_OK) {
+               free(bin_buf);
                return gdb_error(connection, retval);
+       }
 
        if (reg_list_size <= reg_num) {
                LOG_ERROR("gdb requested a non-existing register");
+               free(bin_buf);
+               free(reg_list);
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
-       if (*separator != '=') {
-               LOG_ERROR("GDB 'set register packet', but no '=' following the register number");
-               return ERROR_SERVER_REMOTE_CLOSED;
-       }
-
-       /* convert from GDB-string (target-endian) to hex-string (big-endian) */
-       bin_buf = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8));
-       int chars = (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2);
-
-       if ((unsigned int)chars != strlen(separator + 1)) {
-               LOG_ERROR("gdb sent %zu bits for a %d-bit register (%s)",
-                               strlen(separator + 1) * 4, chars * 4, reg_list[reg_num]->name);
+       if (chars != (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2)) {
+               LOG_ERROR("gdb sent %d bits for a %d-bit register (%s)",
+                               (int) chars * 4, reg_list[reg_num]->size, reg_list[reg_num]->name);
                free(bin_buf);
+               free(reg_list);
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
@@ -1634,7 +1648,7 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
        char *separator;
        int retval;
 
-       LOG_DEBUG("-");
+       LOG_DEBUG("[%s]", target_name(target));
 
        type = strtoul(packet + 1, &separator, 16);
 
@@ -2200,7 +2214,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
 
        arch_defined_types = calloc(1, sizeof(char *));
 
-       retval = target_get_gdb_reg_list(target, &reg_list,
+       retval = target_get_gdb_reg_list_noread(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
 
        if (retval != ERROR_OK) {
@@ -2388,7 +2402,7 @@ static int gdb_target_description_supported(struct target *target, int *supporte
 
        char const *architecture = target_get_gdb_arch(target);
 
-       retval = target_get_gdb_reg_list(target, &reg_list,
+       retval = target_get_gdb_reg_list_noread(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
        if (retval != ERROR_OK) {
                LOG_ERROR("get register list failed");
@@ -2783,13 +2797,11 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 
                                if (parse[0] == 'c') {
                                        parse += 1;
-                                       packet_size -= 1;
 
                                        /* check if thread-id follows */
                                        if (parse[0] == ':') {
                                                int64_t tid;
                                                parse += 1;
-                                               packet_size -= 1;
 
                                                tid = strtoll(parse, &endp, 16);
                                                if (tid == thread_id) {
@@ -2879,10 +2891,9 @@ static int gdb_v_packet(struct connection *connection,
                char const *packet, int packet_size)
 {
        struct gdb_connection *gdb_connection = connection->priv;
-       struct target *target;
        int result;
 
-       target = get_target_from_connection(connection);
+       struct target *target = get_target_from_connection(connection);
 
        if (strncmp(packet, "vCont", 5) == 0) {
                bool handled;

diff --git a/src/target/register.c b/src/target/register.c
index 5352d2f21d7786753c35b2cf1448a79a85cf4989..4ddda6e6b3c420978d7c58e5692e8d2e1b1fa9ff 100644 (file)
--- a/src/target/register.c
+++ b/src/target/register.c
@@ -36,6 +36,29 @@
  * may be separate registers associated with debug or trace modules.
  */
 
+struct reg *register_get_by_number(struct reg_cache *first,
+               uint32_t reg_num, bool search_all)
+{
+       unsigned i;
+       struct reg_cache *cache = first;
+
+       while (cache) {
+               for (i = 0; i < cache->num_regs; i++) {
+                       if (cache->reg_list[i].exist == false)
+                               continue;
+                       if (cache->reg_list[i].number == reg_num)
+                               return &(cache->reg_list[i]);
+               }
+
+               if (search_all)
+                       cache = cache->next;
+               else
+                       break;
+       }
+
+       return NULL;
+}
+
 struct reg *register_get_by_name(struct reg_cache *first,
                const char *name, bool search_all)
 {

diff --git a/src/target/register.h b/src/target/register.h
index 32c1f39aceadc50b55673de05137a994565a3182..7c53d6e16deb531c82879001f494f98df69a7da7 100644 (file)
--- a/src/target/register.h
+++ b/src/target/register.h
@@ -159,6 +159,8 @@ struct reg_arch_type {
        int (*set)(struct reg *reg, uint8_t *buf);
 };
 
+struct reg *register_get_by_number(struct reg_cache *first,
+               uint32_t reg_num, bool search_all);
 struct reg *register_get_by_name(struct reg_cache *first,
                const char *name, bool search_all);
 struct reg_cache **register_get_last_cache_p(struct reg_cache **first);

diff --git a/src/target/target.c b/src/target/target.c
index e6c434362a9eb6c2507451540e127ff51ab624a6..51fdff3425e0e630084690fefa1f234b9d3ccbc7 100644 (file)
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -1215,7 +1215,24 @@ int target_get_gdb_reg_list(struct target *target,
                struct reg **reg_list[], int *reg_list_size,
                enum target_register_class reg_class)
 {
-       return target->type->get_gdb_reg_list(target, reg_list, reg_list_size, reg_class);
+       int result = target->type->get_gdb_reg_list(target, reg_list,
+                       reg_list_size, reg_class);
+       if (result != ERROR_OK) {
+               *reg_list = NULL;
+               *reg_list_size = 0;
+       }
+       return result;
+}
+
+int target_get_gdb_reg_list_noread(struct target *target,
+               struct reg **reg_list[], int *reg_list_size,
+               enum target_register_class reg_class)
+{
+       if (target->type->get_gdb_reg_list_noread &&
+                       target->type->get_gdb_reg_list_noread(target, reg_list,
+                               reg_list_size, reg_class) == ERROR_OK)
+               return ERROR_OK;
+       return target_get_gdb_reg_list(target, reg_list, reg_list_size, reg_class);
 }
 
 bool target_supports_gdb_connection(struct target *target)
@@ -1587,8 +1604,9 @@ int target_call_event_callbacks(struct target *target, enum target_event event)
                target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT);
        }
 
-       LOG_DEBUG("target event %i (%s)", event,
-                       Jim_Nvp_value2name_simple(nvp_target_event, event)->name);
+       LOG_DEBUG("target event %i (%s) for core %s", event,
+                       Jim_Nvp_value2name_simple(nvp_target_event, event)->name,
+                       target_name(target));
 
        target_handle_event(target, event);
 

diff --git a/src/target/target.h b/src/target/target.h
index e944838acef9c7be4394722441d57b5762206db0..81fd9d23dc5c1e387c4876e75fbc4b9683c268a1 100644 (file)
--- a/src/target/target.h
+++ b/src/target/target.h
@@ -502,6 +502,16 @@ int target_get_gdb_reg_list(struct target *target,
                struct reg **reg_list[], int *reg_list_size,
                enum target_register_class reg_class);
 
+/**
+ * Obtain the registers for GDB, but don't read register values from the
+ * target.
+ *
+ * This routine is a wrapper for target->type->get_gdb_reg_list_noread.
+ */
+int target_get_gdb_reg_list_noread(struct target *target,
+               struct reg **reg_list[], int *reg_list_size,
+               enum target_register_class reg_class);
+
 /**
  * Check if @a target allows GDB connections.
  *

diff --git a/src/target/target_type.h b/src/target/target_type.h
index 95745c9ebd3cacfe660662504220f3c55a606e24..4bdea721e853d27b746a590829686f2bda7e9ab6 100644 (file)
--- a/src/target/target_type.h
+++ b/src/target/target_type.h
@@ -111,6 +111,13 @@ struct target_type {
        int (*get_gdb_reg_list)(struct target *target, struct reg **reg_list[],
                        int *reg_list_size, enum target_register_class reg_class);
 
+       /**
+        * Same as get_gdb_reg_list, but doesn't read the register values.
+        * */
+       int (*get_gdb_reg_list_noread)(struct target *target,
+                       struct reg **reg_list[], int *reg_list_size,
+                       enum target_register_class reg_class);
+
        /* target memory access
        * size: 1 = byte (8bit), 2 = half-word (16bit), 4 = word (32bit)
        * count: number of items of <size>