Re: [PATCH v15 8/9] target/loongarch: Adjust functions and structure to support user-mode

[Richard Henderson]

On 6/8/22 19:42, Song Gao wrote:
> diff --git a/target/loongarch/helper.h b/target/loongarch/helper.h
> index 85c11a60d4..ee42707868 100644
> --- a/target/loongarch/helper.h
> +++ b/target/loongarch/helper.h
> @@ -93,8 +93,7 @@ DEF_HELPER_2(frint_d, i64, env, i64)
>   
>   DEF_HELPER_FLAGS_2(set_rounding_mode, TCG_CALL_NO_RWG, void, env, i32)
>   
> -DEF_HELPER_1(rdtime_d, i64, env)
> -
> +#ifndef CONFIG_USER_ONLY
>   /* CSRs helper */
>   DEF_HELPER_1(csrrd_pgd, i64, env)
>   DEF_HELPER_1(csrrd_tval, i64, env)
> @@ -128,3 +127,5 @@ DEF_HELPER_4(lddir, tl, env, tl, tl, i32)
>   DEF_HELPER_4(ldpte, void, env, tl, tl, i32)
>   DEF_HELPER_1(ertn, void, env)
>   DEF_HELPER_1(idle, void, env)
> +DEF_HELPER_1(rdtime_d, i64, env)
> +#endif

This is wrong.

>   static bool trans_rdtimel_w(DisasContext *ctx, arg_rdtimel_w *a)
>   {
> +#ifdef CONFIG_USER_ONLY
> +    return cpu_get_host_ticks();

This is very wrong.  You're calling cpu_get_host_ticks at translation time.
There are no changes required during translation.

You should in fact be calling cpu_get_host_ticks in helper_rdtime_d.

>   void helper_asrtle_d(CPULoongArchState *env, target_ulong rj, target_ulong rk)
>   {
>       if (rj > rk) {
> +#ifdef CONFIG_USER_ONLY
> +        cpu_loop_exit_sigsegv(env_cpu(env), GETPC(),
> +                              MMU_DATA_LOAD, true, GETPC());
> +#else
>           do_raise_exception(env, EXCCODE_ADEM, GETPC());
> +#endif

This change is wrong.  First, the kernel's do_ade raises SIGBUS.  Second, GETPC() is a 
host address, not a guest address.  Third, this highlights the fact that the existing 
system code is wrong, and should be setting badvaddr.

You need to
(1) set badvaddr here, and then
(2) handle EXCCODE_ADEM in linux-user/loongarch/cpu_loop.c to 
force_fix_fault(TARGET_SIGBUS, TARGET_BUS_ADRERR, env->badvaddr).

>   void helper_asrtgt_d(CPULoongArchState *env, target_ulong rj, target_ulong rk)
>   {
>       if (rj <= rk) {
> +#ifdef CONFIG_USER_ONLY
> +        cpu_loop_exit_sigsegv(env_cpu(env), GETPC(),
> +                              MMU_DATA_LOAD, true, GETPC());
> +#else
>           do_raise_exception(env, EXCCODE_ADEM, GETPC());
> +#endif
>       }
>   }

Likewise.


r~