Re: [PATCH v4 24/53] semihosting: Split out common-semi-target.h

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Move the ARM and RISCV specific helpers into
> their own header file.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/common-semi-target.h   | 62 ++++++++++++++++++++
>  target/riscv/common-semi-target.h | 50 ++++++++++++++++
>  semihosting/arm-compat-semi.c     | 94 +------------------------------
>  3 files changed, 113 insertions(+), 93 deletions(-)
>  create mode 100644 target/arm/common-semi-target.h
>  create mode 100644 target/riscv/common-semi-target.h
>
> diff --git a/target/arm/common-semi-target.h b/target/arm/common-semi-target.h
> new file mode 100644
> index 0000000000..629d75ca5a
> --- /dev/null
> +++ b/target/arm/common-semi-target.h
> @@ -0,0 +1,62 @@
> +/*
> + * Target-specific parts of semihosting/arm-compat-semi.c.
> + *
> + * Copyright (c) 2005, 2007 CodeSourcery.
> + * Copyright (c) 2019, 2022 Linaro
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TARGET_ARM_COMMON_SEMI_TARGET_H
> +#define TARGET_ARM_COMMON_SEMI_TARGET_H
> +
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/arm/boot.h"
> +#endif
> +
> +static inline target_ulong common_semi_arg(CPUState *cs, int argno)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    if (is_a64(env)) {
> +        return env->xregs[argno];
> +    } else {
> +        return env->regs[argno];
> +    }
> +}
> +
> +static inline void common_semi_set_ret(CPUState *cs, target_ulong ret)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    if (is_a64(env)) {
> +        env->xregs[0] = ret;
> +    } else {
> +        env->regs[0] = ret;
> +    }
> +}
> +
> +static inline bool common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || is_a64(cs->env_ptr));
> +}
> +
> +static inline bool is_64bit_semihosting(CPUArchState *env)
> +{
> +    return is_a64(env);
> +}
> +
> +static inline target_ulong common_semi_stack_bottom(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    return is_a64(env) ? env->xregs[31] : env->regs[13];
> +}
> +
> +static inline bool common_semi_has_synccache(CPUArchState *env)
> +{
> +    /* Ok for A64, invalid for A32/T32 */
> +    return is_a64(env);
> +}
> +
> +#endif
> diff --git a/target/riscv/common-semi-target.h 
> b/target/riscv/common-semi-target.h
> new file mode 100644
> index 0000000000..7c8a59e0cc
> --- /dev/null
> +++ b/target/riscv/common-semi-target.h
> @@ -0,0 +1,50 @@
> +/*
> + * Target-specific parts of semihosting/arm-compat-semi.c.
> + *
> + * Copyright (c) 2005, 2007 CodeSourcery.
> + * Copyright (c) 2019, 2022 Linaro
> + * Copyright © 2020 by Keith Packard <keithp@keithp.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TARGET_RISCV_COMMON_SEMI_TARGET_H
> +#define TARGET_RISCV_COMMON_SEMI_TARGET_H
> +
> +static inline target_ulong common_semi_arg(CPUState *cs, int argno)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return env->gpr[xA0 + argno];
> +}
> +
> +static inline void common_semi_set_ret(CPUState *cs, target_ulong ret)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    env->gpr[xA0] = ret;
> +}
> +
> +static inline bool common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> +}
> +
> +static inline bool is_64bit_semihosting(CPUArchState *env)
> +{
> +    return riscv_cpu_mxl(env) != MXL_RV32;
> +}
> +
> +static inline target_ulong common_semi_stack_bottom(CPUState *cs)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return env->gpr[xSP];
> +}
> +
> +static inline bool common_semi_has_synccache(CPUArchState *env)
> +{
> +    return true;
> +}
> +
> +#endif
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 50f40a2a1a..5e442e549d 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -46,9 +46,6 @@
>  #else
>  #include "qemu/cutils.h"
>  #include "hw/loader.h"
> -#ifdef TARGET_ARM
> -#include "hw/arm/boot.h"
> -#endif
>  #include "hw/boards.h"
>  #endif
>  
> @@ -182,96 +179,7 @@ static LayoutInfo common_semi_find_bases(CPUState *cs)
>  
>  #endif
>  
> -#ifdef TARGET_ARM
> -static inline target_ulong
> -common_semi_arg(CPUState *cs, int argno)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -    if (is_a64(env)) {
> -        return env->xregs[argno];
> -    } else {
> -        return env->regs[argno];
> -    }
> -}
> -
> -static inline void
> -common_semi_set_ret(CPUState *cs, target_ulong ret)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -    if (is_a64(env)) {
> -        env->xregs[0] = ret;
> -    } else {
> -        env->regs[0] = ret;
> -    }
> -}
> -
> -static inline bool
> -common_semi_sys_exit_extended(CPUState *cs, int nr)
> -{
> -    return (nr == TARGET_SYS_EXIT_EXTENDED || is_a64(cs->env_ptr));
> -}
> -
> -static inline bool is_64bit_semihosting(CPUArchState *env)
> -{
> -    return is_a64(env);
> -}
> -
> -static inline target_ulong common_semi_stack_bottom(CPUState *cs)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -    return is_a64(env) ? env->xregs[31] : env->regs[13];
> -}
> -
> -static inline bool common_semi_has_synccache(CPUArchState *env)
> -{
> -    /* Ok for A64, invalid for A32/T32. */
> -    return is_a64(env);
> -}
> -#endif /* TARGET_ARM */
> -
> -#ifdef TARGET_RISCV
> -static inline target_ulong
> -common_semi_arg(CPUState *cs, int argno)
> -{
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -    CPURISCVState *env = &cpu->env;
> -    return env->gpr[xA0 + argno];
> -}
> -
> -static inline void
> -common_semi_set_ret(CPUState *cs, target_ulong ret)
> -{
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -    CPURISCVState *env = &cpu->env;
> -    env->gpr[xA0] = ret;
> -}
> -
> -static inline bool
> -common_semi_sys_exit_extended(CPUState *cs, int nr)
> -{
> -    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> -}
> -
> -static inline bool is_64bit_semihosting(CPUArchState *env)
> -{
> -    return riscv_cpu_mxl(env) != MXL_RV32;
> -}
> -
> -static inline target_ulong common_semi_stack_bottom(CPUState *cs)
> -{
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -    CPURISCVState *env = &cpu->env;
> -    return env->gpr[xSP];
> -}
> -
> -static inline bool common_semi_has_synccache(CPUArchState *env)
> -{
> -    return true;
> -}
> -#endif
> +#include "common-semi-target.h"
>  
>  /*
>   * The semihosting API has no concept of its errno being thread-safe,


-- 
Alex Bennée