Implementations need to access the register struct they modify;
make it easier and less error-prone to identify the instance.
(This removes over 10% of the ARMV4_5_CORE_REG_MODE nastiness...)

Plus some minor fixes noted when making these updates:  ARM7/ARM9
accessor methods should be static, don't leave CPSR wrongly marked
"dirty", note significant XScale omissions in register handling.

Rename "struct armv4_5_core_reg" as "struct arm_reg"; it's used
for more than those older architecture generations.
---
 src/target/arm7_9_common.c |   42 ++++++++++++++++-----------------------
 src/target/arm7_9_common.h |    1 
 src/target/armv4_5.c       |   42 +++++++++++++++++++++++++++------------
 src/target/armv4_5.h       |    6 ++---
 src/target/cortex_a8.c     |   46 +++++++++++++++++++++----------------------
 src/target/xscale.c        |   10 +++++----
 6 files changed, 79 insertions(+), 68 deletions(-)

--- a/src/target/arm7_9_common.c
+++ b/src/target/arm7_9_common.c
@@ -1577,7 +1577,7 @@ int arm7_9_restore_context(struct target
        struct arm7_9_common *arm7_9 = target_to_arm7_9(target);
        struct armv4_5_common_s *armv4_5 = &arm7_9->armv4_5_common;
        struct reg *reg;
-       struct armv4_5_core_reg *reg_arch_info;
+       struct arm_reg *reg_arch_info;
        enum armv4_5_mode current_mode = armv4_5->core_mode;
        int i, j;
        int dirty;
@@ -2084,25 +2084,24 @@ int arm7_9_step(struct target *target, i
        return err;
 }
 
-int arm7_9_read_core_reg(struct target *target, int num, enum armv4_5_mode 
mode)
+static int arm7_9_read_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode)
 {
        uint32_t* reg_p[16];
        uint32_t value;
        int retval;
+       struct arm_reg *areg = r->arch_info;
        struct arm7_9_common *arm7_9 = target_to_arm7_9(target);
        struct armv4_5_common_s *armv4_5 = &arm7_9->armv4_5_common;
 
        if (!is_arm_mode(armv4_5->core_mode))
                return ERROR_FAIL;
-
-       enum armv4_5_mode reg_mode = ((struct 
armv4_5_core_reg*)ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, 
num).arch_info)->mode;
-
        if ((num < 0) || (num > 16))
                return ERROR_INVALID_ARGUMENTS;
 
        if ((mode != ARMV4_5_MODE_ANY)
                        && (mode != armv4_5->core_mode)
-                       && (reg_mode != ARMV4_5_MODE_ANY))
+                       && (areg->mode != ARMV4_5_MODE_ANY))
        {
                uint32_t tmp_cpsr;
 
@@ -2125,10 +2124,7 @@ int arm7_9_read_core_reg(struct target *
                /* read a program status register
                 * if the register mode is MODE_ANY, we read the cpsr, 
otherwise a spsr
                 */
-               struct armv4_5_core_reg *arch_info = 
ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).arch_info;
-               int spsr = (arch_info->mode == ARMV4_5_MODE_ANY) ? 0 : 1;
-
-               arm7_9->read_xpsr(target, &value, spsr);
+               arm7_9->read_xpsr(target, &value, areg->mode != 
ARMV4_5_MODE_ANY);
        }
 
        if ((retval = jtag_execute_queue()) != ERROR_OK)
@@ -2136,13 +2132,13 @@ int arm7_9_read_core_reg(struct target *
                return retval;
        }
 
-       ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).valid = 1;
-       ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).dirty = 0;
-       buf_set_u32(ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, 
num).value, 0, 32, value);
+       r->valid = 1;
+       r->dirty = 0;
+       buf_set_u32(r->value, 0, 32, value);
 
        if ((mode != ARMV4_5_MODE_ANY)
                        && (mode != armv4_5->core_mode)
-                       && (reg_mode != ARMV4_5_MODE_ANY))      {
+                       && (areg->mode != ARMV4_5_MODE_ANY))    {
                /* restore processor mode (mask T bit) */
                arm7_9->write_xpsr_im8(target, 
buf_get_u32(armv4_5->core_cache->reg_list[ARMV4_5_CPSR].value, 0, 8) & ~0x20, 
0, 0);
        }
@@ -2150,23 +2146,22 @@ int arm7_9_read_core_reg(struct target *
        return ERROR_OK;
 }
 
-int arm7_9_write_core_reg(struct target *target, int num, enum armv4_5_mode 
mode, uint32_t value)
+static int arm7_9_write_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode, uint32_t value)
 {
        uint32_t reg[16];
+       struct arm_reg *areg = r->arch_info;
        struct arm7_9_common *arm7_9 = target_to_arm7_9(target);
        struct armv4_5_common_s *armv4_5 = &arm7_9->armv4_5_common;
 
        if (!is_arm_mode(armv4_5->core_mode))
                return ERROR_FAIL;
-
-       enum armv4_5_mode reg_mode = ((struct 
armv4_5_core_reg*)ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, 
num).arch_info)->mode;
-
        if ((num < 0) || (num > 16))
                return ERROR_INVALID_ARGUMENTS;
 
        if ((mode != ARMV4_5_MODE_ANY)
                        && (mode != armv4_5->core_mode)
-                       && (reg_mode != ARMV4_5_MODE_ANY))      {
+                       && (areg->mode != ARMV4_5_MODE_ANY))    {
                uint32_t tmp_cpsr;
 
                /* change processor mode (mask T bit) */
@@ -2188,8 +2183,7 @@ int arm7_9_write_core_reg(struct target 
                /* write a program status register
                * if the register mode is MODE_ANY, we write the cpsr, 
otherwise a spsr
                */
-               struct armv4_5_core_reg *arch_info = 
ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).arch_info;
-               int spsr = (arch_info->mode == ARMV4_5_MODE_ANY) ? 0 : 1;
+               int spsr = (areg->mode != ARMV4_5_MODE_ANY);
 
                /* if we're writing the CPSR, mask the T bit */
                if (!spsr)
@@ -2198,12 +2192,12 @@ int arm7_9_write_core_reg(struct target 
                arm7_9->write_xpsr(target, value, spsr);
        }
 
-       ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).valid = 1;
-       ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, mode, num).dirty = 0;
+       r->valid = 1;
+       r->dirty = 0;
 
        if ((mode != ARMV4_5_MODE_ANY)
                        && (mode != armv4_5->core_mode)
-                       && (reg_mode != ARMV4_5_MODE_ANY))      {
+                       && (areg->mode != ARMV4_5_MODE_ANY))    {
                /* restore processor mode (mask T bit) */
                arm7_9->write_xpsr_im8(target, 
buf_get_u32(armv4_5->core_cache->reg_list[ARMV4_5_CPSR].value, 0, 8) & ~0x20, 
0, 0);
        }
--- a/src/target/arm7_9_common.h
+++ b/src/target/arm7_9_common.h
@@ -139,7 +139,6 @@ int arm7_9_full_context(struct target *t
 int arm7_9_restore_context(struct target *target);
 int arm7_9_resume(struct target *target, int current, uint32_t address, int 
handle_breakpoints, int debug_execution);
 int arm7_9_step(struct target *target, int current, uint32_t address, int 
handle_breakpoints);
-int arm7_9_read_core_reg(struct target *target, int num, enum armv4_5_mode 
mode);
 int arm7_9_read_memory(struct target *target, uint32_t address, uint32_t size, 
uint32_t count, uint8_t *buffer);
 int arm7_9_write_memory(struct target *target, uint32_t address, uint32_t 
size, uint32_t count, uint8_t *buffer);
 int arm7_9_bulk_write_memory(struct target *target, uint32_t address, uint32_t 
count, uint8_t *buffer);
--- a/src/target/armv4_5.c
+++ b/src/target/armv4_5.c
@@ -363,7 +363,7 @@ static void arm_gdb_dummy_init(void)
 static int armv4_5_get_core_reg(struct reg *reg)
 {
        int retval;
-       struct armv4_5_core_reg *armv4_5 = reg->arch_info;
+       struct arm_reg *armv4_5 = reg->arch_info;
        struct target *target = armv4_5->target;
 
        if (target->state != TARGET_HALTED)
@@ -372,7 +372,7 @@ static int armv4_5_get_core_reg(struct r
                return ERROR_TARGET_NOT_HALTED;
        }
 
-       retval = armv4_5->armv4_5_common->read_core_reg(target, armv4_5->num, 
armv4_5->mode);
+       retval = armv4_5->armv4_5_common->read_core_reg(target, reg, 
armv4_5->num, armv4_5->mode);
        if (retval == ERROR_OK)
                reg->valid = 1;
 
@@ -381,7 +381,7 @@ static int armv4_5_get_core_reg(struct r
 
 static int armv4_5_set_core_reg(struct reg *reg, uint8_t *buf)
 {
-       struct armv4_5_core_reg *armv4_5 = reg->arch_info;
+       struct arm_reg *armv4_5 = reg->arch_info;
        struct target *target = armv4_5->target;
        struct armv4_5_common_s *armv4_5_target = target_to_armv4_5(target);
        uint32_t value = buf_get_u32(buf, 0, 32);
@@ -392,8 +392,16 @@ static int armv4_5_set_core_reg(struct r
                return ERROR_TARGET_NOT_HALTED;
        }
 
+       /* Except for CPSR, the "reg" command exposes a writeback model
+        * for the register cache.
+        */
+       buf_set_u32(reg->value, 0, 32, value);
+       reg->dirty = 1;
+       reg->valid = 1;
+
        if (reg == &armv4_5_target->core_cache->reg_list[ARMV4_5_CPSR])
        {
+               /* FIXME handle J bit too; mostly for ThumbEE, also Jazelle */
                if (value & 0x20)
                {
                        /* T bit should be set */
@@ -415,19 +423,23 @@ static int armv4_5_set_core_reg(struct r
                        }
                }
 
+               /* REVISIT Why only update core for mode change, not also
+                * for state changes?  Possibly older cores need to stay
+                * in ARM mode during halt mode debug, not execute Thumb;
+                * v6/v7a/v7r seem to do that automatically...
+                */
+
                if (armv4_5_target->core_mode != (enum armv4_5_mode)(value & 
0x1f))
                {
                        LOG_DEBUG("changing ARM core mode to '%s'",
                                        arm_mode_name(value & 0x1f));
                        armv4_5_target->core_mode = value & 0x1f;
-                       armv4_5_target->write_core_reg(target, 16, 
ARMV4_5_MODE_ANY, value);
+                       armv4_5_target->write_core_reg(target, reg,
+                                       16, ARMV4_5_MODE_ANY, value);
+                       reg->dirty = 0;
                }
        }
 
-       buf_set_u32(reg->value, 0, 32, value);
-       reg->dirty = 1;
-       reg->valid = 1;
-
        return ERROR_OK;
 }
 
@@ -441,8 +453,7 @@ struct reg_cache* armv4_5_build_reg_cach
        int num_regs = ARRAY_SIZE(arm_core_regs);
        struct reg_cache *cache = malloc(sizeof(struct reg_cache));
        struct reg *reg_list = calloc(num_regs, sizeof(struct reg));
-       struct armv4_5_core_reg *arch_info = calloc(num_regs,
-                                       sizeof(struct armv4_5_core_reg));
+       struct arm_reg *arch_info = calloc(num_regs, sizeof(struct arm_reg));
        int i;
 
        if (!cache || !reg_list || !arch_info) {
@@ -811,9 +822,14 @@ int armv4_5_run_algorithm_inner(struct t
 
        for (i = 0; i <= 16; i++)
        {
-               if (!ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, 
armv4_5_algorithm_info->core_mode, i).valid)
-                       armv4_5->read_core_reg(target, i, 
armv4_5_algorithm_info->core_mode);
-               context[i] = 
buf_get_u32(ARMV4_5_CORE_REG_MODE(armv4_5->core_cache, 
armv4_5_algorithm_info->core_mode, i).value, 0, 32);
+               struct reg *r;
+
+               r = &ARMV4_5_CORE_REG_MODE(armv4_5->core_cache,
+                               armv4_5_algorithm_info->core_mode, i);
+               if (!r->valid)
+                       armv4_5->read_core_reg(target, r, i,
+                                       armv4_5_algorithm_info->core_mode);
+               context[i] = buf_get_u32(r->value, 0, 32);
        }
        cpsr = buf_get_u32(armv4_5->core_cache->reg_list[ARMV4_5_CPSR].value, 
0, 32);
 
--- a/src/target/armv4_5.h
+++ b/src/target/armv4_5.h
@@ -109,9 +109,9 @@ struct arm
        struct etm_context *etm;
 
        int (*full_context)(struct target *target);
-       int (*read_core_reg)(struct target *target,
+       int (*read_core_reg)(struct target *target, struct reg *reg,
                        int num, enum armv4_5_mode mode);
-       int (*write_core_reg)(struct target *target,
+       int (*write_core_reg)(struct target *target, struct reg *reg,
                        int num, enum armv4_5_mode mode, uint32_t value);
        void *arch_info;
 };
@@ -137,7 +137,7 @@ struct armv4_5_algorithm
        enum armv4_5_state core_state;
 };
 
-struct armv4_5_core_reg
+struct arm_reg
 {
        int num;
        enum armv4_5_mode mode;
--- a/src/target/cortex_a8.c
+++ b/src/target/cortex_a8.c
@@ -877,7 +877,7 @@ static int cortex_a8_restore_context(str
 
                /* write dirty non-{R0,CPSR} registers sharing the same mode */
                for (i = max - 1, r = cache->reg_list + 1; i > 0; i--, r++) {
-                       struct armv4_5_core_reg *reg;
+                       struct arm_reg *reg;
 
                        if (!r->dirty || i == ARMV4_5_CPSR)
                                continue;
@@ -1018,16 +1018,17 @@ static int cortex_a8_store_core_reg_u32(
 #endif
 
 
-static int cortex_a8_write_core_reg(struct target *target, int num,
-               enum armv4_5_mode mode, uint32_t value);
+static int cortex_a8_write_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode, uint32_t value);
 
-static int cortex_a8_read_core_reg(struct target *target, int num,
-               enum armv4_5_mode mode)
+static int cortex_a8_read_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode)
 {
        uint32_t value;
        int retval;
        struct armv4_5_common_s *armv4_5 = target_to_armv4_5(target);
        struct reg_cache *cache = armv4_5->core_cache;
+       struct reg *cpsr_r = NULL;
        uint32_t cpsr = 0;
        unsigned cookie = num;
 
@@ -1042,10 +1043,10 @@ static int cortex_a8_read_core_reg(struc
                        mode = ARMV4_5_MODE_ANY;
 
                if (mode != ARMV4_5_MODE_ANY) {
-                       cpsr = buf_get_u32(cache ->reg_list[ARMV4_5_CPSR]
-                                       .value, 0, 32);
-                       cortex_a8_write_core_reg(target, 16,
-                                       ARMV4_5_MODE_ANY, mode);
+                       cpsr_r = cache->reg_list + ARMV4_5_CPSR;
+                       cpsr = buf_get_u32(cpsr_r->value, 0, 32);
+                       cortex_a8_write_core_reg(target, cpsr_r,
+                                       16, ARMV4_5_MODE_ANY, mode);
                }
        }
 
@@ -1066,24 +1067,24 @@ static int cortex_a8_read_core_reg(struc
        cortex_a8_dap_read_coreregister_u32(target, &value, cookie);
        retval = jtag_execute_queue();
        if (retval == ERROR_OK) {
-               struct reg *r = &ARMV4_5_CORE_REG_MODE(cache, mode, num);
-
                r->valid = 1;
                r->dirty = 0;
                buf_set_u32(r->value, 0, 32, value);
        }
 
-       if (cpsr)
-               cortex_a8_write_core_reg(target, 16, ARMV4_5_MODE_ANY, cpsr);
+       if (cpsr_r)
+               cortex_a8_write_core_reg(target, cpsr_r,
+                               16, ARMV4_5_MODE_ANY, cpsr);
        return retval;
 }
 
-static int cortex_a8_write_core_reg(struct target *target, int num,
-               enum armv4_5_mode mode, uint32_t value)
+static int cortex_a8_write_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode, uint32_t value)
 {
        int retval;
        struct armv4_5_common_s *armv4_5 = target_to_armv4_5(target);
        struct reg_cache *cache = armv4_5->core_cache;
+       struct reg *cpsr_r = NULL;
        uint32_t cpsr = 0;
        unsigned cookie = num;
 
@@ -1098,10 +1099,10 @@ static int cortex_a8_write_core_reg(stru
                        mode = ARMV4_5_MODE_ANY;
 
                if (mode != ARMV4_5_MODE_ANY) {
-                       cpsr = buf_get_u32(cache ->reg_list[ARMV4_5_CPSR]
-                                       .value, 0, 32);
-                       cortex_a8_write_core_reg(target, 16,
-                                       ARMV4_5_MODE_ANY, mode);
+                       cpsr_r = cache->reg_list + ARMV4_5_CPSR;
+                       cpsr = buf_get_u32(cpsr_r->value, 0, 32);
+                       cortex_a8_write_core_reg(target, cpsr_r,
+                                       16, ARMV4_5_MODE_ANY, mode);
                }
        }
 
@@ -1122,15 +1123,14 @@ static int cortex_a8_write_core_reg(stru
 
        cortex_a8_dap_write_coreregister_u32(target, value, cookie);
        if ((retval = jtag_execute_queue()) == ERROR_OK) {
-               struct reg *r = &ARMV4_5_CORE_REG_MODE(cache, mode, num);
-
                buf_set_u32(r->value, 0, 32, value);
                r->valid = 1;
                r->dirty = 0;
        }
 
-       if (cpsr)
-               cortex_a8_write_core_reg(target, 16, ARMV4_5_MODE_ANY, cpsr);
+       if (cpsr_r)
+               cortex_a8_write_core_reg(target, cpsr_r,
+                               16, ARMV4_5_MODE_ANY, cpsr);
        return retval;
 }
 
--- a/src/target/xscale.c
+++ b/src/target/xscale.c
@@ -1646,16 +1646,18 @@ static int xscale_deassert_reset(struct 
        return ERROR_OK;
 }
 
-static int xscale_read_core_reg(struct target *target, int num,
-               enum armv4_5_mode mode)
+static int xscale_read_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode)
 {
+       /** \todo add debug handler support for core register reads */
        LOG_ERROR("not implemented");
        return ERROR_OK;
 }
 
-static int xscale_write_core_reg(struct target *target, int num,
-               enum armv4_5_mode mode, uint32_t value)
+static int xscale_write_core_reg(struct target *target, struct reg *r,
+               int num, enum armv4_5_mode mode, uint32_t value)
 {
+       /** \todo add debug handler support for core register writes */
        LOG_ERROR("not implemented");
        return ERROR_OK;
 }
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to