Re: [PATCH v2 2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers

[Peter Maydell]

On Tue, 9 Mar 2021 at 14:24, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> From the semihosting point of view what we want to know is the current
> mode of the processor. Unify this into a single helper and allow us to
> use the same GET/SET_ARG helpers for the rest of the code.
>
> Note: we aren't currently testing riscv32 due to missing toolchain for
> check-tcg tests.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  semihosting/arm-compat-semi.c | 54 ++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 36 deletions(-)
>
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 94950b6c56..733eea1e2d 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -767,15 +767,28 @@ static const GuestFDFunctions guestfd_fns[] = {
>      },
>  };
>
> -/* Read the input value from the argument block; fail the semihosting
> - * call if the memory read fails.
> +/*
> + * Read the input value from the argument block; fail the semihosting
> + * call if the memory read fails. Eventually we could use a generic
> + * CPUState helper function here.
>   */
> +static inline bool is_64bit_semihosting(CPUArchState *env)
> +{
>  #ifdef TARGET_ARM
> +    return is_a64(env);
> +#elif defined(TARGET_RISCV)

can we be consistent about whether we use defined() or not, please ?

> +    return !riscv_cpu_is_32bit(env);
> +#else
> +#error un-handled architecture
> +#endif

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM