From: Richard Henderson <richard.henderson@linaro.org> To: LIU Zhiwei <zhiwei_liu@c-sky.com>, qemu-devel@nongnu.org, qemu-riscv@nongnu.org Cc: palmer@dabbelt.com, bin.meng@windriver.com, Alistair.Francis@wdc.com Subject: Re: [PATCH 11/13] target/riscv: Switch context in exception return Date: Mon, 1 Nov 2021 12:43:20 -0400 [thread overview] Message-ID: <0105910e-fd68-21ea-8ff0-36752dd0b2e7@linaro.org> (raw) In-Reply-To: <20211101100143.44356-12-zhiwei_liu@c-sky.com> On 11/1/21 6:01 AM, LIU Zhiwei wrote: > After excpetion return, we should give a xlen view of context in new > priveledge, including the general registers, pc, and CSRs. > > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com> > --- > target/riscv/helper.h | 1 + > .../riscv/insn_trans/trans_privileged.c.inc | 2 ++ > target/riscv/op_helper.c | 26 +++++++++++++++++++ > 3 files changed, 29 insertions(+) > > diff --git a/target/riscv/helper.h b/target/riscv/helper.h > index e198d43981..9b379d7232 100644 > --- a/target/riscv/helper.h > +++ b/target/riscv/helper.h > @@ -71,6 +71,7 @@ DEF_HELPER_2(sret, tl, env, tl) > DEF_HELPER_2(mret, tl, env, tl) > DEF_HELPER_1(wfi, void, env) > DEF_HELPER_1(tlb_flush, void, env) > +DEF_HELPER_1(switch_context_xl, void, env) > #endif > > /* Hypervisor functions */ > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc > index 75c6ef80a6..6e39632f83 100644 > --- a/target/riscv/insn_trans/trans_privileged.c.inc > +++ b/target/riscv/insn_trans/trans_privileged.c.inc > @@ -78,6 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) > > if (has_ext(ctx, RVS)) { > gen_helper_sret(cpu_pc, cpu_env, cpu_pc); > + gen_helper_switch_context_xl(cpu_env); > tcg_gen_exit_tb(NULL, 0); /* no chaining */ > ctx->base.is_jmp = DISAS_NORETURN; > } else { > @@ -94,6 +95,7 @@ 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_switch_context_xl(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..20cf8ad883 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -70,6 +70,32 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, > } > > #ifndef CONFIG_USER_ONLY > +void helper_switch_context_xl(CPURISCVState *env) > +{ > + RISCVMXL xl = cpu_get_xl(env); > + if (xl == env->misa_mxl_max) { > + return; > + } > + assert(xl < env->misa_mxl_max); > + switch (xl) { > + case MXL_RV32: > + for (int i = 1; i < 32; i++) { > + env->gpr[i] = (int32_t)env->gpr[i]; > + } I think this is wrong. As I read the spec, the new context ignores high bits and writes sign-extended values, but registers that are not written should not be changed. I think that a unit test with SXLEN == 64 and UXLEN == 32, where the U-mode program executes only the ECALL instruction, should leave the high 32 bits of all gprs unchanged on re-entry to S-mode. > + env->pc = (int32_t)env->pc; I think this will happen naturally via patch 3. > + /* > + * For the read-only bits of the previous-width CSR, the bits at the > + * same positions in the temporary register are set to zeros. > + */ > + if ((env->priv == PRV_U) && (env->misa_ext & RVV)) { > + env->vl = 0; > + env->vtype = 0; > + } I don't understand this. The return from the S-mode interrupt handler to the U-mode program should preserve the U-mode VTYPE. r~
WARNING: multiple messages have this Message-ID (diff)
From: Richard Henderson <richard.henderson@linaro.org> To: LIU Zhiwei <zhiwei_liu@c-sky.com>, qemu-devel@nongnu.org, qemu-riscv@nongnu.org Cc: Alistair.Francis@wdc.com, palmer@dabbelt.com, bin.meng@windriver.com Subject: Re: [PATCH 11/13] target/riscv: Switch context in exception return Date: Mon, 1 Nov 2021 12:43:20 -0400 [thread overview] Message-ID: <0105910e-fd68-21ea-8ff0-36752dd0b2e7@linaro.org> (raw) In-Reply-To: <20211101100143.44356-12-zhiwei_liu@c-sky.com> On 11/1/21 6:01 AM, LIU Zhiwei wrote: > After excpetion return, we should give a xlen view of context in new > priveledge, including the general registers, pc, and CSRs. > > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com> > --- > target/riscv/helper.h | 1 + > .../riscv/insn_trans/trans_privileged.c.inc | 2 ++ > target/riscv/op_helper.c | 26 +++++++++++++++++++ > 3 files changed, 29 insertions(+) > > diff --git a/target/riscv/helper.h b/target/riscv/helper.h > index e198d43981..9b379d7232 100644 > --- a/target/riscv/helper.h > +++ b/target/riscv/helper.h > @@ -71,6 +71,7 @@ DEF_HELPER_2(sret, tl, env, tl) > DEF_HELPER_2(mret, tl, env, tl) > DEF_HELPER_1(wfi, void, env) > DEF_HELPER_1(tlb_flush, void, env) > +DEF_HELPER_1(switch_context_xl, void, env) > #endif > > /* Hypervisor functions */ > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc > index 75c6ef80a6..6e39632f83 100644 > --- a/target/riscv/insn_trans/trans_privileged.c.inc > +++ b/target/riscv/insn_trans/trans_privileged.c.inc > @@ -78,6 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) > > if (has_ext(ctx, RVS)) { > gen_helper_sret(cpu_pc, cpu_env, cpu_pc); > + gen_helper_switch_context_xl(cpu_env); > tcg_gen_exit_tb(NULL, 0); /* no chaining */ > ctx->base.is_jmp = DISAS_NORETURN; > } else { > @@ -94,6 +95,7 @@ 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_switch_context_xl(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..20cf8ad883 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -70,6 +70,32 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, > } > > #ifndef CONFIG_USER_ONLY > +void helper_switch_context_xl(CPURISCVState *env) > +{ > + RISCVMXL xl = cpu_get_xl(env); > + if (xl == env->misa_mxl_max) { > + return; > + } > + assert(xl < env->misa_mxl_max); > + switch (xl) { > + case MXL_RV32: > + for (int i = 1; i < 32; i++) { > + env->gpr[i] = (int32_t)env->gpr[i]; > + } I think this is wrong. As I read the spec, the new context ignores high bits and writes sign-extended values, but registers that are not written should not be changed. I think that a unit test with SXLEN == 64 and UXLEN == 32, where the U-mode program executes only the ECALL instruction, should leave the high 32 bits of all gprs unchanged on re-entry to S-mode. > + env->pc = (int32_t)env->pc; I think this will happen naturally via patch 3. > + /* > + * For the read-only bits of the previous-width CSR, the bits at the > + * same positions in the temporary register are set to zeros. > + */ > + if ((env->priv == PRV_U) && (env->misa_ext & RVV)) { > + env->vl = 0; > + env->vtype = 0; > + } I don't understand this. The return from the S-mode interrupt handler to the U-mode program should preserve the U-mode VTYPE. r~
next prev parent reply other threads:[~2021-11-01 16:44 UTC|newest] Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-01 10:01 [PATCH 00/13] Support UXL filed in xstatus LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:01 ` [PATCH 01/13] target/riscv: Sign extend pc for different ol LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:29 ` Richard Henderson 2021-11-01 10:29 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 02/13] target/riscv: Extend pc for runtime pc write LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:33 ` Richard Henderson 2021-11-01 10:33 ` Richard Henderson 2021-11-02 1:48 ` LIU Zhiwei 2021-11-02 1:48 ` LIU Zhiwei 2021-11-02 10:18 ` Richard Henderson 2021-11-02 10:18 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 03/13] target/riscv: Ignore the pc bits above XLEN LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:35 ` Richard Henderson 2021-11-01 10:35 ` Richard Henderson 2021-11-02 10:20 ` Richard Henderson 2021-11-02 10:20 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 04/13] target/riscv: Use gdb xml according to max mxlen LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:40 ` Richard Henderson 2021-11-01 10:40 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 05/13] target/riscv: Calculate address according to ol LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:46 ` Richard Henderson 2021-11-01 10:46 ` Richard Henderson 2021-11-01 15:56 ` LIU Zhiwei 2021-11-01 15:56 ` LIU Zhiwei 2021-11-01 10:01 ` [PATCH 06/13] target/riscv: Adjust vsetvl " LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:53 ` Richard Henderson 2021-11-01 10:53 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 07/13] target/riscv: Ajdust vector atomic check with ol LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 10:55 ` Richard Henderson 2021-11-01 10:55 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 08/13] target/riscv: Fix check range for first fault only LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 13:41 ` Richard Henderson 2021-11-01 13:41 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 09/13] target/riscv: Adjust vector address with ol LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 11:35 ` Richard Henderson 2021-11-01 11:35 ` Richard Henderson 2021-11-08 9:28 ` LIU Zhiwei 2021-11-08 9:28 ` LIU Zhiwei 2021-11-09 6:37 ` Richard Henderson 2021-11-09 6:37 ` Richard Henderson 2021-11-09 8:04 ` LIU Zhiwei 2021-11-09 8:04 ` LIU Zhiwei 2021-11-09 8:18 ` Richard Henderson 2021-11-09 8:18 ` Richard Henderson 2021-11-09 8:39 ` LIU Zhiwei 2021-11-09 8:39 ` LIU Zhiwei 2021-11-09 9:05 ` LIU Zhiwei 2021-11-09 9:05 ` LIU Zhiwei 2021-11-09 9:25 ` Richard Henderson 2021-11-09 9:25 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 10/13] target/riscv: Adjust scalar reg in vector " LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 16:33 ` Richard Henderson 2021-11-01 16:33 ` Richard Henderson 2021-11-08 9:38 ` LIU Zhiwei 2021-11-08 9:38 ` LIU Zhiwei 2021-11-01 10:01 ` [PATCH 11/13] target/riscv: Switch context in exception return LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 16:43 ` Richard Henderson [this message] 2021-11-01 16:43 ` Richard Henderson 2021-11-08 11:23 ` LIU Zhiwei 2021-11-08 11:23 ` LIU Zhiwei 2021-11-09 6:38 ` LIU Zhiwei 2021-11-09 6:38 ` LIU Zhiwei 2021-11-09 6:51 ` LIU Zhiwei 2021-11-09 6:51 ` LIU Zhiwei 2021-11-01 10:01 ` [PATCH 12/13] target/riscv: Don't save pc when " LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 16:49 ` Richard Henderson 2021-11-01 16:49 ` Richard Henderson 2021-11-01 10:01 ` [PATCH 13/13] target/riscv: Enable uxl field write LIU Zhiwei 2021-11-01 10:01 ` LIU Zhiwei 2021-11-01 17:01 ` Richard Henderson 2021-11-01 17:01 ` Richard Henderson 2021-11-08 12:10 ` LIU Zhiwei 2021-11-08 12:10 ` LIU Zhiwei 2021-11-10 3:01 ` LIU Zhiwei 2021-11-10 3:01 ` LIU Zhiwei
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=0105910e-fd68-21ea-8ff0-36752dd0b2e7@linaro.org \ --to=richard.henderson@linaro.org \ --cc=Alistair.Francis@wdc.com \ --cc=bin.meng@windriver.com \ --cc=palmer@dabbelt.com \ --cc=qemu-devel@nongnu.org \ --cc=qemu-riscv@nongnu.org \ --cc=zhiwei_liu@c-sky.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.