Re: [PATCH v2] target/loongarch: Fix emulation of float-point disable exception

[Richard Henderson]

On 11/3/22 23:25, Rui Wang wrote:
> +/*
> + * LoongArch CPUs hardware flags.
> + * bit[2..0] for MMU index.
> + * bit[7..4] for CSR.EUEN.{ BTE, ASXE, SXE, FPE }.
> + */
> +#define HW_FLAGS_MMU_MASK   0x07
> +#define HW_FLAGS_EUEN_FPE   0x10
> +
>   static inline void cpu_get_tb_cpu_state(CPULoongArchState *env,
>                                           target_ulong *pc,
>                                           target_ulong *cs_base,
> @@ -399,6 +408,10 @@ static inline void cpu_get_tb_cpu_state(CPULoongArchState 
> *env,
>       *pc = env->pc;
>       *cs_base = 0;
>       *flags = cpu_mmu_index(env, false);
> +
> +    if (FIELD_EX64(env->CSR_EUEN, CSR_EUEN, FPE)) {
> +        *flags |= HW_FLAGS_EUEN_FPE;
> +    }
>   }
>   
>   void loongarch_cpu_list(void);
> diff --git a/target/loongarch/insn_trans/trans_farith.c.inc 
> b/target/loongarch/insn_trans/trans_farith.c.inc
> index 7bb3f41aee..e2dec75dfb 100644
> --- a/target/loongarch/insn_trans/trans_farith.c.inc
> +++ b/target/loongarch/insn_trans/trans_farith.c.inc
> @@ -3,9 +3,22 @@
>    * Copyright (c) 2021 Loongson Technology Corporation Limited
>    */
>   
> +#ifndef CONFIG_USER_ONLY
> +#define CHECK_FPE do { \
> +    if ((ctx->base.tb->flags & HW_FLAGS_EUEN_FPE) == 0) { \
> +        generate_exception(ctx, EXCCODE_FPD); \
> +        return false; \
> +    } \
> +} while (0)
> +#else
> +#define CHECK_FPE
> +#endif

Oh excellent, you found the correct solution on your own.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

The only thing remaining from my comments on v1, which should be a separate patch, is to 
separate PG from PLV in tb->flags, so that the check

> --- a/target/loongarch/insn_trans/trans_privileged.c.inc
> +++ b/target/loongarch/insn_trans/trans_privileged.c.inc
> @@ -159,7 +159,7 @@ static const CSRInfo csr_info[] = {
>   
>   static bool check_plv(DisasContext *ctx)
>   {
> -    if (ctx->base.tb->flags == MMU_USER_IDX) {
> +    if (ctx->mem_idx == MMU_USER_IDX) {

here is correct.  I would suggest

#define HW_FLAGS_PLV_MASK   R_CSR_CRMD_PLV_MASK  /* 0x03 */
#define HW_FLAGS_CRMD_PG    R_CSR_CRMD_PG_MASK   /* 0x10 */
#define HW_FLAGS_EUEN_FPE   0x04

For cpu_get_tb_cpu_state

    *flags = env->CSR_CRMD & (R_CSR_CRMD_PLV_MASK | R_CSR_CRMD_PG_MASK);
    *flags |= FIELD_EX64(env->CSR_EUEN, CSR_EUEN, FPE) * HW_FLAGS_EUEN_FPE;

And for loongarch_tr_init_disas_context,

    ctx->plv = ctx->base.tb->flags & HW_FLAGS_PLV_MASK;
    if (ctx->base.tb->flags & HW_FLAGS_CRMD_PG_MASK) {
        ctx->mem_idx = ctx->plv;
    } else {
        ctx->mem_idx = MMU_DA_IDX;
    }


r~