Re: [PATCH v5 12/22] target/arm: Implement the STGP instruction

[Peter Maydell]

On Fri, 11 Oct 2019 at 14:50, Richard Henderson
<address@hidden> wrote:
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> v3: Handle atomicity, require pre-cleaned address.
> ---
>  target/arm/translate-a64.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index c17b36ebb2..4ecb0a2fb7 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -2657,7 +2657,7 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
>   * +-----+-------+---+---+-------+---+-------+-------+------+------+
>   *
>   * opc: LDP/STP/LDNP/STNP        00 -> 32 bit, 10 -> 64 bit
> - *      LDPSW                    01
> + *      LDPSW/STGP               01
>   *      LDP/STP/LDNP/STNP (SIMD) 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit
>   *   V: 0 -> GPR, 1 -> Vector
>   * idx: 00 -> signed offset with non-temporal hint, 01 -> post-index,
> @@ -2682,6 +2682,7 @@ static void disas_ldst_pair(DisasContext *s, uint32_t 
> insn)
>      bool is_signed = false;
>      bool postindex = false;
>      bool wback = false;
> +    bool set_tag = false;
>
>      TCGv_i64 clean_addr, dirty_addr;
>
> @@ -2694,6 +2695,14 @@ static void disas_ldst_pair(DisasContext *s, uint32_t 
> insn)
>
>      if (is_vector) {
>          size = 2 + opc;
> +    } else if (opc == 1 && !is_load) {
> +        /* STGP */
> +        if (!dc_isar_feature(aa64_mte_insn_reg, s) || index == 0) {
> +            unallocated_encoding(s);
> +            return;
> +        }
> +        size = 3;
> +        set_tag = true;
>      } else {
>          size = 2 + extract32(opc, 1, 1);
>          is_signed = extract32(opc, 0, 1);
> @@ -2746,6 +2755,15 @@ static void disas_ldst_pair(DisasContext *s, uint32_t 
> insn)
>      }
>      clean_addr = clean_data_tbi(s, dirty_addr, wback || rn != 31);

Don't we need to adjust the 'check' argument to clean_data_tbi()
here? The pseudocode says STGP doesn't do tag-checking.

>
> +    if (set_tag) {
> +        TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
> +        if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> +            gen_helper_stg_parallel(cpu_env, clean_addr, tcg_rn);
> +        } else {
> +            gen_helper_stg(cpu_env, clean_addr, tcg_rn);
> +        }
> +    }
> +
>      if (is_vector) {
>          if (is_load) {
>              do_fp_ld(s, rt, clean_addr, size);
> --
> 2.17.1

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM