From 69c751956293e822faa6cf844f2864d81c36a578 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Fri, 20 Nov 2009 16:27:24 -0800 Subject: [PATCH] ARM: pass 'struct reg *' to register r/w routines 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; and have armv4_5_build_reg_cache() record its result. Rename "struct armv4_5_core_reg" as "struct arm_reg"; it's used for more than those older architecture generations. Signed-off-by: David Brownell --- src/target/arm7_9_common.c | 42 +++++++++++++++------------------- src/target/arm7_9_common.h | 1 - src/target/arm7tdmi.c | 1 - src/target/arm9tdmi.c | 1 - src/target/armv4_5.c | 47 ++++++++++++++++++++++++++------------ src/target/armv4_5.h | 6 ++--- src/target/cortex_a8.c | 47 +++++++++++++++++++------------------- src/target/xscale.c | 11 +++++---- 8 files changed, 83 insertions(+), 73 deletions(-) diff --git a/src/target/arm7_9_common.c b/src/target/arm7_9_common.c index 7a2b5429..3a327646 100644 --- 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 *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, int current, uint32_t address, int handle 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 *target, int num, enum armv4_5_mode mode) /* 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 *target, int num, enum armv4_5_mode mode) 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 *target, int num, enum armv4_5_mode mode) 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 *target, int num, enum armv4_5_mode mode /* 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 *target, int num, enum armv4_5_mode mode 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); } diff --git a/src/target/arm7_9_common.h b/src/target/arm7_9_common.h index 266bf800..2f7132a1 100644 --- 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 *target); 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); diff --git a/src/target/arm7tdmi.c b/src/target/arm7tdmi.c index 3bd52364..e7ea768f 100644 --- a/src/target/arm7tdmi.c +++ b/src/target/arm7tdmi.c @@ -644,7 +644,6 @@ static void arm7tdmi_build_reg_cache(struct target *target) struct armv4_5_common_s *armv4_5 = target_to_armv4_5(target); (*cache_p) = armv4_5_build_reg_cache(target, armv4_5); - armv4_5->core_cache = (*cache_p); } int arm7tdmi_init_target(struct command_context *cmd_ctx, struct target *target) diff --git a/src/target/arm9tdmi.c b/src/target/arm9tdmi.c index a69e49e7..38b2284b 100644 --- a/src/target/arm9tdmi.c +++ b/src/target/arm9tdmi.c @@ -754,7 +754,6 @@ static void arm9tdmi_build_reg_cache(struct target *target) struct armv4_5_common_s *armv4_5 = target_to_armv4_5(target); (*cache_p) = armv4_5_build_reg_cache(target, armv4_5); - armv4_5->core_cache = (*cache_p); } int arm9tdmi_init_target(struct command_context *cmd_ctx, diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c index f8ab1532..71c7299e 100644 --- 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,16 +372,18 @@ static int armv4_5_get_core_reg(struct reg *reg) return ERROR_TARGET_NOT_HALTED; } - retval = armv4_5->armv4_5_common->read_core_reg(target, armv4_5->num, armv4_5->mode); - if (retval == ERROR_OK) + retval = armv4_5->armv4_5_common->read_core_reg(target, reg, armv4_5->num, armv4_5->mode); + if (retval == ERROR_OK) { reg->valid = 1; + reg->dirty = 0; + } return retval; } 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 +394,16 @@ static int armv4_5_set_core_reg(struct reg *reg, uint8_t *buf) 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 +425,23 @@ static int armv4_5_set_core_reg(struct reg *reg, uint8_t *buf) } } + /* 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 +455,7 @@ struct reg_cache* armv4_5_build_reg_cache(struct target *target, struct arm *arm 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) { @@ -480,6 +493,7 @@ struct reg_cache* armv4_5_build_reg_cache(struct target *target, struct arm *arm cache->num_regs++; } + armv4_5_common->core_cache = cache; return cache; } @@ -811,9 +825,14 @@ int armv4_5_run_algorithm_inner(struct target *target, int num_mem_params, struc 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); diff --git a/src/target/armv4_5.h b/src/target/armv4_5.h index dbd62c0d..c8fc5580 100644 --- 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; diff --git a/src/target/cortex_a8.c b/src/target/cortex_a8.c index 168fe127..c6a46c50 100644 --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -877,7 +877,7 @@ static int cortex_a8_restore_context(struct target *target) /* 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(struct target *target, int num, #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(struct target *target, int num, 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(struct target *target, int num, 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(struct target *target, int num, 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(struct target *target, int num, 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; } @@ -1619,7 +1619,6 @@ static void cortex_a8_build_reg_cache(struct target *target) armv4_5->core_type = ARM_MODE_MON; (*cache_p) = armv4_5_build_reg_cache(target, armv4_5); - armv4_5->core_cache = (*cache_p); } diff --git a/src/target/xscale.c b/src/target/xscale.c index f13366ac..c908fd70 100644 --- a/src/target/xscale.c +++ b/src/target/xscale.c @@ -1646,16 +1646,18 @@ static int xscale_deassert_reset(struct target *target) 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; } @@ -2829,7 +2831,6 @@ static void xscale_build_reg_cache(struct target *target) int num_regs = sizeof(xscale_reg_arch_info) / sizeof(struct xscale_reg); (*cache_p) = armv4_5_build_reg_cache(target, armv4_5); - armv4_5->core_cache = (*cache_p); (*cache_p)->next = malloc(sizeof(struct reg_cache)); cache_p = &(*cache_p)->next; -- 2.39.5