From bb19cbc95ada89ce5e02c132bc6f3268b1a1bfa3 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 7 Sep 2015 10:39:27 +0100 Subject: [PATCH] target-arm/arm-semi.c: Factor out repeated 'return env->regs[0]' Factor out a repeated pattern in the semihosting code: gdb_do_syscall(arm_semi_cb, "system,%s", arg0, (int)arg1+1); /* arm_semi_cb sets env->regs[0] to the syscall return value */ return env->regs[0]; For A64 the return value will go in a different register; pull the sequence out into its own function that passes the return value in a static variable rather than overloading regs[0] for the purpose, so the code will work on both A32/T32 and A64. Note that the lack-of-synchronization bug noted in the FIXME comment is not introduced by this commit, but was already present. Signed-off-by: Peter Maydell Reviewed-by: Christopher Covington Tested-by: Christopher Covington Message-id: 1439483745-28752-5-git-send-email-peter.maydell@linaro.org --- target-arm/arm-semi.c | 79 +++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c index 42522a70d9..dbdc211581 100644 --- a/target-arm/arm-semi.c +++ b/target-arm/arm-semi.c @@ -134,6 +134,7 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) #ifdef CONFIG_USER_ONLY TaskState *ts = cs->opaque; #endif + target_ulong reg0 = env->regs[0]; if (ret == (target_ulong)-1) { #ifdef CONFIG_USER_ONLY @@ -141,22 +142,23 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) #else syscall_err = err; #endif - env->regs[0] = ret; + reg0 = ret; } else { /* Fixup syscalls that use nonstardard return conventions. */ - switch (env->regs[0]) { + switch (reg0) { case TARGET_SYS_WRITE: case TARGET_SYS_READ: - env->regs[0] = arm_semi_syscall_len - ret; + reg0 = arm_semi_syscall_len - ret; break; case TARGET_SYS_SEEK: - env->regs[0] = 0; + reg0 = 0; break; default: - env->regs[0] = ret; + reg0 = ret; break; } } + env->regs[0] = reg0; } static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err) @@ -175,6 +177,25 @@ static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err) #endif } +static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb, + const char *fmt, ...) +{ + va_list va; + CPUARMState *env = &cpu->env; + + va_start(va, fmt); + gdb_do_syscallv(cb, fmt, va); + va_end(va); + + /* FIXME: we are implicitly relying on the syscall completing + * before this point, which is not guaranteed. We should + * put in an explicit synchronization between this and + * the callback function. + */ + + return env->regs[0]; +} + /* Read the input value from the argument block; fail the semihosting * call if the memory read fails. */ @@ -223,9 +244,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) return result_fileno; } if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "open,%s,%x,1a4", arg0, - (int)arg2+1, gdb_open_modeflags[arg1]); - ret = env->regs[0]; + ret = arm_gdb_syscall(cpu, arm_semi_cb, "open,%s,%x,1a4", arg0, + (int)arg2+1, gdb_open_modeflags[arg1]); } else { ret = set_swi_errno(ts, open(s, open_modeflags[arg1], 0644)); } @@ -234,8 +254,7 @@ uint32_t do_arm_semihosting(CPUARMState *env) case TARGET_SYS_CLOSE: GET_ARG(0); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "close,%x", arg0); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "close,%x", arg0); } else { return set_swi_errno(ts, close(arg0)); } @@ -248,8 +267,7 @@ uint32_t do_arm_semihosting(CPUARMState *env) return (uint32_t)-1; /* Write to debug console. stderr is near enough. */ if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "write,2,%x,1", args); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args); } else { return write(STDERR_FILENO, &c, 1); } @@ -260,8 +278,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) return (uint32_t)-1; len = strlen(s); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "write,2,%x,%x", args, len); - ret = env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x", + args, len); } else { ret = write(STDERR_FILENO, s, len); } @@ -274,8 +292,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) len = arg2; if (use_gdb_syscalls()) { arm_semi_syscall_len = len; - gdb_do_syscall(arm_semi_cb, "write,%x,%x,%x", arg0, arg1, len); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "write,%x,%x,%x", + arg0, arg1, len); } else { s = lock_user(VERIFY_READ, arg1, len, 1); if (!s) { @@ -295,8 +313,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) len = arg2; if (use_gdb_syscalls()) { arm_semi_syscall_len = len; - gdb_do_syscall(arm_semi_cb, "read,%x,%x,%x", arg0, arg1, len); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "read,%x,%x,%x", + arg0, arg1, len); } else { s = lock_user(VERIFY_WRITE, arg1, len, 0); if (!s) { @@ -317,8 +335,7 @@ uint32_t do_arm_semihosting(CPUARMState *env) case TARGET_SYS_ISTTY: GET_ARG(0); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "isatty,%x", arg0); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "isatty,%x", arg0); } else { return isatty(arg0); } @@ -326,8 +343,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) GET_ARG(0); GET_ARG(1); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "lseek,%x,%x,0", arg0, arg1); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "lseek,%x,%x,0", + arg0, arg1); } else { ret = set_swi_errno(ts, lseek(arg0, arg1, SEEK_SET)); if (ret == (uint32_t)-1) @@ -337,9 +354,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) case TARGET_SYS_FLEN: GET_ARG(0); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_flen_cb, "fstat,%x,%x", - arg0, env->regs[13]-64); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_flen_cb, "fstat,%x,%x", + arg0, env->regs[13]-64); } else { struct stat buf; ret = set_swi_errno(ts, fstat(arg0, &buf)); @@ -354,8 +370,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) GET_ARG(0); GET_ARG(1); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "unlink,%s", arg0, (int)arg1+1); - ret = env->regs[0]; + ret = arm_gdb_syscall(cpu, arm_semi_cb, "unlink,%s", + arg0, (int)arg1+1); } else { s = lock_user_string(arg0); if (!s) { @@ -372,9 +388,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) GET_ARG(2); GET_ARG(3); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "rename,%s,%s", - arg0, (int)arg1+1, arg2, (int)arg3+1); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "rename,%s,%s", + arg0, (int)arg1+1, arg2, (int)arg3+1); } else { char *s2; s = lock_user_string(arg0); @@ -398,8 +413,8 @@ uint32_t do_arm_semihosting(CPUARMState *env) GET_ARG(0); GET_ARG(1); if (use_gdb_syscalls()) { - gdb_do_syscall(arm_semi_cb, "system,%s", arg0, (int)arg1+1); - return env->regs[0]; + return arm_gdb_syscall(cpu, arm_semi_cb, "system,%s", + arg0, (int)arg1+1); } else { s = lock_user_string(arg0); if (!s) {