[PATCH v2 00/14] Support UXL filed in xstatus

[PATCH v2 00/14] Support UXL filed in xstatus

[LIU Zhiwei]

In this patch set, we process the pc reigsters writes,
gdb reads and writes, and address calculation under
different UXLEN settings.

v2:
  Split out vill from vtype
  Remove context switch when xlen changes at exception
  Use XL instead of OL in many places
  Use pointer masking and XLEN for vector address
  Define an common fuction to calculate address for ldst

LIU Zhiwei (14):
  target/riscv: Sign extend pc for different XLEN
  target/riscv: Ignore the pc bits above XLEN
  target/riscv: Extend pc for runtime pc write
  target/riscv: Use gdb xml according to max mxlen
  target/riscv: Calculate address according to XLEN
  target/riscv: Adjust vsetvl according to XLEN
  target/riscv: Ajdust vector atomic check with XLEN
  target/riscv: Fix check range for first fault only
  target/riscv: Relax debug check for pm write
  target/riscv: Adjust vector address with mask
  target/riscv: Adjust scalar reg in vector with XLEN
  target/riscv: Split out the vill from vtype
  target/riscv: Don't save pc when exception return
  target/riscv: Enable uxl field write

 target/riscv/cpu.c                            | 23 +++++-
 target/riscv/cpu.h                            |  9 +++
 target/riscv/cpu_helper.c                     | 47 +++++++++++-
 target/riscv/csr.c                            | 42 ++++++++++-
 target/riscv/gdbstub.c                        | 73 ++++++++++++++-----
 target/riscv/helper.h                         |  6 +-
 .../riscv/insn_trans/trans_privileged.c.inc   |  7 +-
 target/riscv/insn_trans/trans_rvd.c.inc       | 23 +-----
 target/riscv/insn_trans/trans_rvf.c.inc       | 23 +-----
 target/riscv/insn_trans/trans_rvi.c.inc       | 22 +-----
 target/riscv/insn_trans/trans_rvv.c.inc       | 12 +--
 target/riscv/internals.h                      |  1 +
 target/riscv/machine.c                        | 11 +++
 target/riscv/op_helper.c                      |  7 +-
 target/riscv/translate.c                      | 29 +++++++-
 target/riscv/vector_helper.c                  | 54 +++++++++-----
 16 files changed, 263 insertions(+), 126 deletions(-)

-- 
2.25.1

[PATCH v2 01/14] target/riscv: Sign extend pc for different XLEN

[LIU Zhiwei]

When pc is written, it is sign-extended to fill the widest supported XLEN.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/translate.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 1d57bc97b5..a6a73ced9e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -150,16 +150,24 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in)
     tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan);
 }
 
+static void gen_set_pc(DisasContext *ctx, target_ulong dest)
+{
+    if (get_xl(ctx) == MXL_RV32) {
+        dest = (int32_t)dest;
+    }
+    tcg_gen_movi_tl(cpu_pc, dest);
+}
+
 static void generate_exception(DisasContext *ctx, int excp)
 {
-    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
+    gen_set_pc(ctx, ctx->base.pc_next);
     gen_helper_raise_exception(cpu_env, tcg_constant_i32(excp));
     ctx->base.is_jmp = DISAS_NORETURN;
 }
 
 static void generate_exception_mtval(DisasContext *ctx, int excp)
 {
-    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
+    gen_set_pc(ctx, ctx->base.pc_next);
     tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr));
     gen_helper_raise_exception(cpu_env, tcg_constant_i32(excp));
     ctx->base.is_jmp = DISAS_NORETURN;
@@ -179,10 +187,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
     if (translator_use_goto_tb(&ctx->base, dest)) {
         tcg_gen_goto_tb(n);
-        tcg_gen_movi_tl(cpu_pc, dest);
+        gen_set_pc(ctx, dest);
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
-        tcg_gen_movi_tl(cpu_pc, dest);
+        gen_set_pc(ctx, dest);
         tcg_gen_lookup_and_goto_ptr();
     }
 }
-- 
2.25.1

Re: [PATCH v2 01/14] target/riscv: Sign extend pc for different XLEN

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> When pc is written, it is sign-extended to fill the widest supported XLEN.
> 
> Signed-off-by: LIU Zhiwei<zhiwei_liu@c-sky.com>
> ---
>   target/riscv/translate.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)

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

r~

[PATCH v2 02/14] target/riscv: Ignore the pc bits above XLEN

[LIU Zhiwei]

The read from PC for translation is in cpu_get_tb_cpu_state, before translation.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9eeed38c7e..4c048cc266 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -70,8 +70,9 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
                           target_ulong *cs_base, uint32_t *pflags)
 {
     uint32_t flags = 0;
+    RISCVMXL xl = cpu_get_xl(env);
 
-    *pc = env->pc;
+    *pc = xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc;
     *cs_base = 0;
 
     if (riscv_has_ext(env, RVV)) {
@@ -127,7 +128,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
     }
 #endif
 
-    flags = FIELD_DP32(flags, TB_FLAGS, XL, cpu_get_xl(env));
+    flags = FIELD_DP32(flags, TB_FLAGS, XL, xl);
 
     *pflags = flags;
 }
-- 
2.25.1

[PATCH v2 03/14] target/riscv: Extend pc for runtime pc write

[LIU Zhiwei]

In some cases, we must restore the guest PC to the address of the start of
the TB, such as when the instruction counter hits zero. So extend pc register
according to current xlen for these cases.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.c        | 22 +++++++++++++++++++---
 target/riscv/cpu.h        |  2 ++
 target/riscv/cpu_helper.c |  2 +-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f812998123..0d2d175fa2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -319,7 +319,12 @@ static void riscv_cpu_set_pc(CPUState *cs, vaddr value)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-    env->pc = value;
+
+    if (cpu_get_xl(env) == MXL_RV32) {
+        env->pc = (int32_t)value;
+    } else {
+        env->pc = value;
+    }
 }
 
 static void riscv_cpu_synchronize_from_tb(CPUState *cs,
@@ -327,7 +332,13 @@ static void riscv_cpu_synchronize_from_tb(CPUState *cs,
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-    env->pc = tb->pc;
+    RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL);
+
+    if (xl == MXL_RV32) {
+        env->pc = (int32_t)tb->pc;
+    } else {
+        env->pc = tb->pc;
+    }
 }
 
 static bool riscv_cpu_has_work(CPUState *cs)
@@ -348,7 +359,12 @@ static bool riscv_cpu_has_work(CPUState *cs)
 void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
                           target_ulong *data)
 {
-    env->pc = data[0];
+    RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL);
+    if (xl == MXL_RV32) {
+        env->pc = (int32_t)data[0];
+    } else {
+        env->pc = data[0];
+    }
 }
 
 static void riscv_cpu_reset(DeviceState *dev)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0760c0af93..8befff0166 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -420,6 +420,8 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
 }
 #endif
 
+RISCVMXL cpu_get_xl(CPURISCVState *env);
+
 /*
  * A simplification for VLMAX
  * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 4c048cc266..79aba9c880 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -35,7 +35,7 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 #endif
 }
 
-static RISCVMXL cpu_get_xl(CPURISCVState *env)
+RISCVMXL cpu_get_xl(CPURISCVState *env)
 {
 #if defined(TARGET_RISCV32)
     return MXL_RV32;
-- 
2.25.1

[PATCH v2 04/14] target/riscv: Use gdb xml according to max mxlen

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/gdbstub.c | 73 +++++++++++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 23429179e2..7563414ef7 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -24,11 +24,25 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
+    target_ulong tmp;
 
     if (n < 32) {
-        return gdb_get_regl(mem_buf, env->gpr[n]);
+        tmp = env->gpr[n];
     } else if (n == 32) {
-        return gdb_get_regl(mem_buf, env->pc);
+        tmp = env->pc;
+    } else {
+        return 0;
+    }
+
+    switch (env->misa_mxl_max) {
+    case MXL_RV32:
+        gdb_get_reg32(mem_buf, tmp);
+        break;
+    case MXL_RV64:
+        gdb_get_reg64(mem_buf, tmp);
+        break;
+    default:
+        g_assert_not_reached();
     }
     return 0;
 }
@@ -37,18 +51,32 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-
-    if (n == 0) {
-        /* discard writes to x0 */
-        return sizeof(target_ulong);
-    } else if (n < 32) {
-        env->gpr[n] = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
+    int length = 0;
+    target_ulong tmp;
+
+    switch (env->misa_mxl_max) {
+    case MXL_RV32:
+        tmp = (int32_t)ldl_p(mem_buf);
+        length = 4;
+        break;
+    case MXL_RV64:
+        if (cpu_get_xl(env) < MXL_RV64) {
+            tmp = (int32_t)ldq_p(mem_buf);
+        } else {
+            tmp = ldq_p(mem_buf);
+        }
+        length = 8;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    if (n > 0 && n < 32) {
+        env->gpr[n] = tmp;
     } else if (n == 32) {
-        env->pc = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
+        env->pc = tmp;
     }
-    return 0;
+
+    return length;
 }
 
 static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
@@ -198,13 +226,20 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
         gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
                                  36, "riscv-32bit-fpu.xml", 0);
     }
-#if defined(TARGET_RISCV32)
-    gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
-                             1, "riscv-32bit-virtual.xml", 0);
-#elif defined(TARGET_RISCV64)
-    gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
-                             1, "riscv-64bit-virtual.xml", 0);
-#endif
+    switch (env->misa_mxl_max) {
+    case MXL_RV32:
+        gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
+                                 riscv_gdb_set_virtual,
+                                 1, "riscv-32bit-virtual.xml", 0);
+        break;
+    case MXL_RV64:
+        gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
+                                 riscv_gdb_set_virtual,
+                                 1, "riscv-64bit-virtual.xml", 0);
+        break;
+    default:
+        g_assert_not_reached();
+    }
 
     gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
                              riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
-- 
2.25.1

Re: [PATCH v2 04/14] target/riscv: Use gdb xml according to max mxlen

[LIU Zhiwei]

On 2021/11/10 下午3:04, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/riscv/gdbstub.c | 73 +++++++++++++++++++++++++++++++-----------
>   1 file changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 23429179e2..7563414ef7 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -24,11 +24,25 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>   {
>       RISCVCPU *cpu = RISCV_CPU(cs);
>       CPURISCVState *env = &cpu->env;
> +    target_ulong tmp;
>   
>       if (n < 32) {
> -        return gdb_get_regl(mem_buf, env->gpr[n]);
> +        tmp = env->gpr[n];
>       } else if (n == 32) {
> -        return gdb_get_regl(mem_buf, env->pc);
> +        tmp = env->pc;
> +    } else {
> +        return 0;
> +    }
> +
> +    switch (env->misa_mxl_max) {
> +    case MXL_RV32:
> +        gdb_get_reg32(mem_buf, tmp);

Oops. This is a typo. It should be

return gdb_get_reg32(mem_buf, tmp)

> +        break;
> +    case MXL_RV64:
> +        gdb_get_reg64(mem_buf, tmp);

and

gdb_get_reg64(mem_buf, tmp);


Will fix it in next patch set.

Thanks,
Zhiwei

> +        break;
> +    default:
> +        g_assert_not_reached();
>       }
>       return 0;
>   }
> @@ -37,18 +51,32 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>   {
>       RISCVCPU *cpu = RISCV_CPU(cs);
>       CPURISCVState *env = &cpu->env;
> -
> -    if (n == 0) {
> -        /* discard writes to x0 */
> -        return sizeof(target_ulong);
> -    } else if (n < 32) {
> -        env->gpr[n] = ldtul_p(mem_buf);
> -        return sizeof(target_ulong);
> +    int length = 0;
> +    target_ulong tmp;
> +
> +    switch (env->misa_mxl_max) {
> +    case MXL_RV32:
> +        tmp = (int32_t)ldl_p(mem_buf);
> +        length = 4;
> +        break;
> +    case MXL_RV64:
> +        if (cpu_get_xl(env) < MXL_RV64) {
> +            tmp = (int32_t)ldq_p(mem_buf);
> +        } else {
> +            tmp = ldq_p(mem_buf);
> +        }
> +        length = 8;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    if (n > 0 && n < 32) {
> +        env->gpr[n] = tmp;
>       } else if (n == 32) {
> -        env->pc = ldtul_p(mem_buf);
> -        return sizeof(target_ulong);
> +        env->pc = tmp;
>       }
> -    return 0;
> +
> +    return length;
>   }
>   
>   static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
> @@ -198,13 +226,20 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
>           gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
>                                    36, "riscv-32bit-fpu.xml", 0);
>       }
> -#if defined(TARGET_RISCV32)
> -    gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
> -                             1, "riscv-32bit-virtual.xml", 0);
> -#elif defined(TARGET_RISCV64)
> -    gdb_register_coprocessor(cs, riscv_gdb_get_virtual, riscv_gdb_set_virtual,
> -                             1, "riscv-64bit-virtual.xml", 0);
> -#endif
> +    switch (env->misa_mxl_max) {
> +    case MXL_RV32:
> +        gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> +                                 riscv_gdb_set_virtual,
> +                                 1, "riscv-32bit-virtual.xml", 0);
> +        break;
> +    case MXL_RV64:
> +        gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> +                                 riscv_gdb_set_virtual,
> +                                 1, "riscv-64bit-virtual.xml", 0);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
>   
>       gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
>                                riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),

[PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvd.c.inc | 23 ++---------------------
 target/riscv/insn_trans/trans_rvf.c.inc | 23 ++---------------------
 target/riscv/insn_trans/trans_rvi.c.inc | 18 ++----------------
 target/riscv/translate.c                | 13 +++++++++++++
 4 files changed, 19 insertions(+), 58 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 64fb0046f7..29066a8ef3 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -20,19 +20,10 @@
 
 static bool trans_fld(DisasContext *ctx, arg_fld *a)
 {
-    TCGv addr;
-
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVD);
 
-    addr = get_gpr(ctx, a->rs1, EXT_NONE);
-    if (a->imm) {
-        TCGv temp = temp_new(ctx);
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
-
     tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], addr, ctx->mem_idx, MO_TEQ);
 
     mark_fs_dirty(ctx);
@@ -41,21 +32,11 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
 
 static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
 {
-    TCGv addr;
-
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVD);
 
-    addr = get_gpr(ctx, a->rs1, EXT_NONE);
-    if (a->imm) {
-        TCGv temp = temp_new(ctx);
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
-
     tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, MO_TEQ);
-
     return true;
 }
 
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index b5459249c4..a33897db7d 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -26,19 +26,10 @@
 static bool trans_flw(DisasContext *ctx, arg_flw *a)
 {
     TCGv_i64 dest;
-    TCGv addr;
-
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVF);
 
-    addr = get_gpr(ctx, a->rs1, EXT_NONE);
-    if (a->imm) {
-        TCGv temp = temp_new(ctx);
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
-
     dest = cpu_fpr[a->rd];
     tcg_gen_qemu_ld_i64(dest, addr, ctx->mem_idx, MO_TEUL);
     gen_nanbox_s(dest, dest);
@@ -49,21 +40,11 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
 
 static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
 {
-    TCGv addr;
-
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVF);
 
-    addr = get_gpr(ctx, a->rs1, EXT_NONE);
-    if (a->imm) {
-        TCGv temp = tcg_temp_new();
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
-
     tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, MO_TEUL);
-
     return true;
 }
 
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index e51dbc41c5..7a0b037594 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -137,14 +137,7 @@ static bool trans_bgeu(DisasContext *ctx, arg_bgeu *a)
 static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
 {
     TCGv dest = dest_gpr(ctx, a->rd);
-    TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
-
-    if (a->imm) {
-        TCGv temp = temp_new(ctx);
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
 
     tcg_gen_qemu_ld_tl(dest, addr, ctx->mem_idx, memop);
     gen_set_gpr(ctx, a->rd, dest);
@@ -178,16 +171,9 @@ static bool trans_lhu(DisasContext *ctx, arg_lhu *a)
 
 static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
 {
-    TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
+    TCGv addr = get_address(ctx, a->rs1, a->imm);
     TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
 
-    if (a->imm) {
-        TCGv temp = temp_new(ctx);
-        tcg_gen_addi_tl(temp, addr, a->imm);
-        addr = temp;
-    }
-    addr = gen_pm_adjust_address(ctx, addr);
-
     tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop);
     return true;
 }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index a6a73ced9e..f52f6ef246 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -303,6 +303,19 @@ static TCGv gen_pm_adjust_address(DisasContext *s, TCGv src)
     }
 }
 
+static TCGv get_address(DisasContext *ctx, int rs1, int imm)
+{
+    TCGv addr = temp_new(ctx);
+    TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
+
+    tcg_gen_addi_tl(addr, src1, imm);
+    addr = gen_pm_adjust_address(ctx, addr);
+    if (get_xl(ctx) == MXL_RV32) {
+        tcg_gen_ext32u_tl(addr, addr);
+    }
+    return addr;
+}
+
 #ifndef CONFIG_USER_ONLY
 /* The states of mstatus_fs are:
  * 0 = disabled, 1 = initial, 2 = clean, 3 = dirty
-- 
2.25.1

Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/insn_trans/trans_rvd.c.inc | 23 ++---------------------
>   target/riscv/insn_trans/trans_rvf.c.inc | 23 ++---------------------
>   target/riscv/insn_trans/trans_rvi.c.inc | 18 ++----------------
>   target/riscv/translate.c                | 13 +++++++++++++
>   4 files changed, 19 insertions(+), 58 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 64fb0046f7..29066a8ef3 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -20,19 +20,10 @@
>   
>   static bool trans_fld(DisasContext *ctx, arg_fld *a)
>   {
> -    TCGv addr;
> -
> +    TCGv addr = get_address(ctx, a->rs1, a->imm);
>       REQUIRE_FPU;
>       REQUIRE_EXT(ctx, RVD);

It would be better to leave the address calculation after the REQUIRE checks.

> +static TCGv get_address(DisasContext *ctx, int rs1, int imm)
> +{
> +    TCGv addr = temp_new(ctx);
> +    TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
> +
> +    tcg_gen_addi_tl(addr, src1, imm);
> +    addr = gen_pm_adjust_address(ctx, addr);
> +    if (get_xl(ctx) == MXL_RV32) {
> +        tcg_gen_ext32u_tl(addr, addr);
> +    }
> +    return addr;
> +}

I suspect the extend should come before the pointer mask and not after, but this is is a 
weakness in the current RVJ spec that it does not specify how the extension interacts with 
UXL.  (The reverse ordering would allow a 64-bit os to place a 32-bit application at a 
base address above 4gb, which could allow address separation without paging enabled.)

I do think we should merge gen_pm_adjust_address into this function so that we only create 
one new temporary.


r~

Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[LIU Zhiwei]

On 2021/11/10 下午6:52, Richard Henderson wrote:
> On 11/10/21 8:04 AM, LIU Zhiwei wrote:
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/insn_trans/trans_rvd.c.inc | 23 ++---------------------
>>   target/riscv/insn_trans/trans_rvf.c.inc | 23 ++---------------------
>>   target/riscv/insn_trans/trans_rvi.c.inc | 18 ++----------------
>>   target/riscv/translate.c                | 13 +++++++++++++
>>   4 files changed, 19 insertions(+), 58 deletions(-)
>>
>> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
>> b/target/riscv/insn_trans/trans_rvd.c.inc
>> index 64fb0046f7..29066a8ef3 100644
>> --- a/target/riscv/insn_trans/trans_rvd.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
>> @@ -20,19 +20,10 @@
>>     static bool trans_fld(DisasContext *ctx, arg_fld *a)
>>   {
>> -    TCGv addr;
>> -
>> +    TCGv addr = get_address(ctx, a->rs1, a->imm);
>>       REQUIRE_FPU;
>>       REQUIRE_EXT(ctx, RVD);
>
> It would be better to leave the address calculation after the REQUIRE 
> checks.
OK.
>
>> +static TCGv get_address(DisasContext *ctx, int rs1, int imm)
>> +{
>> +    TCGv addr = temp_new(ctx);
>> +    TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
>> +
>> +    tcg_gen_addi_tl(addr, src1, imm);
>> +    addr = gen_pm_adjust_address(ctx, addr);
>> +    if (get_xl(ctx) == MXL_RV32) {
>> +        tcg_gen_ext32u_tl(addr, addr);
>> +    }
>> +    return addr;
>> +}
>
> I suspect the extend should come before the pointer mask and not 
> after, but this is is a weakness in the current RVJ spec that it does 
> not specify how the extension interacts with UXL.  (The reverse 
> ordering would allow a 64-bit os to place a 32-bit application at a 
> base address above 4gb, which could allow address separation without 
> paging enabled.)

Agree. Should we adjust currently, or just add a TODO comment here?

>
> I do think we should merge gen_pm_adjust_address into this function so 
> that we only create one new temporary.

I think custom instructions will be added in the future. And possibly 
there will be  some custom load/store instructions.
If we merge gen_pm_adjust_address,  we may have to split it once again 
at that time.

Thanks,
Zhiwei

>
>
> r~

Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[Richard Henderson]

On 11/10/21 2:44 PM, LIU Zhiwei wrote:
>> I suspect the extend should come before the pointer mask and not after, but this is is a 
>> weakness in the current RVJ spec that it does not specify how the extension interacts 
>> with UXL.  (The reverse ordering would allow a 64-bit os to place a 32-bit application 
>> at a base address above 4gb, which could allow address separation without paging enabled.)
> 
> Agree. Should we adjust currently, or just add a TODO comment here?

Let's add a todo comment for sure.

>> I do think we should merge gen_pm_adjust_address into this function so that we only 
>> create one new temporary.
> 
> I think custom instructions will be added in the future. And possibly there will be  some 
> custom load/store instructions.
> If we merge gen_pm_adjust_address,  we may have to split it once again at that time.

I don't think so.  We're simply having one function to compute a canonical address from a 
register plus offset plus mods.

Also, patch 10 combines pm-mask with zero-extension, so we shouldn't need to do both here. 
  The checks should be combined like

     tcg_gen_addi_tl(addr, src1, imm);
     if (ctx->pm_enabled) {
         tcg_gen_and_tl(addr, addr, pm_mask);
         tcg_gen_or_tl(addr, addr, pm_base);
     } else if (get_xl(ctx) == MXL_RV32) {
         tcg_gen_ext32u_tl(addr, addr);
     }

and could possibly be extended to

     if (ctx->pm_mask_enabled) {
         tcg_gen_and_tl(addr, addr, pm_mask);
     } else if (get_xl(ctx) == MXL_RV32) {
         tcg_gen_ext32u_tl(addr, addr);
     }
     if (ctx->pm_base_enabled) {
         tcg_gen_or_tl(addr, addr, pm_base);
     }

with one more bit in TB_FLAGS, e.g.

     if (env->cur_pm_mask < (xl == MVL_RV32 ? UINT32_MAX : UINT64_MAX)) {
         flags = FIELD_DP32(flags, TB_FLAGS, PM_MASK_ENABLED, 1);
     }
     if (env->cur_pm_base != 0) {
         flags = FIELD_DP32(flags, TB_FLAGS, PM_BASE_ENABLED, 1);
     }


r~

Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[LIU Zhiwei]

On 2021/11/10 下午10:40, Richard Henderson wrote:
> On 11/10/21 2:44 PM, LIU Zhiwei wrote:
>>> I suspect the extend should come before the pointer mask and not 
>>> after, but this is is a weakness in the current RVJ spec that it 
>>> does not specify how the extension interacts with UXL.  (The reverse 
>>> ordering would allow a 64-bit os to place a 32-bit application at a 
>>> base address above 4gb, which could allow address separation without 
>>> paging enabled.)
>>
>> Agree. Should we adjust currently, or just add a TODO comment here?
>
> Let's add a todo comment for sure.
>
>>> I do think we should merge gen_pm_adjust_address into this function 
>>> so that we only create one new temporary.
>>
>> I think custom instructions will be added in the future. And possibly 
>> there will be  some custom load/store instructions.
>> If we merge gen_pm_adjust_address,  we may have to split it once 
>> again at that time.
>
> I don't think so.  We're simply having one function to compute a 
> canonical address from a register plus offset plus mods.
>
> Also, patch 10 combines pm-mask with zero-extension, so we shouldn't 
> need to do both here.  The checks should be combined like
>
>     tcg_gen_addi_tl(addr, src1, imm);
>     if (ctx->pm_enabled) {
>         tcg_gen_and_tl(addr, addr, pm_mask);
>         tcg_gen_or_tl(addr, addr, pm_base);
>     } else if (get_xl(ctx) == MXL_RV32) {
>         tcg_gen_ext32u_tl(addr, addr);
>     }
>
> and could possibly be extended to
>
>     if (ctx->pm_mask_enabled) {
>         tcg_gen_and_tl(addr, addr, pm_mask);
>     } else if (get_xl(ctx) == MXL_RV32) {
>         tcg_gen_ext32u_tl(addr, addr);
>     }
>     if (ctx->pm_base_enabled) {
>         tcg_gen_or_tl(addr, addr, pm_base);
>     }
>
Can we just use

  tcg_gen_and_tl(addr, addr, pm_mask);
  tcg_gen_or_tl(addr, addr, pm_base);

Therefore we can remove all PM flags in TB_FLAGS.

Thanks,
Zhiwei

> with one more bit in TB_FLAGS, e.g.
>
>     if (env->cur_pm_mask < (xl == MVL_RV32 ? UINT32_MAX : UINT64_MAX)) {
>         flags = FIELD_DP32(flags, TB_FLAGS, PM_MASK_ENABLED, 1);
>     }
>     if (env->cur_pm_base != 0) {
>         flags = FIELD_DP32(flags, TB_FLAGS, PM_BASE_ENABLED, 1);
>     }
>
>
> r~

[PATCH v2 06/14] target/riscv: Adjust vsetvl according to XLEN

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.h                      |  2 ++
 target/riscv/helper.h                   |  2 +-
 target/riscv/insn_trans/trans_rvv.c.inc |  4 ++--
 target/riscv/vector_helper.c            | 19 +++++++++++++++----
 4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8befff0166..11590a510e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -107,6 +107,8 @@ FIELD(VTYPE, VSEW, 2, 3)
 FIELD(VTYPE, VEDIV, 5, 2)
 FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
 FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
+FIELD(VTYPE, RESERVED_XLEN32, 7, 23)
+FIELD(VTYPE, VILL_XLEN32, 31, 1)
 
 struct CPURISCVState {
     target_ulong gpr[32];
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index c7a5376227..e198d43981 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -82,7 +82,7 @@ DEF_HELPER_2(hyp_hlvx_wu, tl, env, tl)
 #endif
 
 /* Vector functions */
-DEF_HELPER_3(vsetvl, tl, env, tl, tl)
+DEF_HELPER_4(vsetvl, tl, env, tl, tl, tl)
 DEF_HELPER_5(vlb_v_b, void, ptr, ptr, tl, env, i32)
 DEF_HELPER_5(vlb_v_b_mask, void, ptr, ptr, tl, env, i32)
 DEF_HELPER_5(vlb_v_h, void, ptr, ptr, tl, env, i32)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 17ee3babef..f5aabd5263 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -37,7 +37,7 @@ static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl *a)
     } else {
         s1 = get_gpr(ctx, a->rs1, EXT_ZERO);
     }
-    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_helper_vsetvl(dst, cpu_env, s1, s2, tcg_constant_tl(get_xlen(ctx)));
     gen_set_gpr(ctx, a->rd, dst);
 
     tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
@@ -64,7 +64,7 @@ static bool trans_vsetvli(DisasContext *ctx, arg_vsetvli *a)
     } else {
         s1 = get_gpr(ctx, a->rs1, EXT_ZERO);
     }
-    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_helper_vsetvl(dst, cpu_env, s1, s2, tcg_constant_tl(get_xlen(ctx)));
     gen_set_gpr(ctx, a->rd, dst);
 
     gen_goto_tb(ctx, 0, ctx->pc_succ_insn);
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 12c31aa4b4..cb6fa8718d 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -27,18 +27,29 @@
 #include <math.h>
 
 target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
-                            target_ulong s2)
+                            target_ulong s2, target_ulong xlen)
 {
     int vlmax, vl;
     RISCVCPU *cpu = env_archcpu(env);
     uint16_t sew = 8 << FIELD_EX64(s2, VTYPE, VSEW);
     uint8_t ediv = FIELD_EX64(s2, VTYPE, VEDIV);
-    bool vill = FIELD_EX64(s2, VTYPE, VILL);
-    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
+    bool vill;
+    target_ulong reserved;
 
+    if (xlen < TARGET_LONG_BITS) {
+        vill = FIELD_EX64(s2, VTYPE, VILL_XLEN32);
+        reserved = FIELD_EX64(s2, VTYPE, RESERVED_XLEN32);
+    } else {
+        vill = FIELD_EX64(s2, VTYPE, VILL);
+        reserved = FIELD_EX64(s2, VTYPE, RESERVED);
+    }
     if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
         /* only set vill bit. */
-        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        if (xlen < TARGET_LONG_BITS) {
+            env->vtype = FIELD_DP64(0, VTYPE, VILL_XLEN32, 1);
+        } else {
+            env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        }
         env->vl = 0;
         env->vstart = 0;
         return 0;
-- 
2.25.1

[PATCH v2 07/14] target/riscv: Ajdust vector atomic check with XLEN

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index f5aabd5263..41c7c88904 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -739,7 +739,8 @@ static bool amo_check(DisasContext *s, arg_rwdvm* a)
             (!a->wd || vext_check_overlap_mask(s, a->rd, a->vm, false)) &&
             vext_check_reg(s, a->rd, false) &&
             vext_check_reg(s, a->rs2, false) &&
-            ((1 << s->sew) <= sizeof(target_ulong)) &&
+            /* TODO: RV128 could allow 128-bit atomics */
+            ((1 << s->sew) <=  (get_xl(s) == MXL_RV32 ? 4 : 8)) &&
             ((1 << s->sew) >= 4));
 }
 
-- 
2.25.1

Re: [PATCH v2 07/14] target/riscv: Ajdust vector atomic check with XLEN

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

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

r~

[PATCH v2 08/14] target/riscv: Fix check range for first fault only

[LIU Zhiwei]

Only check the range that has passed the address translation.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/vector_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index cb6fa8718d..60006b1b1b 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -638,12 +638,12 @@ vext_ldff(void *vd, void *v0, target_ulong base,
                                          cpu_mmu_index(env, false));
                 if (host) {
 #ifdef CONFIG_USER_ONLY
-                    if (page_check_range(addr, nf * msz, PAGE_READ) < 0) {
+                    if (page_check_range(addr, offset, PAGE_READ) < 0) {
                         vl = i;
                         goto ProbeSuccess;
                     }
 #else
-                    probe_pages(env, addr, nf * msz, ra, MMU_DATA_LOAD);
+                    probe_pages(env, addr, offset, ra, MMU_DATA_LOAD);
 #endif
                 } else {
                     vl = i;
-- 
2.25.1

[PATCH v2 09/14] target/riscv: Relax debug check for pm write

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/csr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9f41954894..74c0b788fd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1445,6 +1445,9 @@ static bool check_pm_current_disabled(CPURISCVState *env, int csrno)
     int csr_priv = get_field(csrno, 0x300);
     int pm_current;
 
+    if (env->debugger) {
+        return false;
+    }
     /*
      * If priv lvls differ that means we're accessing csr from higher priv lvl,
      * so allow the access
-- 
2.25.1

Re: [PATCH v2 09/14] target/riscv: Relax debug check for pm write

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/csr.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9f41954894..74c0b788fd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1445,6 +1445,9 @@ static bool check_pm_current_disabled(CPURISCVState *env, int csrno)
>       int csr_priv = get_field(csrno, 0x300);
>       int pm_current;
>   
> +    if (env->debugger) {
> +        return false;
> +    }
>       /*
>        * If priv lvls differ that means we're accessing csr from higher priv lvl,
>        * so allow the access

Seems reasonable.

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


r~

[PATCH v2 10/14] target/riscv: Adjust vector address with mask

[LIU Zhiwei]

The mask comes from the pointer masking extension, or the max value
corresponding to XLEN bits.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.c           |  1 +
 target/riscv/cpu.h           |  4 ++++
 target/riscv/cpu_helper.c    | 40 ++++++++++++++++++++++++++++++++++++
 target/riscv/csr.c           | 19 +++++++++++++++++
 target/riscv/machine.c       | 10 +++++++++
 target/riscv/vector_helper.c | 23 +++++++++++++--------
 6 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0d2d175fa2..886388f066 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -378,6 +378,7 @@ static void riscv_cpu_reset(DeviceState *dev)
 #ifndef CONFIG_USER_ONLY
     env->misa_mxl = env->misa_mxl_max;
     env->priv = PRV_M;
+    riscv_cpu_update_mask(env);
     env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
     if (env->misa_mxl > MXL_RV32) {
         /*
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 11590a510e..73d7aa9ad7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -252,6 +252,8 @@ struct CPURISCVState {
     target_ulong upmmask;
     target_ulong upmbase;
 #endif
+    target_ulong mask;
+    target_ulong base;
 
     float_status fp_status;
 
@@ -443,6 +445,8 @@ static inline uint32_t vext_get_vlmax(RISCVCPU *cpu, target_ulong vtype)
 void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
                           target_ulong *cs_base, uint32_t *pflags);
 
+void riscv_cpu_update_mask(CPURISCVState *env);
+
 RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
                            target_ulong *ret_value,
                            target_ulong new_value, target_ulong write_mask);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 79aba9c880..d1ecdea392 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -133,6 +133,46 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
     *pflags = flags;
 }
 
+void riscv_cpu_update_mask(CPURISCVState *env)
+{
+    target_ulong mask = -1, base = 0;
+#ifndef CONFIG_USER_ONLY
+    if (riscv_has_ext(env, RVJ)) {
+        switch (env->priv) {
+        case PRV_M:
+            if (env->mmte & M_PM_ENABLE) {
+                mask = env->mpmmask;
+                base = env->mpmbase;
+            }
+            break;
+        case PRV_S:
+            if (env->mmte & S_PM_ENABLE) {
+                mask = env->spmmask;
+                base = env->spmbase;
+            }
+            break;
+        case PRV_U:
+            if (env->mmte & U_PM_ENABLE) {
+                mask = env->upmmask;
+                base = env->upmbase;
+            }
+            break;
+        default:
+            g_assert_not_reached();
+        }
+    }
+#endif
+    if (cpu_get_xl(env) == MXL_RV32) {
+        env->mask = mask & UINT32_MAX;
+        env->base = base & UINT32_MAX;
+    } else {
+        env->mask = mask;
+        env->base = base;
+    }
+}
+
+
+
 #ifndef CONFIG_USER_ONLY
 static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 74c0b788fd..59e368f004 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1496,6 +1496,7 @@ static RISCVException write_mmte(CPURISCVState *env, int csrno,
     /* hardwiring pm.instruction bit to 0, since it's not supported yet */
     wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
     env->mmte = wpri_val | PM_EXT_DIRTY;
+    riscv_cpu_update_mask(env);
 
     /* Set XS and SD bits, since PM CSRs are dirty */
     mstatus = env->mstatus | MSTATUS_XS;
@@ -1571,6 +1572,9 @@ static RISCVException write_mpmmask(CPURISCVState *env, int csrno,
     uint64_t mstatus;
 
     env->mpmmask = val;
+    if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
+        env->mask = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
@@ -1596,6 +1600,9 @@ static RISCVException write_spmmask(CPURISCVState *env, int csrno,
         return RISCV_EXCP_NONE;
     }
     env->spmmask = val;
+    if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
+        env->mask = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
@@ -1621,6 +1628,9 @@ static RISCVException write_upmmask(CPURISCVState *env, int csrno,
         return RISCV_EXCP_NONE;
     }
     env->upmmask = val;
+    if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
+        env->mask = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
@@ -1642,6 +1652,9 @@ static RISCVException write_mpmbase(CPURISCVState *env, int csrno,
     uint64_t mstatus;
 
     env->mpmbase = val;
+    if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
+        env->base = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
@@ -1667,6 +1680,9 @@ static RISCVException write_spmbase(CPURISCVState *env, int csrno,
         return RISCV_EXCP_NONE;
     }
     env->spmbase = val;
+    if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
+        env->base = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
@@ -1692,6 +1708,9 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
         return RISCV_EXCP_NONE;
     }
     env->upmbase = val;
+    if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
+        env->base = val;
+    }
     env->mmte |= PM_EXT_DIRTY;
 
     /* Set XS and SD bits, since PM CSRs are dirty */
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 7b4c739564..19e982d3f0 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -164,10 +164,20 @@ static const VMStateDescription vmstate_hyper = {
     }
 };
 
+static int riscv_cpu_post_load(void *opaque, int version_id)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    riscv_cpu_update_mask(env);
+    return 0;
+}
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 3,
     .minimum_version_id = 3,
+    .post_load = riscv_cpu_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
         VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32),
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 60006b1b1b..0b297f6bc8 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -123,6 +123,11 @@ static inline uint32_t vext_maxsz(uint32_t desc)
     return simd_maxsz(desc) << vext_lmul(desc);
 }
 
+static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
+{
+    return (addr & env->mask) | env->base;
+}
+
 /*
  * This function checks watchpoint before real load operation.
  *
@@ -140,12 +145,12 @@ static void probe_pages(CPURISCVState *env, target_ulong addr,
     target_ulong pagelen = -(addr | TARGET_PAGE_MASK);
     target_ulong curlen = MIN(pagelen, len);
 
-    probe_access(env, addr, curlen, access_type,
+    probe_access(env, adjust_addr(env, addr), curlen, access_type,
                  cpu_mmu_index(env, false), ra);
     if (len > curlen) {
         addr += curlen;
         curlen = len - curlen;
-        probe_access(env, addr, curlen, access_type,
+        probe_access(env, adjust_addr(env, addr), curlen, access_type,
                      cpu_mmu_index(env, false), ra);
     }
 }
@@ -306,7 +311,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
         }
         while (k < nf) {
             target_ulong addr = base + stride * i + k * msz;
-            ldst_elem(env, addr, i + k * vlmax, vd, ra);
+            ldst_elem(env, adjust_addr(env, addr), i + k * vlmax, vd, ra);
             k++;
         }
     }
@@ -399,7 +404,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
         k = 0;
         while (k < nf) {
             target_ulong addr = base + (i * nf + k) * msz;
-            ldst_elem(env, addr, i + k * vlmax, vd, ra);
+            ldst_elem(env, adjust_addr(env, addr), i + k * vlmax, vd, ra);
             k++;
         }
     }
@@ -536,7 +541,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
         }
         while (k < nf) {
             abi_ptr addr = get_index_addr(base, i, vs2) + k * msz;
-            ldst_elem(env, addr, i + k * vlmax, vd, ra);
+            ldst_elem(env, adjust_addr(env, addr), i + k * vlmax, vd, ra);
             k++;
         }
     }
@@ -626,7 +631,7 @@ vext_ldff(void *vd, void *v0, target_ulong base,
         if (!vm && !vext_elem_mask(v0, mlen, i)) {
             continue;
         }
-        addr = base + nf * i * msz;
+        addr = adjust_addr(env, base + nf * i * msz);
         if (i == 0) {
             probe_pages(env, addr, nf * msz, ra, MMU_DATA_LOAD);
         } else {
@@ -653,7 +658,7 @@ vext_ldff(void *vd, void *v0, target_ulong base,
                     break;
                 }
                 remain -= offset;
-                addr += offset;
+                addr = adjust_addr(env, addr + offset);
             }
         }
     }
@@ -669,7 +674,7 @@ ProbeSuccess:
         }
         while (k < nf) {
             target_ulong addr = base + (i * nf + k) * msz;
-            ldst_elem(env, addr, i + k * vlmax, vd, ra);
+            ldst_elem(env, adjust_addr(env, addr), i + k * vlmax, vd, ra);
             k++;
         }
     }
@@ -808,7 +813,7 @@ vext_amo_noatomic(void *vs3, void *v0, target_ulong base,
             continue;
         }
         addr = get_index_addr(base, i, vs2);
-        noatomic_op(vs3, addr, wd, i, env, ra);
+        noatomic_op(vs3, adjust_addr(env, addr), wd, i, env, ra);
     }
     clear_elem(vs3, env->vl, env->vl * esz, vlmax * esz);
 }
-- 
2.25.1

Re: [PATCH v2 10/14] target/riscv: Adjust vector address with mask

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> The mask comes from the pointer masking extension, or the max value
> corresponding to XLEN bits.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/cpu.c           |  1 +
>   target/riscv/cpu.h           |  4 ++++
>   target/riscv/cpu_helper.c    | 40 ++++++++++++++++++++++++++++++++++++
>   target/riscv/csr.c           | 19 +++++++++++++++++
>   target/riscv/machine.c       | 10 +++++++++
>   target/riscv/vector_helper.c | 23 +++++++++++++--------
>   6 files changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0d2d175fa2..886388f066 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -378,6 +378,7 @@ static void riscv_cpu_reset(DeviceState *dev)
>   #ifndef CONFIG_USER_ONLY
>       env->misa_mxl = env->misa_mxl_max;
>       env->priv = PRV_M;
> +    riscv_cpu_update_mask(env);
>       env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
>       if (env->misa_mxl > MXL_RV32) {
>           /*
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 11590a510e..73d7aa9ad7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -252,6 +252,8 @@ struct CPURISCVState {
>       target_ulong upmmask;
>       target_ulong upmbase;
>   #endif
> +    target_ulong mask;
> +    target_ulong base;

I think the name here isn't great.  Without the context of the preceeding elements, the 
question becomes: mask of what?

Better might be cur_pmmask, cur_pmbase.

Broader than that, you're doing too many things in this patch.  The subject is "adjust 
vector address with mask", but you're also creating new fields and updating them at priv 
changes, etc.  Too much.

> +void riscv_cpu_update_mask(CPURISCVState *env)

... update_pmmask?

> +}
> +
> +
> +

Watch the extra spaces.

> @@ -1571,6 +1572,9 @@ static RISCVException write_mpmmask(CPURISCVState *env, int csrno,
>       uint64_t mstatus;
>   
>       env->mpmmask = val;
> +    if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
> +        env->mask = val;
> +    }

This needs to use the function; there are pieces missing here, notably the zero-extend for 
RV32.

I don't see any updates to the exception entry and exception return paths.

You'll want to update the translator to use these new fields instead of using the 
[msu]pmmask / [msu]pmbase fields directly.  (Which means that we will have fewer tcg 
variables, and need not copy the "current" into DisasContext.)


> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 60006b1b1b..0b297f6bc8 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -123,6 +123,11 @@ static inline uint32_t vext_maxsz(uint32_t desc)
>       return simd_maxsz(desc) << vext_lmul(desc);
>   }
>   
> +static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
> +{
> +    return (addr & env->mask) | env->base;
> +}

The code here in vector_helper.c looks fine as a patch by itself, under the subject that 
you have given.


r~

Re: [PATCH v2 10/14] target/riscv: Adjust vector address with mask

[LIU Zhiwei]

On 2021/11/10 下午7:11, Richard Henderson wrote:
> On 11/10/21 8:04 AM, LIU Zhiwei wrote:
>> The mask comes from the pointer masking extension, or the max value
>> corresponding to XLEN bits.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/cpu.c           |  1 +
>>   target/riscv/cpu.h           |  4 ++++
>>   target/riscv/cpu_helper.c    | 40 ++++++++++++++++++++++++++++++++++++
>>   target/riscv/csr.c           | 19 +++++++++++++++++
>>   target/riscv/machine.c       | 10 +++++++++
>>   target/riscv/vector_helper.c | 23 +++++++++++++--------
>>   6 files changed, 88 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 0d2d175fa2..886388f066 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -378,6 +378,7 @@ static void riscv_cpu_reset(DeviceState *dev)
>>   #ifndef CONFIG_USER_ONLY
>>       env->misa_mxl = env->misa_mxl_max;
>>       env->priv = PRV_M;
>> +    riscv_cpu_update_mask(env);
>>       env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
>>       if (env->misa_mxl > MXL_RV32) {
>>           /*
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 11590a510e..73d7aa9ad7 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -252,6 +252,8 @@ struct CPURISCVState {
>>       target_ulong upmmask;
>>       target_ulong upmbase;
>>   #endif
>> +    target_ulong mask;
>> +    target_ulong base;
>
> I think the name here isn't great.  Without the context of the 
> preceeding elements, the question becomes: mask of what?
>
> Better might be cur_pmmask, cur_pmbase.
>
> Broader than that, you're doing too many things in this patch. The 
> subject is "adjust vector address with mask", but you're also creating 
> new fields and updating them at priv changes, etc.  Too much.
>
>> +void riscv_cpu_update_mask(CPURISCVState *env)
>
> ... update_pmmask?
>
>> +}
>> +
>> +
>> +
>
> Watch the extra spaces.
>
>> @@ -1571,6 +1572,9 @@ static RISCVException 
>> write_mpmmask(CPURISCVState *env, int csrno,
>>       uint64_t mstatus;
>>         env->mpmmask = val;
>> +    if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
>> +        env->mask = val;
>> +    }
>
> This needs to use the function; there are pieces missing here, notably 
> the zero-extend for RV32.

Zero-extend has been done automatically here, as these operations are in 
csr instruction.

In my opinion, it is enough here to check env->priv and pm_enable.

>
> I don't see any updates to the exception entry and exception return 
> paths.

Oops. I forgot this.  We should update at these places. Exception entry 
and exception return will call one same function
to change privilege,  we can update it there.

>
> You'll want to update the translator to use these new fields instead 
> of using the [msu]pmmask / [msu]pmbase fields directly. (Which means 
> that we will have fewer tcg variables, and need not copy the "current" 
> into DisasContext.)
>
Do you mean we can remove the global TCG variables pm_mask[] and pc_base[]?
If then how to transport env->cur_pmmask and env->cur_pmbase to 
DisasContext?

Thanks,
Zhiwei

>
>> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
>> index 60006b1b1b..0b297f6bc8 100644
>> --- a/target/riscv/vector_helper.c
>> +++ b/target/riscv/vector_helper.c
>> @@ -123,6 +123,11 @@ static inline uint32_t vext_maxsz(uint32_t desc)
>>       return simd_maxsz(desc) << vext_lmul(desc);
>>   }
>>   +static inline target_ulong adjust_addr(CPURISCVState *env, 
>> target_ulong addr)
>> +{
>> +    return (addr & env->mask) | env->base;
>> +}
>
> The code here in vector_helper.c looks fine as a patch by itself, 
> under the subject that you have given.
>
>
> r~

Re: [PATCH v2 10/14] target/riscv: Adjust vector address with mask

[Richard Henderson]

On 11/10/21 3:08 PM, LIU Zhiwei wrote:
>> You'll want to update the translator to use these new fields instead of using the 
>> [msu]pmmask / [msu]pmbase fields directly. (Which means that we will have fewer tcg 
>> variables, and need not copy the "current" into DisasContext.)
>>
> Do you mean we can remove the global TCG variables pm_mask[] and pc_base[]?
> If then how to transport env->cur_pmmask and env->cur_pmbase to DisasContext?

I mean replace the array of pm_mask/pm_base with scalar variables. Remove the cached array 
value in DisasContext, and use global variables for the tcg variables like we do for 
everything else.


r~

[PATCH v2 11/14] target/riscv: Adjust scalar reg in vector with XLEN

[LIU Zhiwei]

When sew <= 32bits, not need to extend scalar reg.
When sew > 32bits, if xlen is less that sew, we should sign extend
the scalar register, except explicitly specified by the spec.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc |  5 +++--
 target/riscv/internals.h                |  1 +
 target/riscv/vector_helper.c            | 11 +++++++++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 41c7c88904..0a956cac5b 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -846,7 +846,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, uint32_t vm,
     dest = tcg_temp_new_ptr();
     mask = tcg_temp_new_ptr();
     src2 = tcg_temp_new_ptr();
-    src1 = get_gpr(s, rs1, EXT_NONE);
+    src1 = get_gpr(s, rs1, EXT_SIGN);
 
     data = FIELD_DP32(data, VDATA, MLEN, s->mlen);
     data = FIELD_DP32(data, VDATA, VM, vm);
@@ -2670,6 +2670,7 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
         /* This instruction ignores LMUL and vector register groups */
         int maxsz = s->vlen >> 3;
         TCGv_i64 t1;
+        TCGv src1 = get_gpr(s, a->rs1, EXT_ZERO);
         TCGLabel *over = gen_new_label();
 
         tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
@@ -2679,7 +2680,7 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
         }
 
         t1 = tcg_temp_new_i64();
-        tcg_gen_extu_tl_i64(t1, cpu_gpr[a->rs1]);
+        tcg_gen_extu_tl_i64(t1, src1);
         vec_element_storei(s, a->rd, 0, t1);
         tcg_temp_free_i64(t1);
     done:
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index b15ad394bb..07e882160d 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -27,6 +27,7 @@ FIELD(VDATA, VM, 8, 1)
 FIELD(VDATA, LMUL, 9, 2)
 FIELD(VDATA, NF, 11, 4)
 FIELD(VDATA, WD, 11, 1)
+FIELD(VDATA, TRUNC, 15, 1)
 
 /* float point classify helpers */
 target_ulong fclass_h(uint64_t frs1);
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 0b297f6bc8..51bcf63d65 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -112,6 +112,11 @@ static uint32_t vext_wd(uint32_t desc)
     return (simd_data(desc) >> 11) & 0x1;
 }
 
+static inline bool vext_trunc(uint32_t desc)
+{
+    return FIELD_EX32(simd_data(desc), VDATA, TRUNC);
+}
+
 /*
  * Get vector group length in bytes. Its range is [64, 2048].
  *
@@ -4748,6 +4753,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
     uint32_t mlen = vext_mlen(desc);                                      \
     uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen;                   \
     uint32_t vm = vext_vm(desc);                                          \
+    bool trunc = vext_trunc(desc);                                        \
     uint32_t vl = env->vl;                                                \
     uint32_t i;                                                           \
                                                                           \
@@ -4756,7 +4762,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
             continue;                                                     \
         }                                                                 \
         if (i == 0) {                                                     \
-            *((ETYPE *)vd + H(i)) = s1;                                   \
+            *((ETYPE *)vd + H(i)) = trunc ? (s1 & UINT32_MAX) : s1;       \
         } else {                                                          \
             *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - 1));           \
         }                                                                 \
@@ -4777,6 +4783,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
     uint32_t mlen = vext_mlen(desc);                                      \
     uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen;                   \
     uint32_t vm = vext_vm(desc);                                          \
+    bool trunc = vext_trunc(desc);                                        \
     uint32_t vl = env->vl;                                                \
     uint32_t i;                                                           \
                                                                           \
@@ -4785,7 +4792,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
             continue;                                                     \
         }                                                                 \
         if (i == vl - 1) {                                                \
-            *((ETYPE *)vd + H(i)) = s1;                                   \
+            *((ETYPE *)vd + H(i)) = trunc ? (s1 & UINT32_MAX) : s1;       \
         } else {                                                          \
             *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i + 1));           \
         }                                                                 \
-- 
2.25.1

Re: [PATCH v2 11/14] target/riscv: Adjust scalar reg in vector with XLEN

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index b15ad394bb..07e882160d 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -27,6 +27,7 @@ FIELD(VDATA, VM, 8, 1)
>   FIELD(VDATA, LMUL, 9, 2)
>   FIELD(VDATA, NF, 11, 4)
>   FIELD(VDATA, WD, 11, 1)
> +FIELD(VDATA, TRUNC, 15, 1)

No need for this.

> @@ -4756,7 +4762,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
>               continue;                                                     \
>           }                                                                 \
>           if (i == 0) {                                                     \
> -            *((ETYPE *)vd + H(i)) = s1;                                   \
> +            *((ETYPE *)vd + H(i)) = trunc ? (s1 & UINT32_MAX) : s1;       \
>           } else {                                                          \
>               *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - 1));           \
>           }                                                                 \
...
> @@ -4785,7 +4792,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2,         \
>               continue;                                                     \
>           }                                                                 \
>           if (i == vl - 1) {                                                \
> -            *((ETYPE *)vd + H(i)) = s1;                                   \
> +            *((ETYPE *)vd + H(i)) = trunc ? (s1 & UINT32_MAX) : s1;       \

In both of these cases you can simply zero-extend s1 in the translator before passing it in.


r~

[PATCH v2 12/14] target/riscv: Split out the vill from vtype

[LIU Zhiwei]

We need not specially process vtype when XLEN changes.
Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.h           |  1 +
 target/riscv/csr.c           | 15 ++++++++++++++-
 target/riscv/machine.c       |  1 +
 target/riscv/vector_helper.c |  7 ++-----
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 73d7aa9ad7..e67531deab 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -121,6 +121,7 @@ struct CPURISCVState {
     target_ulong vl;
     target_ulong vstart;
     target_ulong vtype;
+    target_ulong vill;
 
     target_ulong pc;
     target_ulong load_res;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 59e368f004..33e342f529 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -286,7 +286,20 @@ static RISCVException write_fcsr(CPURISCVState *env, int csrno,
 static RISCVException read_vtype(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
-    *val = env->vtype;
+    target_ulong vill;
+    switch (cpu_get_xl(env)) {
+    case MXL_RV32:
+        vill = env->vill << 31;
+        break;
+#ifdef TARGET_RISCV64
+    case MXL_RV64:
+        vill = env->vill << 63;
+        break;
+#endif
+    default:
+        g_assert_not_reached();
+    }
+    *val = vill | env->vtype;
     return RISCV_EXCP_NONE;
 }
 
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 19e982d3f0..cc4dda4b93 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -104,6 +104,7 @@ static const VMStateDescription vmstate_vector = {
             VMSTATE_UINTTL(env.vl, RISCVCPU),
             VMSTATE_UINTTL(env.vstart, RISCVCPU),
             VMSTATE_UINTTL(env.vtype, RISCVCPU),
+            VMSTATE_UINTTL(env.vill, RISCVCPU),
             VMSTATE_END_OF_LIST()
         }
 };
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 51bcf63d65..7d7b554789 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -45,11 +45,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
     }
     if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
         /* only set vill bit. */
-        if (xlen < TARGET_LONG_BITS) {
-            env->vtype = FIELD_DP64(0, VTYPE, VILL_XLEN32, 1);
-        } else {
-            env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
-        }
+        env->vill = 1;
+        env->vtype = 0;
         env->vl = 0;
         env->vstart = 0;
         return 0;
-- 
2.25.1

Re: [PATCH v2 12/14] target/riscv: Split out the vill from vtype

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> We need not specially process vtype when XLEN changes.
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/cpu.h           |  1 +
>   target/riscv/csr.c           | 15 ++++++++++++++-
>   target/riscv/machine.c       |  1 +
>   target/riscv/vector_helper.c |  7 ++-----
>   4 files changed, 18 insertions(+), 6 deletions(-)

This patch should come before patch 6, which is over-complicated.

> +    target_ulong vill;

This could be bool, though there's no good place to slot it that does not result in unused 
padding.  Comments should be added to show that this bit is now missing from vtype.

> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 19e982d3f0..cc4dda4b93 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -104,6 +104,7 @@ static const VMStateDescription vmstate_vector = {
>               VMSTATE_UINTTL(env.vl, RISCVCPU),
>               VMSTATE_UINTTL(env.vstart, RISCVCPU),
>               VMSTATE_UINTTL(env.vtype, RISCVCPU),
> +            VMSTATE_UINTTL(env.vill, RISCVCPU),
>               VMSTATE_END_OF_LIST()

This will need a bump to version.

> @@ -45,11 +45,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>       }
>       if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
>           /* only set vill bit. */
> -        if (xlen < TARGET_LONG_BITS) {
> -            env->vtype = FIELD_DP64(0, VTYPE, VILL_XLEN32, 1);
> -        } else {
> -            env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> -        }
> +        env->vill = 1;
> +        env->vtype = 0;

This is fine.

You're missing the updates to cpu_get_tb_cpu_state.


r~

Re: [PATCH v2 12/14] target/riscv: Split out the vill from vtype

[LIU Zhiwei]

On 2021/11/10 下午7:23, Richard Henderson wrote:
> On 11/10/21 8:04 AM, LIU Zhiwei wrote:
>> We need not specially process vtype when XLEN changes.
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/cpu.h           |  1 +
>>   target/riscv/csr.c           | 15 ++++++++++++++-
>>   target/riscv/machine.c       |  1 +
>>   target/riscv/vector_helper.c |  7 ++-----
>>   4 files changed, 18 insertions(+), 6 deletions(-)
>
> This patch should come before patch 6, which is over-complicated.

Agree.

One question here. Even come before patch 6, we don't have a simple way 
to choose vill and reserved fields from s2 register in patch 6.

>
>> +    target_ulong vill;
>
> This could be bool, though there's no good place to slot it that does 
> not result in unused padding.

As env->vill will be used in read_vtype,  we still need to covert 
env->vill type to target_ulong there.
Is there any benefit to use bool instead of target_ulong?

> Comments should be added to show that this bit is now missing from vtype.
>
>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> index 19e982d3f0..cc4dda4b93 100644
>> --- a/target/riscv/machine.c
>> +++ b/target/riscv/machine.c
>> @@ -104,6 +104,7 @@ static const VMStateDescription vmstate_vector = {
>>               VMSTATE_UINTTL(env.vl, RISCVCPU),
>>               VMSTATE_UINTTL(env.vstart, RISCVCPU),
>>               VMSTATE_UINTTL(env.vtype, RISCVCPU),
>> +            VMSTATE_UINTTL(env.vill, RISCVCPU),
>>               VMSTATE_END_OF_LIST()
>
> This will need a bump to version.
>
>> @@ -45,11 +45,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, 
>> target_ulong s1,
>>       }
>>       if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved 
>> != 0)) {
>>           /* only set vill bit. */
>> -        if (xlen < TARGET_LONG_BITS) {
>> -            env->vtype = FIELD_DP64(0, VTYPE, VILL_XLEN32, 1);
>> -        } else {
>> -            env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>> -        }
>> +        env->vill = 1;
>> +        env->vtype = 0;
>
> This is fine.
>
> You're missing the updates to cpu_get_tb_cpu_state.

Yes.

Thanks,
Zhiwei

>
>
> r~

Re: [PATCH v2 12/14] target/riscv: Split out the vill from vtype

[Richard Henderson]

On 11/10/21 3:26 PM, LIU Zhiwei wrote:
> One question here. Even come before patch 6, we don't have a simple way to choose vill and 
> reserved fields from s2 register in patch 6.

You can certainly split out vill before you create a new way to select it based on xlen. 
In fact, you *should* do that as a separate patch before extending helper_vsetvl to handle 
multiple xlen.

Note that vill is always at the msb, so it's easy to find without necessarily defining an 
XLEN32 field.

As for the reserved "field"... how about

     reserved = MAKE_64BIT_MASK(R_VTYPE_RESERVED_START,
                                xlen - 1 - R_VTYPE_RESERVED_START);
     reserved &= s2;

> As env->vill will be used in read_vtype,  we still need to covert env->vill type to 
> target_ulong there.

A cast in read_vtype will do.  Explicit casts to uint32_t/uint64_t would remove the need 
for an ifdef in that function as well.

> Is there any benefit to use bool instead of target_ulong?

Self-documentation?


r~

[PATCH v2 13/14] target/riscv: Don't save pc when exception return

[LIU Zhiwei]

As pc will be written by the xepc in exception return, just ignore
pc in translation.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/helper.h                          | 4 ++--
 target/riscv/insn_trans/trans_privileged.c.inc | 7 ++-----
 target/riscv/op_helper.c                       | 4 ++--
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index e198d43981..a67965efe5 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -67,8 +67,8 @@ DEF_HELPER_2(csrr, tl, env, int)
 DEF_HELPER_3(csrw, void, env, int, tl)
 DEF_HELPER_4(csrrw, tl, env, int, tl, tl)
 #ifndef CONFIG_USER_ONLY
-DEF_HELPER_2(sret, tl, env, tl)
-DEF_HELPER_2(mret, tl, env, tl)
+DEF_HELPER_1(sret, tl, env)
+DEF_HELPER_1(mret, tl, env)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(tlb_flush, void, env)
 #endif
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 75c6ef80a6..6077bbbf11 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -74,10 +74,8 @@ static bool trans_uret(DisasContext *ctx, arg_uret *a)
 static bool trans_sret(DisasContext *ctx, arg_sret *a)
 {
 #ifndef CONFIG_USER_ONLY
-    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
-
     if (has_ext(ctx, RVS)) {
-        gen_helper_sret(cpu_pc, cpu_env, cpu_pc);
+        gen_helper_sret(cpu_pc, cpu_env);
         tcg_gen_exit_tb(NULL, 0); /* no chaining */
         ctx->base.is_jmp = DISAS_NORETURN;
     } else {
@@ -92,8 +90,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 static bool trans_mret(DisasContext *ctx, arg_mret *a)
 {
 #ifndef CONFIG_USER_ONLY
-    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
-    gen_helper_mret(cpu_pc, cpu_env, cpu_pc);
+    gen_helper_mret(cpu_pc, cpu_env);
     tcg_gen_exit_tb(NULL, 0); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index ee7c24efe7..095d39671b 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -71,7 +71,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr,
 
 #ifndef CONFIG_USER_ONLY
 
-target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
+target_ulong helper_sret(CPURISCVState *env)
 {
     uint64_t mstatus;
     target_ulong prev_priv, prev_virt;
@@ -132,7 +132,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
     return retpc;
 }
 
-target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
+target_ulong helper_mret(CPURISCVState *env)
 {
     if (!(env->priv >= PRV_M)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
-- 
2.25.1

Re: [PATCH v2 13/14] target/riscv: Don't save pc when exception return

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> As pc will be written by the xepc in exception return, just ignore
> pc in translation.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/riscv/helper.h                          | 4 ++--
>   target/riscv/insn_trans/trans_privileged.c.inc | 7 ++-----
>   target/riscv/op_helper.c                       | 4 ++--
>   3 files changed, 6 insertions(+), 9 deletions(-)

This should be ordered before patch 1, so that it does not appear that you have missed 
places that should use gen_set_pc.


r~

[PATCH v2 14/14] target/riscv: Enable uxl field write

[LIU Zhiwei]

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/csr.c                      | 5 ++---
 target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
 target/riscv/op_helper.c                | 3 ++-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 33e342f529..e07cd522ef 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -555,15 +555,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
          * RV32: MPV and GVA are not in mstatus. The current plan is to
          * add them to mstatush. For now, we just don't support it.
          */
-        mask |= MSTATUS_MPV | MSTATUS_GVA;
+        mask |= MSTATUS_MPV | MSTATUS_GVA | MSTATUS64_UXL;
     }
 
     mstatus = (mstatus & ~mask) | (val & mask);
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
-        /* SXL and UXL fields are for now read only */
+        /* SXL fields are for now read only */
         mstatus = set_field(mstatus, MSTATUS64_SXL, MXL_RV64);
-        mstatus = set_field(mstatus, MSTATUS64_UXL, MXL_RV64);
     }
     env->mstatus = mstatus;
 
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 7a0b037594..cb73a2f1ee 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -472,7 +472,7 @@ static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
         return do_csrw(ctx, a->csr, src);
     }
 
-    TCGv mask = tcg_constant_tl(-1);
+    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX : -1);
     return do_csrrw(ctx, a->rd, a->csr, src, mask);
 }
 
@@ -523,7 +523,7 @@ static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
         return do_csrw(ctx, a->csr, src);
     }
 
-    TCGv mask = tcg_constant_tl(-1);
+    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX : -1);
     return do_csrrw(ctx, a->rd, a->csr, src, mask);
 }
 
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 095d39671b..561e156bec 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -50,7 +50,8 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
 
 void helper_csrw(CPURISCVState *env, int csr, target_ulong src)
 {
-    RISCVException ret = riscv_csrrw(env, csr, NULL, src, -1);
+    target_ulong mask = cpu_get_xl(env) == MXL_RV32 ? UINT32_MAX : -1;
+    RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask);
 
     if (ret != RISCV_EXCP_NONE) {
         riscv_raise_exception(env, ret, GETPC());
-- 
2.25.1

Re: [PATCH v2 14/14] target/riscv: Enable uxl field write

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/csr.c                      | 5 ++---
>   target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>   target/riscv/op_helper.c                | 3 ++-
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 33e342f529..e07cd522ef 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -555,15 +555,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>            * RV32: MPV and GVA are not in mstatus. The current plan is to
>            * add them to mstatush. For now, we just don't support it.
>            */
> -        mask |= MSTATUS_MPV | MSTATUS_GVA;
> +        mask |= MSTATUS_MPV | MSTATUS_GVA | MSTATUS64_UXL;
>       }
>   
>       mstatus = (mstatus & ~mask) | (val & mask);
>   
>       if (riscv_cpu_mxl(env) == MXL_RV64) {
> -        /* SXL and UXL fields are for now read only */
> +        /* SXL fields are for now read only */
>           mstatus = set_field(mstatus, MSTATUS64_SXL, MXL_RV64);
> -        mstatus = set_field(mstatus, MSTATUS64_UXL, MXL_RV64);
>       }
>       env->mstatus = mstatus;

Why do you not allow writes to SXL?

You're missing a change to write_sstatus to allow S-mode to write to UXL.

> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index 7a0b037594..cb73a2f1ee 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -472,7 +472,7 @@ static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
>           return do_csrw(ctx, a->csr, src);
>       }
>   
> -    TCGv mask = tcg_constant_tl(-1);
> +    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX : -1);
>       return do_csrrw(ctx, a->rd, a->csr, src, mask);
>   }
>   
> @@ -523,7 +523,7 @@ static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
>           return do_csrw(ctx, a->csr, src);
>       }
>   
> -    TCGv mask = tcg_constant_tl(-1);
> +    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX : -1);
>       return do_csrrw(ctx, a->rd, a->csr, src, mask);
>   }
>   
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 095d39671b..561e156bec 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -50,7 +50,8 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
>   
>   void helper_csrw(CPURISCVState *env, int csr, target_ulong src)
>   {
> -    RISCVException ret = riscv_csrrw(env, csr, NULL, src, -1);
> +    target_ulong mask = cpu_get_xl(env) == MXL_RV32 ? UINT32_MAX : -1;
> +    RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask);
>   
>       if (ret != RISCV_EXCP_NONE) {
>           riscv_raise_exception(env, ret, GETPC());
> 

The rest of this should be a separate patch.


r~

Re: [PATCH v2 14/14] target/riscv: Enable uxl field write

[LIU Zhiwei]

On 2021/11/10 下午7:27, Richard Henderson wrote:
> On 11/10/21 8:04 AM, LIU Zhiwei wrote:
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/csr.c                      | 5 ++---
>>   target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>   target/riscv/op_helper.c                | 3 ++-
>>   3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index 33e342f529..e07cd522ef 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -555,15 +555,14 @@ static RISCVException 
>> write_mstatus(CPURISCVState *env, int csrno,
>>            * RV32: MPV and GVA are not in mstatus. The current plan 
>> is to
>>            * add them to mstatush. For now, we just don't support it.
>>            */
>> -        mask |= MSTATUS_MPV | MSTATUS_GVA;
>> +        mask |= MSTATUS_MPV | MSTATUS_GVA | MSTATUS64_UXL;
>>       }
>>         mstatus = (mstatus & ~mask) | (val & mask);
>>         if (riscv_cpu_mxl(env) == MXL_RV64) {
>> -        /* SXL and UXL fields are for now read only */
>> +        /* SXL fields are for now read only */
>>           mstatus = set_field(mstatus, MSTATUS64_SXL, MXL_RV64);
>> -        mstatus = set_field(mstatus, MSTATUS64_UXL, MXL_RV64);
>>       }
>>       env->mstatus = mstatus;
>
> Why do you not allow writes to SXL?

That means we still don't support the change of SXLEN.
I didn't check the S-mode CSRs behavior when XLEN changes in this patch 
set.

For example, the behavior of satp when trap into M-mode from S-mode if 
SXLEN=32 and MXLEN=64.

>
> You're missing a change to write_sstatus to allow S-mode to write to UXL.
Yes.
>
>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
>> b/target/riscv/insn_trans/trans_rvi.c.inc
>> index 7a0b037594..cb73a2f1ee 100644
>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>> @@ -472,7 +472,7 @@ static bool trans_csrrw(DisasContext *ctx, 
>> arg_csrrw *a)
>>           return do_csrw(ctx, a->csr, src);
>>       }
>>   -    TCGv mask = tcg_constant_tl(-1);
>> +    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX 
>> : -1);
>>       return do_csrrw(ctx, a->rd, a->csr, src, mask);
>>   }
>>   @@ -523,7 +523,7 @@ static bool trans_csrrwi(DisasContext *ctx, 
>> arg_csrrwi *a)
>>           return do_csrw(ctx, a->csr, src);
>>       }
>>   -    TCGv mask = tcg_constant_tl(-1);
>> +    TCGv mask = tcg_constant_tl(get_xl(ctx) == MXL_RV32 ? UINT32_MAX 
>> : -1);
>>       return do_csrrw(ctx, a->rd, a->csr, src, mask);
>>   }
>>   diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> index 095d39671b..561e156bec 100644
>> --- a/target/riscv/op_helper.c
>> +++ b/target/riscv/op_helper.c
>> @@ -50,7 +50,8 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
>>     void helper_csrw(CPURISCVState *env, int csr, target_ulong src)
>>   {
>> -    RISCVException ret = riscv_csrrw(env, csr, NULL, src, -1);
>> +    target_ulong mask = cpu_get_xl(env) == MXL_RV32 ? UINT32_MAX : -1;
>> +    RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask);
>>         if (ret != RISCV_EXCP_NONE) {
>>           riscv_raise_exception(env, ret, GETPC());
>>
>
> The rest of this should be a separate patch.
>
>
> r~

Re: [PATCH v2 14/14] target/riscv: Enable uxl field write

[Richard Henderson]

On 11/10/21 3:38 PM, LIU Zhiwei wrote:
>> Why do you not allow writes to SXL?
> 
> That means we still don't support the change of SXLEN.
> I didn't check the S-mode CSRs behavior when XLEN changes in this patch set.
> 
> For example, the behavior of satp when trap into M-mode from S-mode if SXLEN=32 and MXLEN=64.

Ah, good answer.


r~