[PATCH 3/6] target/riscv: Add pointer masking tb flags

Alexey Baturo posted 6 patches 11 months, 1 week ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.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 3/6] target/riscv: Add pointer masking tb flags
Posted by Alexey Baturo 11 months, 1 week ago
From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
---
 target/riscv/cpu.h        | 19 +++++++++++++------
 target/riscv/cpu_helper.c |  4 ++++
 target/riscv/translate.c  | 10 ++++++++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f49d4aa52c..2099168950 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -390,6 +390,10 @@ struct CPUArchState {
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif
+    /* current number of masked top bits by pointer masking */
+    target_ulong pm_pmlen;
+    /* if pointer masking should do sign extension */
+    bool pm_signext;
 
     /* Fields from here on are preserved across CPU reset. */
     QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
@@ -538,14 +542,17 @@ FIELD(TB_FLAGS, VILL, 14, 1)
 FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
 /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
 FIELD(TB_FLAGS, XL, 16, 2)
-FIELD(TB_FLAGS, VTA, 18, 1)
-FIELD(TB_FLAGS, VMA, 19, 1)
+/* If pointer masking should be applied and address sign extended */
+FIELD(TB_FLAGS, PM_ENABLED, 18, 1)
+FIELD(TB_FLAGS, PM_SIGNEXTEND, 19, 1)
+FIELD(TB_FLAGS, VTA, 20, 1)
+FIELD(TB_FLAGS, VMA, 21, 1)
 /* Native debug itrigger */
-FIELD(TB_FLAGS, ITRIGGER, 20, 1)
+FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 /* Virtual mode enabled */
-FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
-FIELD(TB_FLAGS, PRIV, 22, 2)
-FIELD(TB_FLAGS, AXL, 24, 2)
+FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
+FIELD(TB_FLAGS, PRIV, 24, 2)
+FIELD(TB_FLAGS, AXL, 25, 2)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a3d477d226..79cddbd930 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -135,6 +135,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
     flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
     flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
+    if (env->pm_pmlen != 0) {
+        flags = FIELD_DP32(flags, TB_FLAGS, PM_ENABLED, 1);
+    }
+    flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, env->pm_signext);
 
     *pflags = flags;
 }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 6b4b9a671c..4c0d526b58 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -42,6 +42,8 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
 static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
 static TCGv load_res;
 static TCGv load_val;
+/* number of top masked address bits by pointer masking extension */
+static TCGv pm_pmlen;
 
 /*
  * If an operation is being performed on less than TARGET_LONG_BITS,
@@ -103,6 +105,9 @@ typedef struct DisasContext {
     bool vl_eq_vlmax;
     CPUState *cs;
     TCGv zero;
+    /* pointer masking extension */
+    bool pm_enabled;
+    bool pm_signext;
     /* Use icount trigger for native debug */
     bool itrigger;
     /* FRM is known to contain a valid value. */
@@ -1176,6 +1181,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
     ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
     ctx->cs = cs;
+    ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED);
+    ctx->pm_signext = FIELD_EX32(tb_flags, TB_FLAGS, PM_SIGNEXTEND);
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
     ctx->zero = tcg_constant_tl(0);
     ctx->virt_inst_excp = false;
@@ -1307,4 +1314,7 @@ void riscv_translate_init(void)
                              "load_res");
     load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_val),
                              "load_val");
+    /* Assign var with number of pointer masking masked bits to tcg global */
+    pm_pmlen = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, pm_pmlen),
+                                   "pmlen");
 }
-- 
2.34.1
Re: [PATCH 3/6] target/riscv: Add pointer masking tb flags
Posted by Richard Henderson 11 months, 1 week ago
On 12/21/23 21:40, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> ---
>   target/riscv/cpu.h        | 19 +++++++++++++------
>   target/riscv/cpu_helper.c |  4 ++++
>   target/riscv/translate.c  | 10 ++++++++++
>   3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f49d4aa52c..2099168950 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -390,6 +390,10 @@ struct CPUArchState {
>       target_ulong senvcfg;
>       uint64_t henvcfg;
>   #endif
> +    /* current number of masked top bits by pointer masking */
> +    target_ulong pm_pmlen;
> +    /* if pointer masking should do sign extension */
> +    bool pm_signext;
>   
>       /* Fields from here on are preserved across CPU reset. */
>       QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> @@ -538,14 +542,17 @@ FIELD(TB_FLAGS, VILL, 14, 1)
>   FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
>   /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
>   FIELD(TB_FLAGS, XL, 16, 2)
> -FIELD(TB_FLAGS, VTA, 18, 1)
> -FIELD(TB_FLAGS, VMA, 19, 1)
> +/* If pointer masking should be applied and address sign extended */
> +FIELD(TB_FLAGS, PM_ENABLED, 18, 1)

I think it would be better add the entire two bit field here, so that the translator can 
see the true width of the address.  You can then use tcg_gen_{s}extract_tl to perform the 
truncation.  At which point the 'target_ulong pm_pmlen' is not required.


r~

> +FIELD(TB_FLAGS, PM_SIGNEXTEND, 19, 1)
> +FIELD(TB_FLAGS, VTA, 20, 1)
> +FIELD(TB_FLAGS, VMA, 21, 1)
>   /* Native debug itrigger */
> -FIELD(TB_FLAGS, ITRIGGER, 20, 1)
> +FIELD(TB_FLAGS, ITRIGGER, 22, 1)
>   /* Virtual mode enabled */
> -FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
> -FIELD(TB_FLAGS, PRIV, 22, 2)
> -FIELD(TB_FLAGS, AXL, 24, 2)
> +FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
> +FIELD(TB_FLAGS, PRIV, 24, 2)
> +FIELD(TB_FLAGS, AXL, 25, 2)
>   
>   #ifdef TARGET_RISCV32
>   #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a3d477d226..79cddbd930 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -135,6 +135,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>       flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
>       flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
>       flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
> +    if (env->pm_pmlen != 0) {
> +        flags = FIELD_DP32(flags, TB_FLAGS, PM_ENABLED, 1);
> +    }
> +    flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, env->pm_signext);
>   
>       *pflags = flags;
>   }
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 6b4b9a671c..4c0d526b58 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -42,6 +42,8 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
>   static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
>   static TCGv load_res;
>   static TCGv load_val;
> +/* number of top masked address bits by pointer masking extension */
> +static TCGv pm_pmlen;
>   
>   /*
>    * If an operation is being performed on less than TARGET_LONG_BITS,
> @@ -103,6 +105,9 @@ typedef struct DisasContext {
>       bool vl_eq_vlmax;
>       CPUState *cs;
>       TCGv zero;
> +    /* pointer masking extension */
> +    bool pm_enabled;
> +    bool pm_signext;
>       /* Use icount trigger for native debug */
>       bool itrigger;
>       /* FRM is known to contain a valid value. */
> @@ -1176,6 +1181,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
>       ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
>       ctx->cs = cs;
> +    ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED);
> +    ctx->pm_signext = FIELD_EX32(tb_flags, TB_FLAGS, PM_SIGNEXTEND);
>       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>       ctx->zero = tcg_constant_tl(0);
>       ctx->virt_inst_excp = false;
> @@ -1307,4 +1314,7 @@ void riscv_translate_init(void)
>                                "load_res");
>       load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_val),
>                                "load_val");
> +    /* Assign var with number of pointer masking masked bits to tcg global */
> +    pm_pmlen = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, pm_pmlen),
> +                                   "pmlen");
>   }
Re: [PATCH 3/6] target/riscv: Add pointer masking tb flags
Posted by Alexey Baturo 11 months, 1 week ago
Hi Richard,

Thanks for the suggestion.
If it's ok to consume another bit(3 bits total) for Pointer Masking flags,
I'll do it.
>so that the translator can see the true width of the address
I guess I'll need a helper to calculate the exact number of bits to
shift(0, 7 or 16) based on those 2 extracted bits. Is it ok with you?

Thanks

пт, 22 дек. 2023 г. в 01:49, Richard Henderson <richard.henderson@linaro.org
>:

> On 12/21/23 21:40, Alexey Baturo wrote:
> > From: Alexey Baturo <baturo.alexey@gmail.com>
> >
> > Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> > ---
> >   target/riscv/cpu.h        | 19 +++++++++++++------
> >   target/riscv/cpu_helper.c |  4 ++++
> >   target/riscv/translate.c  | 10 ++++++++++
> >   3 files changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index f49d4aa52c..2099168950 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -390,6 +390,10 @@ struct CPUArchState {
> >       target_ulong senvcfg;
> >       uint64_t henvcfg;
> >   #endif
> > +    /* current number of masked top bits by pointer masking */
> > +    target_ulong pm_pmlen;
> > +    /* if pointer masking should do sign extension */
> > +    bool pm_signext;
> >
> >       /* Fields from here on are preserved across CPU reset. */
> >       QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> > @@ -538,14 +542,17 @@ FIELD(TB_FLAGS, VILL, 14, 1)
> >   FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
> >   /* The combination of MXL/SXL/UXL that applies to the current cpu
> mode. */
> >   FIELD(TB_FLAGS, XL, 16, 2)
> > -FIELD(TB_FLAGS, VTA, 18, 1)
> > -FIELD(TB_FLAGS, VMA, 19, 1)
> > +/* If pointer masking should be applied and address sign extended */
> > +FIELD(TB_FLAGS, PM_ENABLED, 18, 1)
>
> I think it would be better add the entire two bit field here, so that the
> translator can
> see the true width of the address.  You can then use tcg_gen_{s}extract_tl
> to perform the
> truncation.  At which point the 'target_ulong pm_pmlen' is not required.
>
>
> r~
>
> > +FIELD(TB_FLAGS, PM_SIGNEXTEND, 19, 1)
> > +FIELD(TB_FLAGS, VTA, 20, 1)
> > +FIELD(TB_FLAGS, VMA, 21, 1)
> >   /* Native debug itrigger */
> > -FIELD(TB_FLAGS, ITRIGGER, 20, 1)
> > +FIELD(TB_FLAGS, ITRIGGER, 22, 1)
> >   /* Virtual mode enabled */
> > -FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
> > -FIELD(TB_FLAGS, PRIV, 22, 2)
> > -FIELD(TB_FLAGS, AXL, 24, 2)
> > +FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
> > +FIELD(TB_FLAGS, PRIV, 24, 2)
> > +FIELD(TB_FLAGS, AXL, 25, 2)
> >
> >   #ifdef TARGET_RISCV32
> >   #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index a3d477d226..79cddbd930 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -135,6 +135,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr
> *pc,
> >       flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
> >       flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
> >       flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
> > +    if (env->pm_pmlen != 0) {
> > +        flags = FIELD_DP32(flags, TB_FLAGS, PM_ENABLED, 1);
> > +    }
> > +    flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, env->pm_signext);
> >
> >       *pflags = flags;
> >   }
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index 6b4b9a671c..4c0d526b58 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -42,6 +42,8 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl,
> cpu_vstart;
> >   static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> >   static TCGv load_res;
> >   static TCGv load_val;
> > +/* number of top masked address bits by pointer masking extension */
> > +static TCGv pm_pmlen;
> >
> >   /*
> >    * If an operation is being performed on less than TARGET_LONG_BITS,
> > @@ -103,6 +105,9 @@ typedef struct DisasContext {
> >       bool vl_eq_vlmax;
> >       CPUState *cs;
> >       TCGv zero;
> > +    /* pointer masking extension */
> > +    bool pm_enabled;
> > +    bool pm_signext;
> >       /* Use icount trigger for native debug */
> >       bool itrigger;
> >       /* FRM is known to contain a valid value. */
> > @@ -1176,6 +1181,8 @@ static void
> riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> >       ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
> >       ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
> >       ctx->cs = cs;
> > +    ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED);
> > +    ctx->pm_signext = FIELD_EX32(tb_flags, TB_FLAGS, PM_SIGNEXTEND);
> >       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
> >       ctx->zero = tcg_constant_tl(0);
> >       ctx->virt_inst_excp = false;
> > @@ -1307,4 +1314,7 @@ void riscv_translate_init(void)
> >                                "load_res");
> >       load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState,
> load_val),
> >                                "load_val");
> > +    /* Assign var with number of pointer masking masked bits to tcg
> global */
> > +    pm_pmlen = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState,
> pm_pmlen),
> > +                                   "pmlen");
> >   }
>
>