[Qemu-devel] [PATCH 05/17] target/arm: Suppress tag check for sp+offset

[Richard Henderson]

R0078 specifies that base register, or base register plus immediate
offset, is unchecked when the base register is SP.

Signed-off-by: Richard Henderson 
---
 target/arm/translate-a64.c | 37 ++---
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 5c2577a9ac..ee95ba7165 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -336,12 +336,11 @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 src)
  * This is always a fresh temporary, as we need to be able to
  * increment this independently of a dirty write-back address.
  */
-static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr)
+static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr, bool sp_off)
 {
 TCGv_i64 clean = new_tmp_a64(s);
 
-/* FIXME: SP+OFS is always unchecked.  */
-if (s->tbid && s->mte_active) {
+if (s->tbid && s->mte_active && !sp_off) {
 gen_helper_mte_check(clean, cpu_env, addr);
 } else {
 gen_top_byte_ignore(s, clean, addr, s->tbid);
@@ -2374,7 +2373,7 @@ static void gen_compare_and_swap(DisasContext *s, int rs, 
int rt,
 if (rn == 31) {
 gen_check_sp_alignment(s);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 tcg_gen_atomic_cmpxchg_i64(tcg_rs, clean_addr, tcg_rs, tcg_rt, memidx,
size | MO_ALIGN | s->be_data);
 }
@@ -2392,7 +2391,7 @@ static void gen_compare_and_swap_pair(DisasContext *s, 
int rs, int rt,
 if (rn == 31) {
 gen_check_sp_alignment(s);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 
 if (size == 2) {
 TCGv_i64 cmp = tcg_temp_new_i64();
@@ -2517,7 +2516,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 if (is_lasr) {
 tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 gen_store_exclusive(s, rs, rt, rt2, clean_addr, size, false);
 return;
 
@@ -2526,7 +2525,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 if (rn == 31) {
 gen_check_sp_alignment(s);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 s->is_ldex = true;
 gen_load_exclusive(s, rt, rt2, clean_addr, size, false);
 if (is_lasr) {
@@ -2546,7 +2545,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 gen_check_sp_alignment(s);
 }
 tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 do_gpr_st(s, cpu_reg(s, rt), clean_addr, size, true, rt,
   disas_ldst_compute_iss_sf(size, false, 0), is_lasr);
 return;
@@ -2562,7 +2561,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 if (rn == 31) {
 gen_check_sp_alignment(s);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 do_gpr_ld(s, cpu_reg(s, rt), clean_addr, size, false, false, true, rt,
   disas_ldst_compute_iss_sf(size, false, 0), is_lasr);
 tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
@@ -2576,7 +2575,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 if (is_lasr) {
 tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 gen_store_exclusive(s, rs, rt, rt2, clean_addr, size, true);
 return;
 }
@@ -2594,7 +2593,7 @@ static void disas_ldst_excl(DisasContext *s, uint32_t 
insn)
 if (rn == 31) {
 gen_check_sp_alignment(s);
 }
-clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn));
+clean_addr = clean_data_tbi(s, cpu_reg_sp(s, rn), rn == 31);
 s->is_ldex = true;
 gen_load_exclusive(s, rt, rt2, clean_addr, size, true);
 if (is_lasr) {
@@ -2784,7 +2783,7 @@ static void disas_ldst_pair(DisasContext *s, uint32_t 
insn)
 if (!postindex) {
 tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
 }
-clean_addr = clean_data_tbi(s, dirty_addr);
+clean_addr = clean_data_tbi(s, dirty_addr, rn == 31);
 
 if (is_vector) {
 if (is_load) {
@@ -2922,7 +2921,7 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t 
insn,
 if (!post_index) {
 tcg_gen_addi_i64(dirty_addr, dirty_addr, imm9);
 }
-clean_a

Re: [Qemu-devel] [PATCH 05/17] target/arm: Suppress tag check for sp+offset

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
 wrote:
>
> R0078 specifies that base register, or base register plus immediate
> offset, is unchecked when the base register is SP.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-a64.c | 37 ++---
>  1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 5c2577a9ac..ee95ba7165 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -336,12 +336,11 @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 
> src)
>   * This is always a fresh temporary, as we need to be able to
>   * increment this independently of a dirty write-back address.
>   */
> -static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr)
> +static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr, bool sp_off)

I think sp_off is not sufficiently general here. For instance
if you look at the pseudocode for LDR (immediate)
 
https://developer.arm.com/docs/ddi0596/b/base-instructions-alphabetic-order/ldr-immediate-load-register-immediate
we do the tag check if wback || n != 31.

That is, when the spec says "base register only, or base register
plus immediate offset addressing form", it is referencing the
list of addressing modes in the v8A Arm ARM DDA0487D.a C1.3.3,
and "pre-indexed" and "post-indexed" are separate from "base + immediate".
It looks like your patch is treating pre-indexed and
post-indexed the same as base+imm.

thanks
-- PMM