diff mbox series

[4/7] target/hppa: exit tb on flush cache instructions

Message ID 20240317221431.251515-5-svens@stackframe.org
State New
Headers show
Series few fixes for hppa target | expand

Commit Message

Sven Schnelle March 17, 2024, 10:14 p.m. UTC 
When the guest modifies the tb it is currently executing from,
it executes a fic instruction. Exit the tb on such instruction,
otherwise we might execute stale code.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/translate.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Helge Deller March 18, 2024, 8:42 p.m. UTC | #1 
On 3/17/24 23:14, Sven Schnelle wrote:
> When the guest modifies the tb it is currently executing from,
> it executes a fic instruction. Exit the tb on such instruction,
> otherwise we might execute stale code.
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/translate.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 8ba31567e8..58d7ec1ade 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1940,6 +1940,7 @@ static void do_page_zero(DisasContext *ctx)
>   static bool trans_nop(DisasContext *ctx, arg_nop *a)
>   {
>       cond_free(&ctx->null_cond);
> +    ctx->base.is_jmp = DISAS_IAQ_N_STALE;
>       return true;
>   }
>
> @@ -2290,6 +2291,7 @@ static bool trans_nop_addrx(DisasContext *ctx, arg_ldst *a)
>           save_gpr(ctx, a->b, dest);
>       }
>       cond_free(&ctx->null_cond);
> +    ctx->base.is_jmp = DISAS_IAQ_N_STALE;

I wonder if it makes sense to rename trans_nop() and trans_nop_addrx()
to something like trans_cache_flush() and trans_cache_flush_addrx() ?

Other than that:
Reviewed-by: Helge Deller <deller@gmx.de>

Helge
Richard Henderson March 18, 2024, 9:16 p.m. UTC | #2 
On 3/17/24 12:14, Sven Schnelle wrote:
> When the guest modifies the tb it is currently executing from,
> it executes a fic instruction. Exit the tb on such instruction,
> otherwise we might execute stale code.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/translate.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 8ba31567e8..58d7ec1ade 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1940,6 +1940,7 @@ static void do_page_zero(DisasContext *ctx)
>   static bool trans_nop(DisasContext *ctx, arg_nop *a)
>   {
>       cond_free(&ctx->null_cond);
> +    ctx->base.is_jmp = DISAS_IAQ_N_STALE;
>       return true;
>   }
>   
> @@ -2290,6 +2291,7 @@ static bool trans_nop_addrx(DisasContext *ctx, arg_ldst *a)
>           save_gpr(ctx, a->b, dest);
>       }
>       cond_free(&ctx->null_cond);
> +    ctx->base.is_jmp = DISAS_IAQ_N_STALE;
>       return true;
>   }
>   

You should create new functions for fic,

static bool trans_fic(DisasContext *ctx, arg_nop *a)
{
     ctx->base.is_jmp = DISAS_IAQ_N_STALE;
     return trans_nop(ctx, a);
}

because fid and pdc also use trans_nop/trans_nop_addrx.


r~
diff mbox series

Patch

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 8ba31567e8..58d7ec1ade 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1940,6 +1940,7 @@  static void do_page_zero(DisasContext *ctx)
 static bool trans_nop(DisasContext *ctx, arg_nop *a)
 {
     cond_free(&ctx->null_cond);
+    ctx->base.is_jmp = DISAS_IAQ_N_STALE;
     return true;
 }
 
@@ -2290,6 +2291,7 @@  static bool trans_nop_addrx(DisasContext *ctx, arg_ldst *a)
         save_gpr(ctx, a->b, dest);
     }
     cond_free(&ctx->null_cond);
+    ctx->base.is_jmp = DISAS_IAQ_N_STALE;
     return true;
 }