Skip to content

Commit

Permalink
target: fix messages and return values of failed op because not halted
Browse files Browse the repository at this point in the history 
Lot of messages was logged as LOG_WARNING, but the operation failed
immediately. Sometimes no error message was logged at all.
Add missing messages, change warnings to errors.

Sometimes ERROR_TARGET_INVALID was returned. Some command handlers
returned ERROR_OK! Always return ERROR_TARGET_NOT_HALTED.

While on it use LOG_TARGET_ERROR() whenever possible.
Prefix command_print() message with 'Error:' to get closer
to LOG_TARGET_ERROR() variant.

Error message was not added to get() and set() methods of
struct xxx_reg_type - the return value is properly checked and a message
is logged by the caller in case of ERROR_TARGET_NOT_HALTED.

Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: I2fe4187c6025f0038956ab387edbf3f461c69398
Reviewed-on: https://review.openocd.org/c/openocd/+/7819
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
  • Loading branch information
@tom-van
tom-van committed Jul 29, 2023
1 parent 7023deb commit a510824
Show file tree
Hide file tree
Showing 29 changed files with 145 additions and 131 deletions.
18 changes: 11 additions & 7 deletions src/target/aarch64.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,8 +846,10 @@ static int aarch64_resume(struct target *target, int current,
struct armv8_common *armv8 = target_to_armv8(target);
armv8->last_run_control_op = ARMV8_RUNCONTROL_RESUME;

if (target->state != TARGET_HALTED)
if (target->state != TARGET_HALTED) {
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

/*
* If this target is part of a SMP group, prepare the others
Expand Down Expand Up @@ -1095,7 +1097,7 @@ static int aarch64_step(struct target *target, int current, target_addr_t addres
armv8->last_run_control_op = ARMV8_RUNCONTROL_STEP;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2135,7 +2137,7 @@ static int aarch64_write_cpu_memory(struct target *target,
uint32_t dscr;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2353,7 +2355,7 @@ static int aarch64_read_cpu_memory(struct target *target,
address, size, count);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2790,8 +2792,8 @@ static int aarch64_mmu(struct target *target, int *enabled)
struct aarch64_common *aarch64 = target_to_aarch64(target);
struct armv8_common *armv8 = &aarch64->armv8_common;
if (target->state != TARGET_HALTED) {
LOG_ERROR("%s: target %s not halted", __func__, target_name(target));
return ERROR_TARGET_INVALID;
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}
if (armv8->is_armv8r)
*enabled = 0;
Expand Down Expand Up @@ -3010,8 +3012,10 @@ COMMAND_HANDLER(aarch64_mcrmrc_command)
return ERROR_FAIL;
}

if (target->state != TARGET_HALTED)
if (target->state != TARGET_HALTED) {
command_print(CMD, "Error: [%s] not halted", target_name(target));
return ERROR_TARGET_NOT_HALTED;
}

if (arm->core_state == ARM_STATE_AARCH64) {
command_print(CMD, "%s: not 32-bit arm target", target_name(target));
Expand Down
12 changes: 6 additions & 6 deletions src/target/arc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ static int arc_resume(struct target *target, int current, target_addr_t address,
CHECK_RETVAL(arc_reset_caches_states(target));

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1671,7 +1671,7 @@ static int arc_add_breakpoint(struct target *target, struct breakpoint *breakpoi
return arc_set_breakpoint(target, breakpoint);

} else {
LOG_WARNING(" > core was not halted, please try again.");
LOG_TARGET_ERROR(target, "not halted (add breakpoint)");
return ERROR_TARGET_NOT_HALTED;
}
}
Expand All @@ -1683,7 +1683,7 @@ static int arc_remove_breakpoint(struct target *target,
if (breakpoint->is_set)
CHECK_RETVAL(arc_unset_breakpoint(target, breakpoint));
} else {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted (remove breakpoint)");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1905,7 +1905,7 @@ static int arc_add_watchpoint(struct target *target,
struct watchpoint *watchpoint)
{
if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand All @@ -1918,7 +1918,7 @@ static int arc_remove_watchpoint(struct target *target,
struct watchpoint *watchpoint)
{
if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2006,7 +2006,7 @@ static int arc_step(struct target *target, int current, target_addr_t address,
struct reg *pc = &(arc->core_and_aux_cache->reg_list[arc->pc_index_in_cache]);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
2 changes: 1 addition & 1 deletion src/target/arc_mem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ int arc_mem_write(struct target *target, target_addr_t address, uint32_t size,
address, size, count);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
8 changes: 4 additions & 4 deletions src/target/arm11.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ static int arm11_resume(struct target *target, int current,


if (target->state != TARGET_HALTED) {
LOG_ERROR("Target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -551,7 +551,7 @@ static int arm11_step(struct target *target, int current,
target_state_name(target));

if (target->state != TARGET_HALTED) {
LOG_WARNING("target was not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -798,7 +798,7 @@ static int arm11_read_memory_inner(struct target *target,
int retval;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target was not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -896,7 +896,7 @@ static int arm11_write_memory_inner(struct target *target,
int retval;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target was not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
4 changes: 2 additions & 2 deletions src/target/arm720t.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ static int arm720t_arch_state(struct target *target)
static int arm720_mmu(struct target *target, int *enabled)
{
if (target->state != TARGET_HALTED) {
LOG_ERROR("%s: target not halted", __func__);
return ERROR_TARGET_INVALID;
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

*enabled = target_to_arm720(target)->armv4_5_mmu.mmu_enabled;
Expand Down
20 changes: 10 additions & 10 deletions src/target/arm7_9_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ static int arm7_9_set_breakpoint(struct target *target, struct breakpoint *break
breakpoint->type);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -455,7 +455,7 @@ static int arm7_9_set_watchpoint(struct target *target, struct watchpoint *watch
mask = watchpoint->length - 1;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -524,7 +524,7 @@ static int arm7_9_unset_watchpoint(struct target *target, struct watchpoint *wat
struct arm7_9_common *arm7_9 = target_to_arm7_9(target);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1259,7 +1259,7 @@ static int arm7_9_debug_entry(struct target *target)
return retval;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1390,7 +1390,7 @@ static int arm7_9_full_context(struct target *target)
LOG_DEBUG("-");

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1506,7 +1506,7 @@ static int arm7_9_restore_context(struct target *target)
LOG_DEBUG("-");

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1709,7 +1709,7 @@ int arm7_9_resume(struct target *target,
LOG_DEBUG("-");

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -1907,7 +1907,7 @@ int arm7_9_step(struct target *target, int current, target_addr_t address, int h
int err, retval;

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2118,7 +2118,7 @@ int arm7_9_read_memory(struct target *target,
address, size, count);

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -2291,7 +2291,7 @@ int arm7_9_write_memory(struct target *target,
#endif

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
8 changes: 4 additions & 4 deletions src/target/arm920t.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,8 +533,8 @@ int arm920t_arch_state(struct target *target)
static int arm920_mmu(struct target *target, int *enabled)
{
if (target->state != TARGET_HALTED) {
LOG_ERROR("%s: target not halted", __func__);
return ERROR_TARGET_INVALID;
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

*enabled = target_to_arm920(target)->armv4_5_mmu.mmu_enabled;
Expand Down Expand Up @@ -1455,9 +1455,9 @@ COMMAND_HANDLER(arm920t_handle_cp15_command)
return retval;

if (target->state != TARGET_HALTED) {
command_print(CMD, "target must be stopped for "
command_print(CMD, "Error: target must be stopped for "
"\"%s\" command", CMD_NAME);
return ERROR_OK;
return ERROR_TARGET_NOT_HALTED;
}

/* one argument, read a register.
Expand Down
4 changes: 2 additions & 2 deletions src/target/arm926ejs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,8 @@ static int arm926ejs_mmu(struct target *target, int *enabled)
struct arm926ejs_common *arm926ejs = target_to_arm926(target);

if (target->state != TARGET_HALTED) {
LOG_ERROR("Target not halted");
return ERROR_TARGET_INVALID;
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}
*enabled = arm926ejs->armv4_5_mmu.mmu_enabled;
return ERROR_OK;
Expand Down
4 changes: 2 additions & 2 deletions src/target/arm946e.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ COMMAND_HANDLER(arm946e_handle_cp15)
return retval;

if (target->state != TARGET_HALTED) {
command_print(CMD, "target must be stopped for \"%s\" command", CMD_NAME);
command_print(CMD, "Error: target must be stopped for \"%s\" command", CMD_NAME);
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -624,7 +624,7 @@ COMMAND_HANDLER(arm946e_handle_idcache)
return retval;

if (target->state != TARGET_HALTED) {
command_print(CMD, "target must be stopped for \"%s\" command", CMD_NAME);
command_print(CMD, "Error: target must be stopped for \"%s\" command", CMD_NAME);
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
4 changes: 2 additions & 2 deletions src/target/arm966e.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ COMMAND_HANDLER(arm966e_handle_cp15_command)
return retval;

if (target->state != TARGET_HALTED) {
command_print(CMD, "target must be stopped for \"%s\" command", CMD_NAME);
return ERROR_OK;
command_print(CMD, "Error: target must be stopped for \"%s\" command", CMD_NAME);
return ERROR_TARGET_NOT_HALTED;
}

/* one or more argument, access a single register (write if second argument is given */
Expand Down
16 changes: 9 additions & 7 deletions src/target/armv4_5.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ static int armv4_5_get_core_reg(struct reg *reg)
struct target *target = reg_arch_info->target;

if (target->state != TARGET_HALTED) {
LOG_ERROR("Target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand All @@ -600,7 +600,7 @@ static int armv4_5_set_core_reg(struct reg *reg, uint8_t *buf)
uint32_t value = buf_get_u32(buf, 0, 32);

if (target->state != TARGET_HALTED) {
LOG_ERROR("Target not halted");
LOG_TARGET_ERROR(target, "not halted");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down Expand Up @@ -817,8 +817,8 @@ COMMAND_HANDLER(handle_armv4_5_reg_command)
}

if (target->state != TARGET_HALTED) {
command_print(CMD, "error: target must be halted for register accesses");
return ERROR_FAIL;
command_print(CMD, "Error: target must be halted for register accesses");
return ERROR_TARGET_NOT_HALTED;
}

if (arm->core_type != ARM_CORE_TYPE_STD) {
Expand All @@ -833,7 +833,7 @@ COMMAND_HANDLER(handle_armv4_5_reg_command)
}

if (!arm->full_context) {
command_print(CMD, "error: target doesn't support %s",
command_print(CMD, "Error: target doesn't support %s",
CMD_NAME);
return ERROR_FAIL;
}
Expand Down Expand Up @@ -1018,8 +1018,10 @@ COMMAND_HANDLER(handle_armv4_5_mcrmrc)
return ERROR_FAIL;
}

if (target->state != TARGET_HALTED)
if (target->state != TARGET_HALTED) {
command_print(CMD, "Error: [%s] not halted", target_name(target));
return ERROR_TARGET_NOT_HALTED;
}

int cpnum;
uint32_t op1;
Expand Down Expand Up @@ -1307,7 +1309,7 @@ int armv4_5_run_algorithm_inner(struct target *target,
}

if (target->state != TARGET_HALTED) {
LOG_WARNING("target not halted");
LOG_TARGET_ERROR(target, "not halted (run target algo)");
return ERROR_TARGET_NOT_HALTED;
}

Expand Down
Loading

0 comments on commit a510824

Please sign in to comment.