On 7 December 2016 at 14:48, Julian Brown <jul...@codesourcery.com> wrote: > This patch introduces an ARM-specific version of the memory read/write, > etc. functions used for semihosting, in order to byte-swap (big-endian) > target memory that is to be interpreted by the (little-endian) host. > These functions also know about the byte-reversal used for BE32 system > mode. > > Signed-off-by: Julian Brown <jul...@codesourcery.com>
This patch seems to have compile problems: In file included from /home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/arm-semi.c:117:0: /home/petmay01/linaro/qemu-from-laptop/qemu/include/exec/softmmu-arm-semi.h: In function ‘softmmu_unlock_user’: /home/petmay01/linaro/qemu-from-laptop/qemu/include/exec/softmmu-arm-semi.h:139:14: error: unused variable ‘pc’ [-Werror=unused-variable] uint8_t *pc = p; ^ /home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/arm-semi.c: In function ‘arm_semi_flen_cb’: /home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/arm-semi.c:190:5: error: implicit declaration of function ‘armsemi_memory_rw_debug’ [-Werror=implicit-function-declaration] armsemi_memory_rw_debug(cs, arm_flen_buf(cpu) + 32, (uint8_t *)&size, 4, 0); ^ /home/petmay01/linaro/qemu-from-laptop/qemu/target/arm/arm-semi.c:190:5: error: nested extern declaration of ‘armsemi_memory_rw_debug’ [-Werror=nested-externs] [there's no version of armsemi_memory_rw_debug() for CONFIG_USER_ONLY] > --- > include/exec/softmmu-arm-semi.h | 148 > ++++++++++++++++++++++++++++++++++++++++ > target-arm/arm-semi.c | 4 +- > target-arm/cpu.c | 24 +++++++ > target-arm/cpu.h | 6 ++ > 4 files changed, 180 insertions(+), 2 deletions(-) > create mode 100644 include/exec/softmmu-arm-semi.h > > diff --git a/include/exec/softmmu-arm-semi.h b/include/exec/softmmu-arm-semi.h > new file mode 100644 > index 0000000..d97e017 > --- /dev/null > +++ b/include/exec/softmmu-arm-semi.h > @@ -0,0 +1,148 @@ > +/* > + * Helper routines to provide target memory access for ARM semihosting > + * syscalls in system emulation mode. > + * > + * Copyright (c) 2007 CodeSourcery, (c) 2016 Mentor Graphics > + * > + * This code is licensed under the GPL > + */ > + > +#ifndef SOFTMMU_ARM_SEMI_H > +#define SOFTMMU_ARM_SEMI_H 1 > + > +/* In BE32 system mode, the CPU-specific memory_rw_debug method will arrange > to > + * perform byteswapping on the target memory, so that it appears to the host > as > + * it appears to the emulated CPU. Memory is read verbatim in BE8 mode. (In > + * other words, this function arranges so that BUF has the same format in > both > + * BE8 and BE32 system mode.) > + */ > + > +static inline int armsemi_memory_rw_debug(CPUState *cpu, target_ulong addr, > + uint8_t *buf, int len, bool > is_write) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + > + if (cc->memory_rw_debug) { > + return cc->memory_rw_debug(cpu, addr, buf, len, is_write); > + } > + return cpu_memory_rw_debug(cpu, addr, buf, len, is_write); > +} Nothing about this is arm-semihosting specific; in fact it's identical to the existing target_memory_rw_debug() in gdbstub.c. We have a few of these "invoke the class method if it exists, or do this fallback" functions in include/qom/cpu.h, so that seems like a good place for it. (The logical name for the wrapper is "cpu_memory_rw_debug", except that it's already taken.) > +/* In big-endian mode (either BE8 or BE32), values larger than a byte will be > + * transferred to/from memory in big-endian format. Assuming we're on a > + * little-endian host machine, such values will need to be byteswapped before > + * and after the host processes them. > + * > + * This means that byteswapping will occur *twice* in BE32 mode for > + * halfword/word reads/writes. > + */ > + > +static inline bool arm_bswap_needed(CPUARMState *env) > +{ > +#ifdef HOST_WORDS_BIGENDIAN > +#error HOST_WORDS_BIGENDIAN is not supported for ARM semihosting at the > moment. > +#else > + return arm_sctlr_b(env) || arm_sctlr_ee(env); > +#endif > +} This looks odd. SCTLR.EE is the exception endianness, not the current endianness, so why is it involved in working out how to interpret the semihosting arguments? > + > +static inline uint64_t softmmu_tget64(CPUArchState *env, target_ulong addr) > +{ > + uint64_t val; > + > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 8, 0); > + if (arm_bswap_needed(env)) { > + return bswap64(val); > + } else { > + return val; > + } > +} > + > +static inline uint32_t softmmu_tget32(CPUArchState *env, target_ulong addr) > +{ > + uint32_t val; > + > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 4, 0); > + if (arm_bswap_needed(env)) { > + return bswap32(val); > + } else { > + return val; > + } > +} > + > +static inline uint32_t softmmu_tget8(CPUArchState *env, target_ulong addr) > +{ > + uint8_t val; > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, &val, 1, 0); > + return val; > +} > + > +#define get_user_u64(arg, p) ({ arg = softmmu_tget64(env, p); 0; }) > +#define get_user_u32(arg, p) ({ arg = softmmu_tget32(env, p) ; 0; }) > +#define get_user_u8(arg, p) ({ arg = softmmu_tget8(env, p) ; 0; }) > +#define get_user_ual(arg, p) get_user_u32(arg, p) > + > +static inline void softmmu_tput64(CPUArchState *env, > + target_ulong addr, uint64_t val) > +{ > + if (arm_bswap_needed(env)) { > + val = bswap64(val); > + } > + cpu_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 8, 1); > +} > + > +static inline void softmmu_tput32(CPUArchState *env, > + target_ulong addr, uint32_t val) > +{ > + if (arm_bswap_needed(env)) { > + val = bswap32(val); > + } > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 4, 1); > +} > +#define put_user_u64(arg, p) ({ softmmu_tput64(env, p, arg) ; 0; }) > +#define put_user_u32(arg, p) ({ softmmu_tput32(env, p, arg) ; 0; }) > +#define put_user_ual(arg, p) put_user_u32(arg, p) > + > +static void *softmmu_lock_user(CPUArchState *env, > + target_ulong addr, target_ulong len, int copy) > +{ > + uint8_t *p; > + /* TODO: Make this something that isn't fixed size. */ > + p = malloc(len); > + if (p && copy) { > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, p, len, 0); > + } > + return p; > +} > +#define lock_user(type, p, len, copy) softmmu_lock_user(env, p, len, copy) > +static char *softmmu_lock_user_string(CPUArchState *env, target_ulong addr) > +{ > + char *p; > + char *s; > + uint8_t c; > + /* TODO: Make this something that isn't fixed size. */ > + s = p = malloc(1024); > + if (!s) { > + return NULL; > + } > + do { > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, &c, 1, 0); > + addr++; > + *(p++) = c; > + } while (c); > + return s; > +} > +#define lock_user_string(p) softmmu_lock_user_string(env, p) > +static void softmmu_unlock_user(CPUArchState *env, void *p, target_ulong > addr, > + target_ulong len) > +{ > + uint8_t *pc = p; > + if (len) { > + armsemi_memory_rw_debug(ENV_GET_CPU(env), addr, p, len, 1); > + } > + free(p); > +} > + > +#define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) > + > +#endif > diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c > index 7cac873..1ad1e63 100644 > --- a/target-arm/arm-semi.c > +++ b/target-arm/arm-semi.c > @@ -114,7 +114,7 @@ static inline uint32_t set_swi_errno(CPUARMState *env, > uint32_t code) > return code; > } > > -#include "exec/softmmu-semi.h" > +#include "exec/softmmu-arm-semi.h" > #endif > > static target_ulong arm_semi_syscall_len; > @@ -187,7 +187,7 @@ static void arm_semi_flen_cb(CPUState *cs, target_ulong > ret, target_ulong err) > /* The size is always stored in big-endian order, extract > the value. We assume the size always fit in 32 bits. */ > uint32_t size; > - cpu_memory_rw_debug(cs, arm_flen_buf(cpu) + 32, (uint8_t *)&size, 4, 0); > + armsemi_memory_rw_debug(cs, arm_flen_buf(cpu) + 32, (uint8_t *)&size, 4, > 0); > size = be32_to_cpu(size); > if (is_a64(env)) { > env->xregs[0] = size; > diff --git a/target-arm/cpu.c b/target-arm/cpu.c > index 512964e..6afb0d9 100644 > --- a/target-arm/cpu.c > +++ b/target-arm/cpu.c > @@ -1556,6 +1556,27 @@ static gchar *arm_gdb_arch_name(CPUState *cs) > return g_strdup("arm"); > } > > +#ifndef CONFIG_USER_ONLY > +static int arm_cpu_memory_rw_debug(CPUState *cpu, vaddr address, > + uint8_t *buf, int len, bool is_write) > +{ > + target_ulong addr = address; > + ARMCPU *armcpu = ARM_CPU(cpu); > + CPUARMState *env = &armcpu->env; > + > + if (arm_sctlr_b(env)) { > + target_ulong i; > + for (i = 0; i < len; i++) { > + cpu_memory_rw_debug(cpu, (addr + i) ^ 3, &buf[i], 1, is_write); > + } > + } else { > + cpu_memory_rw_debug(cpu, addr, buf, len, is_write); > + } > + > + return 0; > +} > +#endif > + > static void arm_cpu_class_init(ObjectClass *oc, void *data) > { > ARMCPUClass *acc = ARM_CPU_CLASS(oc); > @@ -1573,6 +1594,9 @@ static void arm_cpu_class_init(ObjectClass *oc, void > *data) > cc->has_work = arm_cpu_has_work; > cc->cpu_exec_interrupt = arm_cpu_exec_interrupt; > cc->dump_state = arm_cpu_dump_state; > +#if !defined(CONFIG_USER_ONLY) > + cc->memory_rw_debug = arm_cpu_memory_rw_debug; > +#endif > cc->set_pc = arm_cpu_set_pc; > cc->gdb_read_register = arm_cpu_gdb_read_register; > cc->gdb_write_register = arm_cpu_gdb_write_register; > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index becbfce..087cf96 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -2115,6 +2115,12 @@ static inline bool arm_sctlr_b(CPUARMState *env) > (env->cp15.sctlr_el[1] & SCTLR_B) != 0; > } > > +static inline bool arm_sctlr_ee(CPUARMState *env) > +{ > + return arm_feature(env, ARM_FEATURE_V7) && > + (env->cp15.sctlr_el[1] & SCTLR_EE) != 0; > +} > + > /* Return true if the processor is in big-endian mode. */ > static inline bool arm_cpu_data_is_big_endian(CPUARMState *env) > { thanks -- PMM