diff mbox series

target/riscv: Exit current TB after an sfence.vma

Message ID 20220315192300.250310-1-idan.horowitz@gmail.com (mailing list archive)
State New, archived
Headers show
Series target/riscv: Exit current TB after an sfence.vma | expand

Commit Message

Idan Horowitz March 15, 2022, 7:23 p.m. UTC
If the pages which control the translation of the currently executing
instructions are changed, and then the TLB is flushed using sfence.vma
we have to exit the current TB early, to ensure we don't execute stale
instructions.

Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
---
 target/riscv/insn_trans/trans_privileged.c.inc | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Richard Henderson March 15, 2022, 7:37 p.m. UTC | #1
On 3/15/22 12:23, Idan Horowitz wrote:
> If the pages which control the translation of the currently executing
> instructions are changed, and then the TLB is flushed using sfence.vma
> we have to exit the current TB early, to ensure we don't execute stale
> instructions.
> 
> Signed-off-by: Idan Horowitz<idan.horowitz@gmail.com>
> ---
>   target/riscv/insn_trans/trans_privileged.c.inc | 7 +++++++
>   1 file changed, 7 insertions(+)

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


r~
Alistair Francis March 15, 2022, 10:52 p.m. UTC | #2
On Wed, Mar 16, 2022 at 5:26 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> If the pages which control the translation of the currently executing
> instructions are changed, and then the TLB is flushed using sfence.vma
> we have to exit the current TB early, to ensure we don't execute stale
> instructions.
>
> Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_privileged.c.inc | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 53613682e8..f265e8202d 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -114,6 +114,13 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
>  {
>  #ifndef CONFIG_USER_ONLY
>      gen_helper_tlb_flush(cpu_env);
> +    /*
> +     * The flush might have changed the backing physical memory of
> +     * the instructions we're currently executing
> +     */
> +    gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> +    tcg_gen_exit_tb(NULL, 0);
> +    ctx->base.is_jmp = DISAS_NORETURN;
>      return true;
>  #endif
>      return false;
> --
> 2.35.1
>
>
Alistair Francis March 15, 2022, 11:42 p.m. UTC | #3
On Wed, Mar 16, 2022 at 5:26 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> If the pages which control the translation of the currently executing
> instructions are changed, and then the TLB is flushed using sfence.vma
> we have to exit the current TB early, to ensure we don't execute stale
> instructions.
>
> Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/insn_trans/trans_privileged.c.inc | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 53613682e8..f265e8202d 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -114,6 +114,13 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
>  {
>  #ifndef CONFIG_USER_ONLY
>      gen_helper_tlb_flush(cpu_env);
> +    /*
> +     * The flush might have changed the backing physical memory of
> +     * the instructions we're currently executing
> +     */
> +    gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> +    tcg_gen_exit_tb(NULL, 0);
> +    ctx->base.is_jmp = DISAS_NORETURN;
>      return true;
>  #endif
>      return false;
> --
> 2.35.1
>
>
Alistair Francis March 30, 2022, 6:09 a.m. UTC | #4
On Wed, Mar 16, 2022 at 9:42 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Mar 16, 2022 at 5:26 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
> >
> > If the pages which control the translation of the currently executing
> > instructions are changed, and then the TLB is flushed using sfence.vma
> > we have to exit the current TB early, to ensure we don't execute stale
> > instructions.
> >
> > Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
>
> Thanks!
>
> Applied to riscv-to-apply.next

This results in failed Linux boots, I have dropped this patch

Alistair
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 53613682e8..f265e8202d 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -114,6 +114,13 @@  static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 {
 #ifndef CONFIG_USER_ONLY
     gen_helper_tlb_flush(cpu_env);
+    /*
+     * The flush might have changed the backing physical memory of
+     * the instructions we're currently executing
+     */
+    gen_set_pc_imm(ctx, ctx->pc_succ_insn);
+    tcg_gen_exit_tb(NULL, 0);
+    ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 #endif
     return false;