Re: [PATCH v2 03/12] target/alpha: Simplify call_pal implementation

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 23/9/25 04:39, Richard Henderson wrote:
> Since 288a5fe980f, we don't link translation blocks
> directly to palcode entry points.  If we load palbr
> from env instead of encoding the constant, we avoid
> all need for tb_flush().
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/alpha/helper.h     |  1 -
>   target/alpha/sys_helper.c |  6 ------
>   target/alpha/translate.c  | 21 ++++++---------------
>   3 files changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/target/alpha/helper.h b/target/alpha/helper.h
> index d60f208703..788d2fbf28 100644
> --- a/target/alpha/helper.h
> +++ b/target/alpha/helper.h
> @@ -90,7 +90,6 @@ DEF_HELPER_FLAGS_2(ieee_input_s, TCG_CALL_NO_WG, void, env, i64)
>   #if !defined (CONFIG_USER_ONLY)
>   DEF_HELPER_FLAGS_1(tbia, TCG_CALL_NO_RWG, void, env)
>   DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, i64)
> -DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
>   
>   DEF_HELPER_1(halt, void, i64)
>   
> diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
> index 51e3254428..87e37605c1 100644
> --- a/target/alpha/sys_helper.c
> +++ b/target/alpha/sys_helper.c
> @@ -20,7 +20,6 @@
>   #include "qemu/osdep.h"
>   #include "cpu.h"
>   #include "exec/cputlb.h"
> -#include "exec/tb-flush.h"
>   #include "exec/helper-proto.h"
>   #include "system/runstate.h"
>   #include "system/system.h"
> @@ -38,11 +37,6 @@ void helper_tbis(CPUAlphaState *env, uint64_t p)
>       tlb_flush_page(env_cpu(env), p);
>   }
>   
> -void helper_tb_flush(CPUAlphaState *env)
> -{
> -    tb_flush(env_cpu(env));
> -}
> -
>   void helper_halt(uint64_t restart)
>   {
>       if (restart) {
> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
> index cebab0318c..f11b382438 100644
> --- a/target/alpha/translate.c
> +++ b/target/alpha/translate.c
> @@ -48,8 +48,6 @@ struct DisasContext {
>   
>   #ifdef CONFIG_USER_ONLY
>       MemOp unalign;
> -#else
> -    uint64_t palbr;
>   #endif
>       uint32_t tbflags;
>       int mem_idx;
> @@ -1155,7 +1153,6 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
>   #else
>       {
>           TCGv tmp = tcg_temp_new();
> -        uint64_t entry;
>   
>           gen_pc_disp(ctx, tmp, 0);
>           if (ctx->tbflags & ENV_FLAG_PAL_MODE) {
> @@ -1165,12 +1162,11 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
>           }
>           tcg_gen_st_i64(tmp, tcg_env, offsetof(CPUAlphaState, exc_addr));
>   
> -        entry = ctx->palbr;
> -        entry += (palcode & 0x80
> -                  ? 0x2000 + (palcode - 0x80) * 64
> -                  : 0x1000 + palcode * 64);
> -
> -        tcg_gen_movi_i64(cpu_pc, entry);
> +        tcg_gen_ld_i64(cpu_pc, tcg_env, offsetof(CPUAlphaState, palbr));
> +        tcg_gen_addi_i64(cpu_pc, cpu_pc,
> +                         palcode & 0x80
> +                         ? 0x2000 + (palcode - 0x80) * 64
> +                         : 0x1000 + palcode * 64);
>           return DISAS_PC_UPDATED;
>       }
>   #endif
> @@ -1292,11 +1288,7 @@ static DisasJumpType gen_mtpr(DisasContext *ctx, TCGv vb, int regno)
>       case 7:
>           /* PALBR */
>           tcg_gen_st_i64(vb, tcg_env, offsetof(CPUAlphaState, palbr));
> -        /* Changing the PAL base register implies un-chaining all of the TBs
> -           that ended with a CALL_PAL.  Since the base register usually only
> -           changes during boot, flushing everything works well.  */
> -        gen_helper_tb_flush(tcg_env);
> -        return DISAS_PC_STALE;
> +        break;
>   
>       case 32 ... 39:
>           /* Accessing the "non-shadow" general registers.  */
> @@ -2874,7 +2866,6 @@ static void alpha_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>       ctx->ir = cpu_std_ir;
>       ctx->unalign = (ctx->tbflags & TB_FLAG_UNALIGN ? MO_UNALN : MO_ALIGN);
>   #else
> -    ctx->palbr = env->palbr;
>       ctx->ir = (ctx->tbflags & ENV_FLAG_PAL_MODE ? cpu_pal_ir : cpu_std_ir);
>   #endif
>   

Probably unrelated but still same target, could you also update the
comment added in commit fe57ca82b09 ("target-alpha: Add placeholders
for missing userspace PALcalls")?

         case 0x86:
             /* IMB */
             /* ??? We can probably elide the code using page_unprotect
                that is checking for self-modifying code.  Instead we
                could simply call tb_flush here.  Until we work out the
                changes required to turn off the extra write protection,
                this can be a no-op.  */
             break;