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

Richard Henderson posted 12 patches 5 days, 13 hours ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Laurent Vivier <laurent@vivier.eu>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH v2 03/12] target/alpha: Simplify call_pal implementation
Posted by Richard Henderson 5 days, 13 hours ago
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
 
-- 
2.43.0
Re: [PATCH v2 03/12] target/alpha: Simplify call_pal implementation
Posted by Philippe Mathieu-Daudé 5 days, 8 hours ago
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;
Re: [PATCH v2 03/12] target/alpha: Simplify call_pal implementation
Posted by Philippe Mathieu-Daudé 5 days, 6 hours ago
On 23/9/25 09:30, Philippe Mathieu-Daudé wrote:
> 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(-)


> 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;
> 

Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>