Re: [RFC PATCH v7 12/29] target/loongarch: Add timer related instructions support.

[Richard Henderson <richard.henderson@linaro.org>]

On 3/28/22 06:57, Xiaojuan Yang wrote:
> +#ifndef CONFIG_USER_ONLY
> +static bool gen_rdtime(DisasContext *ctx, arg_rr *a,
> +                       bool word, bool high)
> +{
> +    TCGv dst1 = gpr_dst(ctx, a->rd, EXT_NONE);
> +    TCGv dst2 = gpr_dst(ctx, a->rj, EXT_NONE);
> +
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
> +    gen_helper_rdtime_d(dst1, cpu_env);
> +    if (word) {
> +        tcg_gen_sextract_tl(dst1, dst1, high ? 32 : 0, 32);
> +    }
> +    tcg_gen_ld_i64(dst2, cpu_env, offsetof(CPULoongArchState, CSR_TID));
> +
> +    return true;
> +}

Remove all of the ifdefs.

>   static bool trans_rdtimel_w(DisasContext *ctx, arg_rdtimel_w *a)
>   {
> +#ifdef CONFIG_USER_ONLY
>       tcg_gen_movi_tl(cpu_gpr[a->rd], 0);
>       return true;

This (and all of the others) turns out to be a bug, as it fails to write to rj at all.

> +uint64_t helper_rdtime_d(CPULoongArchState *env)
> +{
> +     LoongArchCPU *cpu = LOONGARCH_CPU(env_cpu(env));
> +     return cpu_loongarch_get_constant_timer_counter(cpu);
> +}

Here, you could have

#ifdef CONFIG_USER_ONLY
     return cpu_get_host_ticks();
#else
     ...

which is the fallback we use for other targets in user-mode.

You seem to be missing the checks on CSR.MISC.DRDTL* which would raise IPE.


r~