[Qemu-devel] [PATCH 00/17] target/arm: Implement ARMv8.5-MemTag

[Qemu-devel] [PATCH 00/17] target/arm: Implement ARMv8.5-MemTag

[Richard Henderson]

Based-on: 20190110124951.15473-1-richard.henderson@linaro.org
aka the TBID patch set, which itself is based on the BTI patch set.

The full tree is available at

  https://github.org/rth7680/qemu.git tgt-arm-mte

This extension isl also spelled MTE in the ARM.

This patch set only attempts to implement linux-user emulation.
For system emulation, I still miss the new cache flushing insns (easy)
and the out-of-band physical memory for the allocation tags (harder).

>From a few mis-steps in writing the test cases for the extension,
I might suggest that some future kernel's userland ABI for this have
TCR.TCMA0 = 1, so that legacy code that is *not* MTE aware can use
a frame pointer without accidentally tripping left over stack tags.
(As seen in patch 5, SP+OFF is unchecked per the ISA but FP+OFF is not.)

OTOH, depending on the application, that does make it easier for an
attack vector to clean the tag off the top of a pointer to bypass
store checking.  So, tricky.


r~


Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: dave.martin@arm.com
Cc: szabolcs.nagy@arm.com
Cc: catalin.marinas@arm.com
Cc: mark.rutland@arm.com

Richard Henderson (17):
  target/arm: Add MTE_ACTIVE to tb_flags
  target/arm: Extract TCMA with ARMVAParameters
  target/arm: Add MTE system registers
  target/arm: Fill in helper_mte_check
  target/arm: Suppress tag check for sp+offset
  target/arm: Implement the IRG instruction
  target/arm: Implement ADDG, SUBG instructions
  target/arm: Implement the GMI instruction
  target/arm: Implement the SUBP instruction
  target/arm: Implement LDG, STG, ST2G instructions
  target/arm: Implement the STGP instruction
  target/arm: Implement the LDGV and STGV instructions
  target/arm: Set PSTATE.TCO on exception entry
  tcg: Introduce target-specific page data for user-only
  target/arm: Add allocation tag storage for user-only
  target/arm: Enable MTE
  tests/tcg/aarch64: Add mte smoke tests

 include/exec/cpu-all.h            |  10 +-
 target/arm/cpu.h                  |  18 ++
 target/arm/helper-a64.h           |  11 +
 target/arm/internals.h            |  22 ++
 target/arm/translate.h            |  13 ++
 accel/tcg/translate-all.c         |  28 +++
 linux-user/mmap.c                 |  10 +-
 linux-user/syscall.c              |   4 +-
 target/arm/cpu.c                  |  10 +
 target/arm/cpu64.c                |   1 +
 target/arm/helper.c               |  99 ++++++--
 target/arm/mte_helper.c           | 369 ++++++++++++++++++++++++++++++
 target/arm/translate-a64.c        | 305 ++++++++++++++++++++----
 tests/tcg/aarch64/mte-1.c         |  27 +++
 tests/tcg/aarch64/mte-2.c         |  39 ++++
 target/arm/Makefile.objs          |   2 +-
 tests/tcg/aarch64/Makefile.target |   4 +
 17 files changed, 907 insertions(+), 65 deletions(-)
 create mode 100644 target/arm/mte_helper.c
 create mode 100644 tests/tcg/aarch64/mte-1.c
 create mode 100644 tests/tcg/aarch64/mte-2.c

-- 
2.17.2

[Qemu-devel] [PATCH 01/17] target/arm: Add MTE_ACTIVE to tb_flags

[Richard Henderson]

When MTE is fully enabled, i.e. access to tags are enabled and
tag checks affect the PE, then arrange to perform the check
while stripping the TBI.

The check is not yet implemented, just the plumbing to that point.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h           | 13 +++++++++++
 target/arm/helper-a64.h    |  2 ++
 target/arm/internals.h     | 18 +++++++++++++++
 target/arm/translate.h     |  2 ++
 target/arm/helper.c        | 45 ++++++++++++++++++++++++++++----------
 target/arm/mte_helper.c    | 32 +++++++++++++++++++++++++++
 target/arm/translate-a64.c |  9 +++++++-
 target/arm/Makefile.objs   |  2 +-
 8 files changed, 110 insertions(+), 13 deletions(-)
 create mode 100644 target/arm/mte_helper.c

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f2ff52f287..22163c9c3f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1164,6 +1164,7 @@ void pmccntr_sync(CPUARMState *env);
 #define PSTATE_BTYPE (3U << 10)
 #define PSTATE_IL (1U << 20)
 #define PSTATE_SS (1U << 21)
+#define PSTATE_TCO (1U << 25)
 #define PSTATE_V (1U << 28)
 #define PSTATE_C (1U << 29)
 #define PSTATE_Z (1U << 30)
@@ -1640,6 +1641,7 @@ FIELD(ID_AA64PFR0, SVE, 32, 4)
 
 FIELD(ID_AA64PFR1, BT, 0, 4)
 FIELD(ID_AA64PFR1, SBSS, 4, 4)
+FIELD(ID_AA64PFR1, MTE, 8, 4)
 
 FIELD(ID_AA64MMFR0, PARANGE, 0, 4)
 FIELD(ID_AA64MMFR0, ASIDBITS, 4, 4)
@@ -3003,6 +3005,7 @@ FIELD(TBFLAG_A64, PAUTH_ACTIVE, 8, 1)
 FIELD(TBFLAG_A64, BT, 9, 1)
 FIELD(TBFLAG_A64, BTYPE, 10, 2)
 FIELD(TBFLAG_A64, TBID, 12, 2)
+FIELD(TBFLAG_A64, MTE_ACTIVE, 14, 1)
 
 static inline bool bswap_code(bool sctlr_b)
 {
@@ -3293,6 +3296,16 @@ static inline bool isar_feature_aa64_bti(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, BT) != 0;
 }
 
+static inline bool isar_feature_aa64_mte_insn_reg(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, MTE) != 0;
+}
+
+static inline bool isar_feature_aa64_mte(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, MTE) >= 2;
+}
+
 /*
  * Forward to the above feature tests given an ARMCPU pointer.
  */
diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index a915c1247f..fa4c371a47 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -102,3 +102,5 @@ DEF_HELPER_FLAGS_3(autda, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
+
+DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 587a1ddf58..6c018e773c 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -983,4 +983,22 @@ static inline int exception_target_el(CPUARMState *env)
     return target_el;
 }
 
+/* Determine if allocation tags are available.  */
+static inline bool allocation_tag_access_enabled(CPUARMState *env, int el,
+                                                 uint64_t sctlr)
+{
+    if (el < 3
+        && arm_feature(env, ARM_FEATURE_EL3)
+        && !(env->cp15.scr_el3 & SCR_ATA)) {
+        return false;
+    }
+    if (el < 2
+        && arm_feature(env, ARM_FEATURE_EL2)
+        && !(arm_hcr_el2_eff(env) & HCR_ATA)) {
+        return false;
+    }
+    sctlr &= (el == 0 ? SCTLR_ATA0 : SCTLR_ATA);
+    return sctlr != 0;
+}
+
 #endif
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 33af50a13f..5a101e1c6d 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -70,6 +70,8 @@ typedef struct DisasContext {
     bool ss_same_el;
     /* True if v8.3-PAuth is active.  */
     bool pauth_active;
+    /* True if v8.5-MTE tag checks affect the PE.  */
+    bool mte_active;
     /* True with v8.5-BTI and SCTLR_ELx.BT* set.  */
     bool bt;
     /*
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6b7b639da5..038e52af4b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3465,22 +3465,31 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
 
-    if (raw_read(env, ri) == value) {
-        /* Skip the TLB flush if nothing actually changed; Linux likes
-         * to do a lot of pointless SCTLR writes.
-         */
-        return;
-    }
-
     if (arm_feature(env, ARM_FEATURE_PMSA) && !cpu->has_mpu) {
         /* M bit is RAZ/WI for PMSA with no MPU implemented */
         value &= ~SCTLR_M;
     }
 
-    raw_write(env, ri, value);
+    if (!cpu_isar_feature(aa64_mte, cpu)) {
+        if (ri->opc1 == 6) { /* SCTLR_EL3 */
+            value &= ~(SCTLR_ITFSB | SCTLR_TCF | SCTLR_ATA);
+        } else {
+            value &= ~(SCTLR_ITFSB | SCTLR_TCF0 | SCTLR_TCF |
+                       SCTLR_ATA0 | SCTLR_ATA);
+        }
+    }
+
     /* ??? Lots of these bits are not implemented.  */
-    /* This may enable/disable the MMU, so do a TLB flush.  */
-    tlb_flush(CPU(cpu));
+
+    if (raw_read(env, ri) != value) {
+        /*
+         * This may enable/disable the MMU, so do a TLB flush.
+         * Skip the TLB flush if nothing actually changed;
+         * Linux likes to do a lot of pointless SCTLR writes.
+         */
+        raw_write(env, ri, value);
+        tlb_flush(CPU(cpu));
+    }
 }
 
 static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -13087,6 +13096,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
     if (is_a64(env)) {
         ARMCPU *cpu = arm_env_get_cpu(env);
         uint64_t sctlr;
+        int tbid;
 
         *pc = env->pc;
         flags = FIELD_DP32(flags, TBFLAG_ANY, AARCH64_STATE, 1);
@@ -13095,7 +13105,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
         {
             ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
             ARMVAParameters p0 = aa64_va_parameters_both(env, 0, stage1);
-            int tbii, tbid;
+            int tbii;
 
             /* FIXME: ARMv8.1-VHE S2 translation regime.  */
             if (regime_el(env, stage1) < 2) {
@@ -13148,6 +13158,19 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
             }
             flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
         }
+
+        /*
+         * If MTE is enabled, and tag checks affect the PE,
+         * then we check the tag as we strip the TBI field.
+         * Note that if TBI is disabled, all accesses are unchecked.
+         */
+        if (tbid
+            && cpu_isar_feature(aa64_mte, cpu)
+            && allocation_tag_access_enabled(env, current_el, sctlr)
+            && !(env->pstate & PSTATE_TCO)
+            && (sctlr & (current_el == 0 ? SCTLR_TCF0 : SCTLR_TCF))) {
+            flags = FIELD_DP32(flags, TBFLAG_A64, MTE_ACTIVE, 1);
+        }
     } else {
         *pc = env->regs[15];
         flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
new file mode 100644
index 0000000000..a3226c44a4
--- /dev/null
+++ b/target/arm/mte_helper.c
@@ -0,0 +1,32 @@
+/*
+ * ARM v8.5-MemTag Operations
+ *
+ * Copyright (c) 2019 Linaro, Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "internals.h"
+#include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
+#include "exec/helper-proto.h"
+
+
+uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
+{
+    /* Only unchecked implemented so far.  */
+    return sextract64(ptr, 0, 55);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index ee6f71c98f..0286507bae 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -339,7 +339,13 @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 src)
 static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr)
 {
     TCGv_i64 clean = new_tmp_a64(s);
-    gen_top_byte_ignore(s, clean, addr, s->tbid);
+
+    /* FIXME: SP+OFS is always unchecked.  */
+    if (s->tbid && s->mte_active) {
+        gen_helper_mte_check(clean, cpu_env, addr);
+    } else {
+        gen_top_byte_ignore(s, clean, addr, s->tbid);
+    }
     return clean;
 }
 
@@ -14000,6 +14006,7 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase,
     dc->pauth_active = FIELD_EX32(tb_flags, TBFLAG_A64, PAUTH_ACTIVE);
     dc->bt = FIELD_EX32(tb_flags, TBFLAG_A64, BT);
     dc->btype = FIELD_EX32(tb_flags, TBFLAG_A64, BTYPE);
+    dc->mte_active = FIELD_EX32(tb_flags, TBFLAG_A64, MTE_ACTIVE);
     dc->vec_len = 0;
     dc->vec_stride = 0;
     dc->cp_regs = arm_cpu->cp_regs;
diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
index 1a4fc06448..c86cb1af5c 100644
--- a/target/arm/Makefile.objs
+++ b/target/arm/Makefile.objs
@@ -8,7 +8,7 @@ obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += neon_helper.o iwmmxt_helper.o vec_helper.o
 obj-y += gdbstub.o
 obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
-obj-$(TARGET_AARCH64) += pauth_helper.o
+obj-$(TARGET_AARCH64) += pauth_helper.o mte_helper.o
 obj-y += crypto_helper.o
 obj-$(CONFIG_SOFTMMU) += arm-powerctl.o
 
-- 
2.17.2

[Qemu-devel] [PATCH 02/17] target/arm: Extract TCMA with ARMVAParameters

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/internals.h | 1 +
 target/arm/helper.c    | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 6c018e773c..2922324f63 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -959,6 +959,7 @@ typedef struct ARMVAParameters {
     bool tbid       : 1;
     bool epd        : 1;
     bool hpd        : 1;
+    bool tcma       : 1;
     bool using16k   : 1;
     bool using64k   : 1;
 } ARMVAParameters;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 038e52af4b..5a59fc4315 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9789,7 +9789,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
 {
     uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
     uint32_t el = regime_el(env, mmu_idx);
-    bool tbi, tbid, epd, hpd, using16k, using64k;
+    bool tbi, tbid, epd, hpd, tcma, using16k, using64k;
     int select, tsz;
 
     /* Bit 55 is always between the two regions, and is canonical for
@@ -9803,11 +9803,12 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
         using16k = extract32(tcr, 15, 1);
         if (mmu_idx == ARMMMUIdx_S2NS) {
             /* VTCR_EL2 */
-            tbi = tbid = hpd = false;
+            tbi = tbid = hpd = tcma = false;
         } else {
             tbi = extract32(tcr, 20, 1);
             hpd = extract32(tcr, 24, 1);
             tbid = extract32(tcr, 29, 1);
+            tcma = extract32(tcr, 30, 1);
         }
         epd = false;
     } else if (!select) {
@@ -9818,6 +9819,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
         tbi = extract64(tcr, 37, 1);
         hpd = extract64(tcr, 41, 1);
         tbid = extract64(tcr, 51, 1);
+        tcma = extract64(tcr, 57, 1);
     } else {
         int tg = extract32(tcr, 30, 2);
         using16k = tg == 1;
@@ -9827,6 +9829,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
         tbi = extract64(tcr, 38, 1);
         hpd = extract64(tcr, 42, 1);
         tbid = extract64(tcr, 52, 1);
+        tcma = extract64(tcr, 58, 1);
     }
     tsz = MIN(tsz, 39);  /* TODO: ARMv8.4-TTST */
     tsz = MAX(tsz, 16);  /* TODO: ARMv8.2-LVA  */
@@ -9838,6 +9841,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
         .tbid = tbid,
         .epd = epd,
         .hpd = hpd,
+        .tcma = tcma,
         .using16k = using16k,
         .using64k = using64k,
     };
-- 
2.17.2

[Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Richard Henderson]

This is TFSRE0_EL1, TFSR_EL1, TFSR_EL2, TFSR_EL3,
RGSR_EL1, GCR_EL1, and PSTATE.TCO.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h           |  5 +++++
 target/arm/translate.h     | 11 ++++++++++
 target/arm/helper.c        | 45 ++++++++++++++++++++++++++++++++++++++
 target/arm/translate-a64.c | 11 ++++++++++
 4 files changed, 72 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 22163c9c3f..c8b447e30a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -482,6 +482,11 @@ typedef struct CPUARMState {
         uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
         uint64_t vpidr_el2; /* Virtualization Processor ID Register */
         uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */
+#ifdef TARGET_AARCH64
+        uint64_t tfsr_el[4]; /* tfsrel0_el1 is index 0.  */
+        uint64_t gcr_el1;
+        uint64_t rgsr_el1;
+#endif
     } cp15;
 
     struct {
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 5a101e1c6d..a24757d3d7 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -204,6 +204,17 @@ static inline TCGv_i32 get_ahp_flag(void)
     return ret;
 }
 
+/* Set bits within PSTATE.  */
+static inline void set_pstate_bits(uint32_t bits)
+{
+    TCGv_i32 p = tcg_temp_new_i32();
+
+    tcg_gen_ld_i32(p, cpu_env, offsetof(CPUARMState, pstate));
+    tcg_gen_ori_i32(p, p, bits);
+    tcg_gen_st_i32(p, cpu_env, offsetof(CPUARMState, pstate));
+    tcg_temp_free_i32(p);
+}
+
 /* Clear bits within PSTATE.  */
 static inline void clear_pstate_bits(uint32_t bits)
 {
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5a59fc4315..df43deb0f8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5132,6 +5132,48 @@ static const ARMCPRegInfo pauth_reginfo[] = {
       .fieldoffset = offsetof(CPUARMState, apib_key.hi) },
     REGINFO_SENTINEL
 };
+
+static uint64_t tco_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pstate & PSTATE_TCO;
+}
+
+static void tco_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t val)
+{
+    env->pstate = (env->pstate & ~PSTATE_TCO) | (val & PSTATE_TCO);
+}
+
+static const ARMCPRegInfo mte_reginfo[] = {
+    { .name = "TFSRE0_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 6, .opc2 = 1,
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[0]) },
+    { .name = "TFSR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 5, .opc2 = 0,
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[1]) },
+    { .name = "TFSR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 5, .opc2 = 0,
+      .access = PL2_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[2]) },
+    { .name = "TFSR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 6, .opc2 = 0,
+      .access = PL3_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[3]) },
+    { .name = "RGSR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 5,
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.rgsr_el1) },
+    { .name = "GCR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 6,
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.gcr_el1) },
+    { .name = "TCO", .state = ARM_CP_STATE_AA64,
+      .opc0 = 0, .opc1 = 3, .crn = 4, .crm = 2, .opc2 = 7,
+      .type = ARM_CP_NO_RAW,
+      .access = PL0_RW, .readfn = tco_read, .writefn = tco_write },
+    REGINFO_SENTINEL
+};
 #endif
 
 void register_cp_regs_for_features(ARMCPU *cpu)
@@ -5923,6 +5965,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     if (cpu_isar_feature(aa64_pauth, cpu)) {
         define_arm_cp_regs(cpu, pauth_reginfo);
     }
+    if (cpu_isar_feature(aa64_mte_insn_reg, cpu)) {
+        define_arm_cp_regs(cpu, mte_reginfo);
+    }
 #endif
 }
 
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 0286507bae..5c2577a9ac 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1668,6 +1668,17 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
         s->base.is_jmp = DISAS_UPDATE;
         break;
 
+    case 0x1c: /* TCO */
+        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        }
+        if (crm & 1) {
+            set_pstate_bits(PSTATE_TCO);
+        } else {
+            clear_pstate_bits(PSTATE_TCO);
+        }
+        break;
+
     default:
     do_unallocated:
         unallocated_encoding(s);
-- 
2.17.2

[Qemu-devel] [PATCH 04/17] target/arm: Fill in helper_mte_check

[Richard Henderson]

Implements the rules of "PE generation of Checked and
Unchecked accesses" which aren't already covered by XXX.
Implements the rules of "PE handling of Tag Check Failure".

Does not implement tag physical address space, so all
operations reduce to unchecked so far.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/mte_helper.c | 80 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index a3226c44a4..6f4bc0aa04 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -25,8 +25,86 @@
 #include "exec/helper-proto.h"
 
 
+static int get_allocation_tag(CPUARMState *env, uint64_t ptr)
+{
+    /* Tag storage not implemented.  */
+    return -1;
+}
+
+static int allocation_tag_from_addr(uint64_t ptr)
+{
+    return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
+}
+
 uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
 {
-    /* Only unchecked implemented so far.  */
+    ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+    ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, true);
+    int ptr_tag, mem_tag;
+
+    /*
+     * If TBI is disabled, then the access is unchecked.
+     * While we filtered out TBI0==0 && TBI1==0 in cpu_get_tb_cpu_state,
+     * we did not save separate bits for TBI0 != TBI1.
+     */
+    if (!param.tbi) {
+        /* Do not ignore the top byte.  */
+        return ptr;
+    }
+
+    /*
+     * If TCMA is enabled, then physical tag 0 is unchecked.
+     * Note the rules R0076 & R0077 are written with logical tags,
+     * and we need the physical tag below anyway.
+     */
+    ptr_tag = allocation_tag_from_addr(ptr);
+    if (param.tcma && ptr_tag == 0) {
+        goto pass;
+    }
+
+    /*
+     * If an access is made to an address that does not provide tag storage,
+     * the result is implementation defined (R0006).  We choose to treat the
+     * access as unchecked.
+     * This is similar to MemAttr != Tagged, which are also unchecked.
+     */
+    mem_tag = get_allocation_tag(env, ptr);
+    if (mem_tag < 0) {
+        goto pass;
+    }
+
+    /* If the tags do not match, the tag check operation fails.  */
+    if (ptr_tag != mem_tag) {
+        int el = arm_current_el(env);
+        int tcf;
+
+        /* Indicate the tag check fail, both async and sync reporting.  */
+        env->cp15.tfsr_el[el] |= 1 << param.select;
+
+        if (el == 0) {
+            /* FIXME: ARMv8.1-VHE S2 translation regime.  */
+            tcf = extract64(env->cp15.sctlr_el[1], 38, 2);
+        } else {
+            tcf = extract64(env->cp15.sctlr_el[el], 40, 2);
+        }
+        if (tcf == 1) {
+            /* Tag check fail causes a synchronous exception.  */
+            CPUState *cs = ENV_GET_CPU(env);
+
+            /*
+             * In restore_state_to_opc, we set the exception syndrome
+             * for the load or store operation.  Do that first so we
+             * may overwrite that with the syndrome for the tag check.
+             */
+            cpu_restore_state(cs, GETPC(), true);
+            env->exception.vaddress = ptr;
+            raise_exception(env, EXCP_DATA_ABORT,
+                            syn_data_abort_no_iss(el != 0, 0, 0, 0, 0, 0x11),
+                            exception_target_el(env));
+        }
+    }
+
+ pass:
+    /* Unchecked, or tag check pass.  Ignore the top byte.  */
     return sextract64(ptr, 0, 55);
 }
-- 
2.17.2

[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 <richard.henderson@linaro.org>
---
 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_addr = clean_data_tbi(s, dirty_addr);
+    clean_addr = clean_data_tbi(s, dirty_addr, rn == 31);
 
     if (is_vector) {
         if (is_store) {
@@ -3029,7 +3028,7 @@ static void disas_ldst_reg_roffset(DisasContext *s, uint32_t insn,
     ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
 
     tcg_gen_add_i64(dirty_addr, dirty_addr, tcg_rm);
-    clean_addr = clean_data_tbi(s, dirty_addr);
+    clean_addr = clean_data_tbi(s, dirty_addr, false);
 
     if (is_vector) {
         if (is_store) {
@@ -3114,7 +3113,7 @@ static void disas_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn,
     dirty_addr = read_cpu_reg_sp(s, rn, 1);
     offset = imm12 << size;
     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_store) {
@@ -3198,7 +3197,7 @@ static void disas_ldst_atomic(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);
     tcg_rs = read_cpu_reg(s, rs, true);
 
     if (o3_opc == 1) { /* LDCLR */
@@ -3259,7 +3258,7 @@ static void disas_ldst_pac(DisasContext *s, uint32_t insn,
     tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
 
     /* Note that "clean" and "dirty" here refer to TBI not PAC.  */
-    clean_addr = clean_data_tbi(s, dirty_addr);
+    clean_addr = clean_data_tbi(s, dirty_addr, rn == 31);
 
     tcg_rt = cpu_reg(s, rt);
     do_gpr_ld(s, tcg_rt, clean_addr, size, /* is_signed */ false,
@@ -3413,7 +3412,7 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
     elements = (is_q ? 16 : 8) / ebytes;
 
     tcg_rn = cpu_reg_sp(s, rn);
-    clean_addr = clean_data_tbi(s, tcg_rn);
+    clean_addr = clean_data_tbi(s, tcg_rn, rn == 31);
     tcg_ebytes = tcg_const_i64(ebytes);
 
     for (r = 0; r < rpt; r++) {
@@ -3547,7 +3546,7 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
     }
 
     tcg_rn = cpu_reg_sp(s, rn);
-    clean_addr = clean_data_tbi(s, tcg_rn);
+    clean_addr = clean_data_tbi(s, tcg_rn, rn == 31);
     tcg_ebytes = tcg_const_i64(ebytes);
 
     for (xs = 0; xs < selem; xs++) {
-- 
2.17.2

[Qemu-devel] [PATCH 06/17] target/arm: Implement the IRG instruction

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.h    |  1 +
 target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++
 target/arm/translate-a64.c |  7 +++++
 3 files changed, 63 insertions(+)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index fa4c371a47..7a6051fdab 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 6f4bc0aa04..1878393fc4 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)
     return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
 }
 
+/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */
+static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
+{
+    if (exclude != 0xffff) {
+        int i;
+        for (i = 0; i < offset; ++i) {
+            do {
+                tag = (tag + 1) & 15;
+            } while (exclude & (1 << tag));
+        }
+    }
+    return tag;
+}
+
+static int choose_random_nonexcluded_tag(CPUARMState *env, uint16_t exclude)
+{
+    /* Ignore GCR_EL1.RRND.  Always produce deterministic results.  */
+    int start = extract32(env->cp15.rgsr_el1, 0, 4);
+    int seed = extract32(env->cp15.rgsr_el1, 8, 16);
+    int offset, rtag, i;
+
+    /* RandomTag */
+    for (i = offset = 0; i < 4; ++i) {
+        /* NextRandomTagBit */
+        int top = (extract32(seed, 5, 1) ^ extract32(seed, 3, 1) ^
+                   extract32(seed, 2, 1) ^ extract32(seed, 0, 1));
+        seed = (top << 15) | (seed >> 1);
+        offset |= top << i;
+    }
+    rtag = choose_nonexcluded_tag(start, offset, exclude);
+
+    env->cp15.rgsr_el1 = rtag | (seed << 8);
+
+    return rtag;
+}
+
+static uint64_t address_with_allocation_tag(uint64_t ptr, int rtag)
+{
+    rtag -= extract64(ptr, 55, 1);
+    return deposit64(ptr, 56, 4, rtag);
+}
+
 uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
 {
     ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
@@ -108,3 +150,16 @@ uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
     /* Unchecked, or tag check pass.  Ignore the top byte.  */
     return sextract64(ptr, 0, 55);
 }
+
+uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag = 0;
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        uint16_t exclude = rm | env->cp15.gcr_el1;
+        rtag = choose_random_nonexcluded_tag(env, exclude);
+    }
+    return address_with_allocation_tag(rn, rtag);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index ee95ba7165..b0349bffc4 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5105,6 +5105,13 @@ static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
     case 3: /* SDIV */
         handle_div(s, true, sf, rm, rn, rd);
         break;
+    case 4: /* IRG */
+        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        }
+        gen_helper_irg(cpu_reg_sp(s, rd), cpu_env,
+                       cpu_reg_sp(s, rn), cpu_reg(s, rm));
+        break;
     case 8: /* LSLV */
         handle_shift_reg(s, A64_SHIFT_TYPE_LSL, sf, rm, rn, rd);
         break;
-- 
2.17.2

[Qemu-devel] [PATCH 07/17] target/arm: Implement ADDG, SUBG instructions

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.h    |  2 ++
 target/arm/internals.h     |  3 ++
 target/arm/mte_helper.c    | 34 ++++++++++++++++++
 target/arm/translate-a64.c | 71 ++++++++++++++++++++++++++------------
 4 files changed, 87 insertions(+), 23 deletions(-)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index 7a6051fdab..47577207b2 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -105,3 +105,5 @@ DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_4(addg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
+DEF_HELPER_FLAGS_4(subg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 2922324f63..a5a249b001 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1002,4 +1002,7 @@ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el,
     return sctlr != 0;
 }
 
+/* We associate one allocation tag per 16 bytes, the minimum.  */
+#define LOG2_TAG_GRANULE 4
+
 #endif
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 1878393fc4..e2b1a5dd40 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -163,3 +163,37 @@ uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
     }
     return address_with_allocation_tag(rn, rtag);
 }
+
+uint64_t HELPER(addg)(CPUARMState *env, uint64_t ptr,
+                      uint32_t offset, uint32_t tag_offset)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag = 0;
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        int start_tag = allocation_tag_from_addr(ptr);
+        uint16_t exclude = env->cp15.gcr_el1;
+        rtag = choose_nonexcluded_tag(start_tag, tag_offset, exclude);
+    }
+
+    offset <<= LOG2_TAG_GRANULE;
+    return address_with_allocation_tag(ptr + offset, rtag);
+}
+
+uint64_t HELPER(subg)(CPUARMState *env, uint64_t ptr,
+                      uint32_t offset, uint32_t tag_offset)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag = 0;
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        int start_tag = allocation_tag_from_addr(ptr);
+        uint16_t exclude = env->cp15.gcr_el1;
+        rtag = choose_nonexcluded_tag(start_tag, tag_offset, exclude);
+    }
+
+    offset <<= LOG2_TAG_GRANULE;
+    return address_with_allocation_tag(ptr - offset, rtag);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b0349bffc4..879d6b8d46 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3652,7 +3652,9 @@ static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
  *    sf: 0 -> 32bit, 1 -> 64bit
  *    op: 0 -> add  , 1 -> sub
  *     S: 1 -> set flags
- * shift: 00 -> LSL imm by 0, 01 -> LSL imm by 12
+ * shift: 00 -> LSL imm by 0,
+ *        01 -> LSL imm by 12
+ *        10 -> ADDG, SUBG
  */
 static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
 {
@@ -3663,10 +3665,10 @@ static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
     bool setflags = extract32(insn, 29, 1);
     bool sub_op = extract32(insn, 30, 1);
     bool is_64bit = extract32(insn, 31, 1);
+    bool is_tag = false;
 
     TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
     TCGv_i64 tcg_rd = setflags ? cpu_reg(s, rd) : cpu_reg_sp(s, rd);
-    TCGv_i64 tcg_result;
 
     switch (shift) {
     case 0x0:
@@ -3674,35 +3676,58 @@ static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
     case 0x1:
         imm <<= 12;
         break;
+    case 0x2:
+        /* ADDG, SUBG */
+        if (!is_64bit || setflags || (imm & 0x30) ||
+            !dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        }
+        is_tag = true;
+        break;
     default:
+    do_unallocated:
         unallocated_encoding(s);
         return;
     }
 
-    tcg_result = tcg_temp_new_i64();
-    if (!setflags) {
-        if (sub_op) {
-            tcg_gen_subi_i64(tcg_result, tcg_rn, imm);
-        } else {
-            tcg_gen_addi_i64(tcg_result, tcg_rn, imm);
-        }
-    } else {
-        TCGv_i64 tcg_imm = tcg_const_i64(imm);
-        if (sub_op) {
-            gen_sub_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
-        } else {
-            gen_add_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
-        }
-        tcg_temp_free_i64(tcg_imm);
-    }
+    if (is_tag) {
+        TCGv_i32 tag_offset = tcg_const_i32(imm & 15);
+        TCGv_i32 offset = tcg_const_i32(imm >> 6);
 
-    if (is_64bit) {
-        tcg_gen_mov_i64(tcg_rd, tcg_result);
+        if (sub_op) {
+            gen_helper_subg(tcg_rd, cpu_env, tcg_rn, offset, tag_offset);
+        } else {
+            gen_helper_addg(tcg_rd, cpu_env, tcg_rn, offset, tag_offset);
+        }
+        tcg_temp_free_i32(tag_offset);
+        tcg_temp_free_i32(offset);
     } else {
-        tcg_gen_ext32u_i64(tcg_rd, tcg_result);
-    }
+        TCGv_i64 tcg_result;
 
-    tcg_temp_free_i64(tcg_result);
+        if (!setflags) {
+            tcg_result = tcg_rd;
+            if (sub_op) {
+                tcg_gen_subi_i64(tcg_result, tcg_rn, imm);
+            } else {
+                tcg_gen_addi_i64(tcg_result, tcg_rn, imm);
+            }
+        } else {
+            TCGv_i64 tcg_imm = tcg_const_i64(imm);
+            tcg_result = new_tmp_a64(s);
+            if (sub_op) {
+                gen_sub_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
+            } else {
+                gen_add_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
+            }
+            tcg_temp_free_i64(tcg_imm);
+        }
+
+        if (is_64bit) {
+            tcg_gen_mov_i64(tcg_rd, tcg_result);
+        } else {
+            tcg_gen_ext32u_i64(tcg_rd, tcg_result);
+        }
+    }
 }
 
 /* The input should be a value in the bottom e bits (with higher
-- 
2.17.2

[Qemu-devel] [PATCH 08/17] target/arm: Implement the GMI instruction

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.h    | 1 +
 target/arm/mte_helper.c    | 6 ++++++
 target/arm/translate-a64.c | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index 47577207b2..ef340cb6f9 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -107,3 +107,4 @@ DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_4(addg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
 DEF_HELPER_FLAGS_4(subg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
+DEF_HELPER_FLAGS_2(gmi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index e2b1a5dd40..2f6ac45150 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -197,3 +197,9 @@ uint64_t HELPER(subg)(CPUARMState *env, uint64_t ptr,
     offset <<= LOG2_TAG_GRANULE;
     return address_with_allocation_tag(ptr - offset, rtag);
 }
+
+uint64_t HELPER(gmi)(uint64_t ptr, uint64_t mask)
+{
+    int tag = allocation_tag_from_addr(ptr);
+    return mask | (1ULL << tag);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 879d6b8d46..6583ad93b1 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5137,6 +5137,12 @@ static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
         gen_helper_irg(cpu_reg_sp(s, rd), cpu_env,
                        cpu_reg_sp(s, rn), cpu_reg(s, rm));
         break;
+    case 5: /* GMI */
+        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        }
+        gen_helper_gmi(cpu_reg(s, rd), cpu_reg_sp(s, rn), cpu_reg(s, rm));
+        break;
     case 8: /* LSLV */
         handle_shift_reg(s, A64_SHIFT_TYPE_LSL, sf, rm, rn, rd);
         break;
-- 
2.17.2

[Qemu-devel] [PATCH 09/17] target/arm: Implement the SUBP instruction

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 6583ad93b1..98ff60c161 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5111,19 +5111,39 @@ static void handle_crc32(DisasContext *s,
  */
 static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
 {
-    unsigned int sf, rm, opcode, rn, rd;
+    unsigned int sf, rm, opcode, rn, rd, setflag;
     sf = extract32(insn, 31, 1);
+    setflag = extract32(insn, 29, 1);
     rm = extract32(insn, 16, 5);
     opcode = extract32(insn, 10, 6);
     rn = extract32(insn, 5, 5);
     rd = extract32(insn, 0, 5);
 
-    if (extract32(insn, 29, 1)) {
+    if (setflag && opcode != 0) {
         unallocated_encoding(s);
         return;
     }
 
     switch (opcode) {
+    case 0: /* SUBP(S) */
+        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
+            goto do_unallocated;
+        } else {
+            TCGv_i64 tcg_n, tcg_m, tcg_d;
+
+            tcg_n = read_cpu_reg_sp(s, rn, true);
+            tcg_m = read_cpu_reg_sp(s, rm, true);
+            tcg_gen_sextract_i64(tcg_n, tcg_n, 0, 55);
+            tcg_gen_sextract_i64(tcg_m, tcg_m, 0, 55);
+            tcg_d = cpu_reg(s, rd);
+
+            if (setflag) {
+                gen_sub_CC(true, tcg_d, tcg_n, tcg_m);
+            } else {
+                tcg_gen_sub_i64(tcg_d, tcg_n, tcg_m);
+            }
+        }
+        break;
     case 2: /* UDIV */
         handle_div(s, false, sf, rm, rn, rd);
         break;
-- 
2.17.2

[Qemu-devel] [PATCH 10/17] target/arm: Implement LDG, STG, ST2G instructions

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.h    |   3 ++
 target/arm/mte_helper.c    |  53 +++++++++++++++++++
 target/arm/translate-a64.c | 106 +++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index ef340cb6f9..ff37c8975a 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -108,3 +108,6 @@ DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_4(addg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
 DEF_HELPER_FLAGS_4(subg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
 DEF_HELPER_FLAGS_2(gmi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(ldg, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_2(stg, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_2(st2g, TCG_CALL_NO_WG, i64, env, i64)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 2f6ac45150..06fd9c18f9 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -31,6 +31,12 @@ static int get_allocation_tag(CPUARMState *env, uint64_t ptr)
     return -1;
 }
 
+static bool set_allocation_tag(CPUARMState *env, uint64_t ptr, int tag)
+{
+    /* Tag storage not implemented.  */
+    return false;
+}
+
 static int allocation_tag_from_addr(uint64_t ptr)
 {
     return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
@@ -203,3 +209,50 @@ uint64_t HELPER(gmi)(uint64_t ptr, uint64_t mask)
     int tag = allocation_tag_from_addr(ptr);
     return mask | (1ULL << tag);
 }
+
+uint64_t HELPER(ldg)(CPUARMState *env, uint64_t ptr)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag = 0;
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        rtag = get_allocation_tag(env, ptr);
+        if (rtag < 0) {
+            rtag = 0;
+        }
+    }
+    return address_with_allocation_tag(ptr, rtag);
+}
+
+uint64_t HELPER(stg)(CPUARMState *env, uint64_t ptr)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        int tag = allocation_tag_from_addr(ptr);
+        set_allocation_tag(env, ptr, tag);
+    }
+
+    /* Clean the pointer for use by stgz.  */
+    /* ??? Do we need a more precise TBI here?  */
+    return sextract64(ptr, 0, 55);
+}
+
+uint64_t HELPER(st2g)(CPUARMState *env, uint64_t ptr)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+
+    if (allocation_tag_access_enabled(env, el, sctlr)) {
+        int tag = allocation_tag_from_addr(ptr);
+        if (set_allocation_tag(env, ptr, tag)) {
+            set_allocation_tag(env, ptr + (1 << LOG2_TAG_GRANULE), tag);
+        }
+    }
+
+    /* Clean the pointer for use by stgz.  */
+    /* ??? Do we need a more precise TBI here?  */
+    return sextract64(ptr, 0, 55);
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 98ff60c161..60865945e4 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3583,6 +3583,109 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
     }
 }
 
+/*
+ * Load/Store memory tags
+ *
+ *  31 30 29         24     22  21     12    10      5      0
+ * +-----+-------------+-----+---+------+-----+------+------+
+ * | 1 1 | 0 1 1 0 0 1 | op1 | 1 | imm9 | op2 |  Rn  |  Rt  |
+ * +-----+-------------+-----+---+------+-----+------+------+
+ */
+static void disas_ldst_tag(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    uint64_t offset = sextract64(insn, 12, 9) << LOG2_TAG_GRANULE;
+    int op2 = extract32(insn, 10, 3);
+    int op1 = extract32(insn, 22, 2);
+    bool is_load = false, is_pair = false, is_zero = false;
+    int index = 0;
+    TCGv_i64 dirty_addr, clean_addr;
+
+    if ((insn & 0xff200000) != 0xd9200000
+        || !dc_isar_feature(aa64_mte_insn_reg, s)) {
+        goto do_unallocated;
+    }
+
+    switch (op1) {
+    case 0: /* STG */
+        if (op2 != 0) {
+            /* STG */
+            index = op2 - 2;
+            break;
+        }
+        goto do_unallocated;
+    case 1:
+        if (op2 != 0) {
+            /* STZG */
+            is_zero = true;
+            index = op2 - 2;
+        } else {
+            /* LDG */
+            is_load = true;
+        }
+        break;
+    case 2:
+        if (op2 != 0) {
+            /* ST2G */
+            is_pair = true;
+            index = op2 - 2;
+            break;
+        }
+        goto do_unallocated;
+    case 3:
+        if (op2 != 0) {
+            /* STZ2G */
+            is_pair = is_zero = true;
+            index = op2 - 2;
+            break;
+        }
+        goto do_unallocated;
+
+    default:
+    do_unallocated:
+        unallocated_encoding(s);
+        return;
+    }
+
+    dirty_addr = read_cpu_reg_sp(s, rn, true);
+    if (index <= 0) {
+        /* pre-index or signed offset */
+        tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
+    }
+
+    clean_addr = tcg_temp_new_i64();
+    if (is_load) {
+        gen_helper_ldg(cpu_reg(s, rt), cpu_env, dirty_addr);
+    } else if (is_pair) {
+        gen_helper_st2g(clean_addr, cpu_env, dirty_addr);
+    } else {
+        gen_helper_stg(clean_addr, cpu_env, dirty_addr);
+    }
+
+    if (is_zero) {
+        TCGv_i64 tcg_zero = tcg_const_i64(0);
+        int mem_index = get_mem_index(s);
+        int i, n = (1 + is_pair) << LOG2_TAG_GRANULE;
+
+        for (i = 0; i < n; i += 8) {
+            tcg_gen_qemu_st_i64(tcg_zero, clean_addr, mem_index, MO_Q);
+            tcg_gen_addi_i64(clean_addr, clean_addr, 8);
+        }
+        tcg_temp_free_i64(tcg_zero);
+    }
+    tcg_temp_free_i64(clean_addr);
+
+    if (index != 0) {
+        /* pre-index or post-index */
+        if (index > 0) {
+            /* post-index */
+            tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
+        }
+        tcg_gen_mov_i64(cpu_reg_sp(s, rn), dirty_addr);
+    }
+}
+
 /* Loads and stores */
 static void disas_ldst(DisasContext *s, uint32_t insn)
 {
@@ -3607,6 +3710,9 @@ static void disas_ldst(DisasContext *s, uint32_t insn)
     case 0x0d: /* AdvSIMD load/store single structure */
         disas_ldst_single_struct(s, insn);
         break;
+    case 0x19: /* Load/store tag */
+        disas_ldst_tag(s, insn);
+        break;
     default:
         unallocated_encoding(s);
         break;
-- 
2.17.2

[Qemu-devel] [PATCH 11/17] target/arm: Implement the STGP instruction

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 60865945e4..911d6f06b3 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2696,7 +2696,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,
@@ -2721,6 +2721,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;
 
@@ -2733,6 +2734,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);
@@ -2783,7 +2792,12 @@ 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, rn == 31);
+    if (set_tag) {
+        clean_addr = new_tmp_a64(s);
+        gen_helper_stg(clean_addr, cpu_env, dirty_addr);
+    } else {
+        clean_addr = clean_data_tbi(s, dirty_addr, rn == 31);
+    }
 
     if (is_vector) {
         if (is_load) {
-- 
2.17.2

[Qemu-devel] [PATCH 12/17] target/arm: Implement the LDGV and STGV instructions

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.h    |  2 ++
 target/arm/mte_helper.c    | 51 ++++++++++++++++++++++++++++++++++++++
 target/arm/translate-a64.c | 34 ++++++++++++++++++++-----
 3 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index ff37c8975a..5bbfe43c13 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -111,3 +111,5 @@ DEF_HELPER_FLAGS_2(gmi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(ldg, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(stg, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(st2g, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_2(ldgv, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_3(stgv, TCG_CALL_NO_WG, void, env, i64, i64)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 06fd9c18f9..b125f49258 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -256,3 +256,54 @@ uint64_t HELPER(st2g)(CPUARMState *env, uint64_t ptr)
     /* ??? Do we need a more precise TBI here?  */
     return sextract64(ptr, 0, 55);
 }
+
+uint64_t HELPER(ldgv)(CPUARMState *env, uint64_t ptr)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    uint64_t ret;
+    int rtag, i;
+
+    if (!allocation_tag_access_enabled(env, el, sctlr)) {
+        return 0;
+    }
+
+    ptr = QEMU_ALIGN_DOWN(ptr, 1 << LOG2_TAG_GRANULE);
+    rtag = get_allocation_tag(env, ptr);
+    if (rtag < 0) {
+        /* The entire page does not have tags.  */
+        return 0;
+    }
+
+    i = extract32(ptr, LOG2_TAG_GRANULE, 4);
+    ret = (uint64_t)rtag << i;
+    for (i++; i < 16; i++) {
+        rtag = get_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE));
+        ret |= (uint64_t)rtag << i;
+    }
+
+    return ret;
+}
+
+void HELPER(stgv)(CPUARMState *env, uint64_t ptr, uint64_t val)
+{
+    int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);
+    int rtag, i;
+
+    if (!allocation_tag_access_enabled(env, el, sctlr)) {
+        return;
+    }
+
+    rtag = allocation_tag_from_addr(ptr);
+    ptr = QEMU_ALIGN_DOWN(ptr, 1 << LOG2_TAG_GRANULE);
+    if (!set_allocation_tag(env, ptr, rtag)) {
+        /* The entire page does not have tags.  */
+        return;
+    }
+
+    i = extract32(ptr, LOG2_TAG_GRANULE, 4);
+    for (i++; i < 16; i++) {
+        set_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE), rtag);
+    }
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 911d6f06b3..b4226def40 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3612,7 +3612,7 @@ static void disas_ldst_tag(DisasContext *s, uint32_t insn)
     uint64_t offset = sextract64(insn, 12, 9) << LOG2_TAG_GRANULE;
     int op2 = extract32(insn, 10, 3);
     int op1 = extract32(insn, 22, 2);
-    bool is_load = false, is_pair = false, is_zero = false;
+    bool is_load = false, is_pair = false, is_zero = false, is_tagv = false;
     int index = 0;
     TCGv_i64 dirty_addr, clean_addr;
 
@@ -3644,17 +3644,29 @@ static void disas_ldst_tag(DisasContext *s, uint32_t insn)
             /* ST2G */
             is_pair = true;
             index = op2 - 2;
-            break;
+        } else {
+            /* STGV */
+            if (s->current_el == 0 || offset != 0) {
+                goto do_unallocated;
+            }
+            is_tagv = true;
+            index = 1;
         }
-        goto do_unallocated;
+        break;
     case 3:
         if (op2 != 0) {
             /* STZ2G */
             is_pair = is_zero = true;
             index = op2 - 2;
-            break;
+        } else {
+            /* LDGV */
+            if (s->current_el == 0 || offset != 0 || rt == rn) {
+                goto do_unallocated;
+            }
+            is_load = is_tagv = true;
+            index = 1;
         }
-        goto do_unallocated;
+        break;
 
     default:
     do_unallocated:
@@ -3669,7 +3681,17 @@ static void disas_ldst_tag(DisasContext *s, uint32_t insn)
     }
 
     clean_addr = tcg_temp_new_i64();
-    if (is_load) {
+    if (is_tagv) {
+        if (is_load) {
+            gen_helper_ldgv(cpu_reg(s, rt), cpu_env, dirty_addr);
+        } else {
+            gen_helper_stgv(cpu_env, dirty_addr, cpu_reg(s, rt));
+        }
+
+        /* Post-increment with dirty = align_up(dirty, 16*TAG_GRANULE).  */
+        tcg_gen_ori_i64(dirty_addr, dirty_addr, (16 << LOG2_TAG_GRANULE) - 1);
+        tcg_gen_addi_i64(dirty_addr, dirty_addr, 1);
+    } else if (is_load) {
         gen_helper_ldg(cpu_reg(s, rt), cpu_env, dirty_addr);
     } else if (is_pair) {
         gen_helper_st2g(clean_addr, cpu_env, dirty_addr);
-- 
2.17.2

[Qemu-devel] [PATCH 13/17] target/arm: Set PSTATE.TCO on exception entry

[Richard Henderson]

R0085 specifies that exception handlers begin with tag checks overridden.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index df43deb0f8..1e9ccf0b2e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8830,7 +8830,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
     qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
                   env->elr_el[new_el]);
 
-    pstate_write(env, PSTATE_DAIF | new_mode);
+    pstate_write(env, PSTATE_DAIF | PSTATE_TCO | new_mode);
     env->aarch64 = 1;
     aarch64_restore_sp(env, new_el);
 
-- 
2.17.2

[Qemu-devel] [PATCH 14/17] tcg: Introduce target-specific page data for user-only

[Richard Henderson]

At the same time, remember MAP_SHARED as PAGE_SHARED.  When mapping
new pages, make sure that old target-specific page data is removed.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h    | 10 ++++++++--
 accel/tcg/translate-all.c | 28 ++++++++++++++++++++++++++++
 linux-user/mmap.c         | 10 ++++++++--
 linux-user/syscall.c      |  4 ++--
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 117d2fbbca..92ec47dc79 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -244,10 +244,14 @@ extern intptr_t qemu_host_page_mask;
 #define PAGE_WRITE_ORG 0x0010
 /* Invalidate the TLB entry immediately, helpful for s390x
  * Low-Address-Protection. Used with PAGE_WRITE in tlb_set_page_with_attrs() */
-#define PAGE_WRITE_INV 0x0040
+#define PAGE_WRITE_INV 0x0020
+/* Page is mapped shared.  */
+#define PAGE_SHARED    0x0040
+/* For use with page_set_flags: page is being replaced; target_data cleared. */
+#define PAGE_RESET     0x0080
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
 /* FIXME: Code that sets/uses this is broken and needs to go away.  */
-#define PAGE_RESERVED  0x0020
+#define PAGE_RESERVED  0x0100
 #endif
 
 #if defined(CONFIG_USER_ONLY)
@@ -260,6 +264,8 @@ int walk_memory_regions(void *, walk_memory_regions_fn);
 int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
 int page_check_range(target_ulong start, target_ulong len, int flags);
+void *page_get_target_data(target_ulong address);
+void *page_alloc_target_data(target_ulong address, size_t size);
 #endif
 
 CPUArchState *cpu_copy(CPUArchState *env);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 639f0b2728..047cd2f50d 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -111,6 +111,7 @@ typedef struct PageDesc {
     unsigned int code_write_count;
 #else
     unsigned long flags;
+    void *target_data;
 #endif
 #ifndef CONFIG_USER_ONLY
     QemuSpin lock;
@@ -2477,6 +2478,7 @@ int page_get_flags(target_ulong address)
 void page_set_flags(target_ulong start, target_ulong end, int flags)
 {
     target_ulong addr, len;
+    bool reset_target_data;
 
     /* This function should never be called with addresses outside the
        guest address space.  If this assert fires, it probably indicates
@@ -2493,6 +2495,8 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
     if (flags & PAGE_WRITE) {
         flags |= PAGE_WRITE_ORG;
     }
+    reset_target_data = !(flags & PAGE_VALID) || (flags & PAGE_RESET);
+    flags &= ~PAGE_RESET;
 
     for (addr = start, len = end - start;
          len != 0;
@@ -2506,10 +2510,34 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
             p->first_tb) {
             tb_invalidate_phys_page(addr, 0);
         }
+        if (reset_target_data && p->target_data) {
+            g_free(p->target_data);
+            p->target_data = NULL;
+        }
         p->flags = flags;
     }
 }
 
+void *page_get_target_data(target_ulong address)
+{
+    PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
+    return p ? p->target_data : NULL;
+}
+
+void *page_alloc_target_data(target_ulong address, size_t size)
+{
+    PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
+    void *ret = NULL;
+
+    if (p) {
+        ret = p->target_data;
+        if (!ret && (p->flags & PAGE_VALID)) {
+            p->target_data = ret = g_malloc0(size);
+        }
+    }
+    return ret;
+}
+
 int page_check_range(target_ulong start, target_ulong len, int flags)
 {
     PageDesc *p;
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 41e0983ce8..f83874b8c1 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -562,7 +562,11 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         }
     }
  the_end1:
-    page_set_flags(start, start + len, prot | PAGE_VALID);
+    if ((flags & MAP_TYPE) == MAP_SHARED) {
+        prot |= PAGE_SHARED;
+    }
+    prot |= PAGE_RESET | PAGE_VALID;
+    page_set_flags(start, start + len, prot);
  the_end:
 #ifdef DEBUG_MMAP
     printf("ret=0x" TARGET_ABI_FMT_lx "\n", start);
@@ -754,9 +758,11 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
         new_addr = -1;
     } else {
         new_addr = h2g(host_addr);
+        /* FIXME: Move page flags (and target_data?) for each page.  */
         prot = page_get_flags(old_addr);
         page_set_flags(old_addr, old_addr + old_size, 0);
-        page_set_flags(new_addr, new_addr + new_size, prot | PAGE_VALID);
+        page_set_flags(new_addr, new_addr + new_size,
+                       prot | PAGE_VALID | PAGE_RESET);
     }
     tb_invalidate_phys_range(new_addr, new_addr + new_size);
     mmap_unlock();
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 280137da8c..715101816d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3845,8 +3845,8 @@ static inline abi_ulong do_shmat(CPUArchState *cpu_env,
     raddr=h2g((unsigned long)host_raddr);
 
     page_set_flags(raddr, raddr + shm_info.shm_segsz,
-                   PAGE_VALID | PAGE_READ |
-                   ((shmflg & SHM_RDONLY)? 0 : PAGE_WRITE));
+                   PAGE_VALID | PAGE_SHARED | PAGE_RESET | PAGE_READ |
+                   (shmflg & SHM_RDONLY ? 0 : PAGE_WRITE));
 
     for (i = 0; i < N_SHM_REGIONS; i++) {
         if (!shm_regions[i].in_use) {
-- 
2.17.2

[Qemu-devel] [PATCH 15/17] target/arm: Add allocation tag storage for user-only

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/mte_helper.c | 82 +++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index b125f49258..87328c7a9a 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -25,16 +25,72 @@
 #include "exec/helper-proto.h"
 
 
-static int get_allocation_tag(CPUARMState *env, uint64_t ptr)
+static int get_allocation_tag(CPUARMState *env, uint64_t ptr, uintptr_t ra)
 {
+#ifdef CONFIG_USER_ONLY
+    uint64_t clean_ptr = extract64(ptr, 0, 56);
+    uint8_t *tags = page_get_target_data(clean_ptr);
+
+    if (tags != NULL) {
+        uintptr_t index = extract64(clean_ptr, LOG2_TAG_GRANULE + 1,
+                                    TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1);
+        return extract32(tags[index], (clean_ptr & 1) * 4, 4);
+    } else {
+        int flags = page_get_flags(clean_ptr);
+
+        if (flags & PAGE_SHARED) {
+            /* There may be multiple mappings; pretend not implemented.  */
+            return -1;
+        } else if (flags & PAGE_VALID) {
+            /* Page is good, but no tags have been written: all are 0.  */
+            return 0;
+        } else {
+            /* Page is invalid: SIGSEGV.  */
+            env->exception.vaddress = ptr;
+            cpu_restore_state(ENV_GET_CPU(env), ra, true);
+            raise_exception(env, EXCP_DATA_ABORT, 0, 1);
+        }
+    }
+#else
     /* Tag storage not implemented.  */
     return -1;
+#endif
 }
 
-static bool set_allocation_tag(CPUARMState *env, uint64_t ptr, int tag)
+static bool set_allocation_tag(CPUARMState *env, uint64_t ptr,
+                               int tag, uintptr_t ra)
 {
+#ifdef CONFIG_USER_ONLY
+    uint64_t clean_ptr = extract64(ptr, 0, 56);
+    uint8_t *tags = page_get_target_data(clean_ptr);
+    uintptr_t index;
+
+    if (tags == NULL) {
+        int flags = page_get_flags(clean_ptr);
+        size_t alloc_size;
+
+        if (flags & PAGE_SHARED) {
+            /* There may be multiple mappings; pretend not implemented.  */
+            return false;
+        } else if (!(flags & PAGE_VALID)) {
+            /* Page is invalid: SIGSEGV.  */
+            env->exception.vaddress = ptr;
+            cpu_restore_state(ENV_GET_CPU(env), ra, true);
+            raise_exception(env, EXCP_DATA_ABORT, 0, 1);
+        }
+
+        alloc_size = TARGET_PAGE_SIZE >> (LOG2_TAG_GRANULE + 1);
+        tags = page_alloc_target_data(clean_ptr, alloc_size);
+        assert(tags != NULL);
+    }
+    index = extract64(clean_ptr, LOG2_TAG_GRANULE + 1,
+                      TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1);
+    tags[index] = deposit32(tags[index], (clean_ptr & 1) * 4, 4, tag);
+    return true;
+#else
     /* Tag storage not implemented.  */
     return false;
+#endif
 }
 
 static int allocation_tag_from_addr(uint64_t ptr)
@@ -116,7 +172,7 @@ uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
      * access as unchecked.
      * This is similar to MemAttr != Tagged, which are also unchecked.
      */
-    mem_tag = get_allocation_tag(env, ptr);
+    mem_tag = get_allocation_tag(env, ptr, GETPC());
     if (mem_tag < 0) {
         goto pass;
     }
@@ -217,7 +273,7 @@ uint64_t HELPER(ldg)(CPUARMState *env, uint64_t ptr)
     int rtag = 0;
 
     if (allocation_tag_access_enabled(env, el, sctlr)) {
-        rtag = get_allocation_tag(env, ptr);
+        rtag = get_allocation_tag(env, ptr, GETPC());
         if (rtag < 0) {
             rtag = 0;
         }
@@ -232,7 +288,7 @@ uint64_t HELPER(stg)(CPUARMState *env, uint64_t ptr)
 
     if (allocation_tag_access_enabled(env, el, sctlr)) {
         int tag = allocation_tag_from_addr(ptr);
-        set_allocation_tag(env, ptr, tag);
+        set_allocation_tag(env, ptr, tag, GETPC());
     }
 
     /* Clean the pointer for use by stgz.  */
@@ -247,8 +303,10 @@ uint64_t HELPER(st2g)(CPUARMState *env, uint64_t ptr)
 
     if (allocation_tag_access_enabled(env, el, sctlr)) {
         int tag = allocation_tag_from_addr(ptr);
-        if (set_allocation_tag(env, ptr, tag)) {
-            set_allocation_tag(env, ptr + (1 << LOG2_TAG_GRANULE), tag);
+        uintptr_t ra = GETPC();
+
+        if (set_allocation_tag(env, ptr, tag, ra)) {
+            set_allocation_tag(env, ptr + (1 << LOG2_TAG_GRANULE), tag, ra);
         }
     }
 
@@ -261,6 +319,7 @@ uint64_t HELPER(ldgv)(CPUARMState *env, uint64_t ptr)
 {
     int el = arm_current_el(env);
     uint64_t sctlr = arm_sctlr(env, el);
+    uintptr_t ra = GETPC();
     uint64_t ret;
     int rtag, i;
 
@@ -269,7 +328,7 @@ uint64_t HELPER(ldgv)(CPUARMState *env, uint64_t ptr)
     }
 
     ptr = QEMU_ALIGN_DOWN(ptr, 1 << LOG2_TAG_GRANULE);
-    rtag = get_allocation_tag(env, ptr);
+    rtag = get_allocation_tag(env, ptr, ra);
     if (rtag < 0) {
         /* The entire page does not have tags.  */
         return 0;
@@ -278,7 +337,7 @@ uint64_t HELPER(ldgv)(CPUARMState *env, uint64_t ptr)
     i = extract32(ptr, LOG2_TAG_GRANULE, 4);
     ret = (uint64_t)rtag << i;
     for (i++; i < 16; i++) {
-        rtag = get_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE));
+        rtag = get_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE), ra);
         ret |= (uint64_t)rtag << i;
     }
 
@@ -289,6 +348,7 @@ void HELPER(stgv)(CPUARMState *env, uint64_t ptr, uint64_t val)
 {
     int el = arm_current_el(env);
     uint64_t sctlr = arm_sctlr(env, el);
+    uintptr_t ra = GETPC();
     int rtag, i;
 
     if (!allocation_tag_access_enabled(env, el, sctlr)) {
@@ -297,13 +357,13 @@ void HELPER(stgv)(CPUARMState *env, uint64_t ptr, uint64_t val)
 
     rtag = allocation_tag_from_addr(ptr);
     ptr = QEMU_ALIGN_DOWN(ptr, 1 << LOG2_TAG_GRANULE);
-    if (!set_allocation_tag(env, ptr, rtag)) {
+    if (!set_allocation_tag(env, ptr, rtag, ra)) {
         /* The entire page does not have tags.  */
         return;
     }
 
     i = extract32(ptr, LOG2_TAG_GRANULE, 4);
     for (i++; i < 16; i++) {
-        set_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE), rtag);
+        set_allocation_tag(env, ptr + (i << LOG2_TAG_GRANULE), rtag, ra);
     }
 }
-- 
2.17.2

[Qemu-devel] [PATCH 16/17] target/arm: Enable MTE

[Richard Henderson]

??? It does not yet work for system mode.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c   | 10 ++++++++++
 target/arm/cpu64.c |  1 +
 2 files changed, 11 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5eff6995ee..aae30207b9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -180,6 +180,16 @@ static void arm_cpu_reset(CPUState *s)
          * make no difference to the user-level emulation.
          */
         env->cp15.tcr_el[1].raw_tcr = (3ULL << 37);
+        /* Enable MTE allocation tags.  */
+        env->cp15.hcr_el2 |= HCR_ATA;
+        env->cp15.scr_el3 |= SCR_ATA;
+        env->cp15.sctlr_el[1] |= SCTLR_ATA0;
+        /* Enable synchronous tag check failures.  */
+        env->cp15.sctlr_el[1] |= 1ull << 38;
+#ifdef TARGET_AARCH64
+        /* Set MTE seed to non-zero value, otherwise RandomTag fails.  */
+        env->cp15.rgsr_el1 = 0x123400;
+#endif
 #else
         /* Reset into the highest available EL */
         if (arm_feature(env, ARM_FEATURE_EL3)) {
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 64fbe75eca..49fdad69ce 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -374,6 +374,7 @@ static void aarch64_max_initfn(Object *obj)
 
         t = cpu->isar.id_aa64pfr1;
         t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
+        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2);
         cpu->isar.id_aa64pfr1 = t;
 
         t = cpu->isar.id_aa64mmfr1;
-- 
2.17.2

[Qemu-devel] [PATCH 17/17] tests/tcg/aarch64: Add mte smoke tests

[Richard Henderson]

??? Requires a quite recent aarch64 assembler.  Use .inst instead?

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/aarch64/mte-1.c         | 27 +++++++++++++++++++++
 tests/tcg/aarch64/mte-2.c         | 39 +++++++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  4 ++++
 3 files changed, 70 insertions(+)
 create mode 100644 tests/tcg/aarch64/mte-1.c
 create mode 100644 tests/tcg/aarch64/mte-2.c

diff --git a/tests/tcg/aarch64/mte-1.c b/tests/tcg/aarch64/mte-1.c
new file mode 100644
index 0000000000..740bf506f1
--- /dev/null
+++ b/tests/tcg/aarch64/mte-1.c
@@ -0,0 +1,27 @@
+/*
+ * Memory tagging, basic pass cases.
+ */
+
+#include <assert.h>
+
+asm(".arch armv8.5-a+memtag");
+
+int data[16 / sizeof(int)] __attribute__((aligned(16)));
+
+int main(int ac, char **av)
+{
+    int *p0 = data;
+    int *p1, *p2;
+    long c;
+
+    asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(1));
+    assert(p1 != p0);
+    asm("subp %0,%1,%2" : "=r"(c) : "r"(p0), "r"(p1));
+    assert(c == 0);
+
+    asm("stg [%0]" : : "r"(p1));
+    asm("ldg %0, [%1]" : "=r"(p2) : "r"(p0));
+    assert(p1 == p2);
+
+    return 0;
+}
diff --git a/tests/tcg/aarch64/mte-2.c b/tests/tcg/aarch64/mte-2.c
new file mode 100644
index 0000000000..4d2004ab41
--- /dev/null
+++ b/tests/tcg/aarch64/mte-2.c
@@ -0,0 +1,39 @@
+/*
+ * Memory tagging, basic fail cases.
+ */
+
+#include <assert.h>
+#include <signal.h>
+#include <stdlib.h>
+
+asm(".arch armv8.5-a+memtag");
+
+int data[16 / sizeof(int)] __attribute__((aligned(16)));
+
+void pass(int sig)
+{
+    exit(0);
+}
+
+int main(int ac, char **av)
+{
+    int *p0 = data;
+    int *p1, *p2;
+    long excl = 1;
+
+    /* Create two differently tagged pointers.  */
+    asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(excl));
+    asm("gmi %0,%1,%0" : "+r"(excl) : "r" (p1));
+    assert(excl != 1);
+    asm("irg %0,%1,%2" : "=r"(p2) : "r"(p0), "r"(excl));
+    assert(p1 != p2);
+
+    /* Store the tag from the first pointer.  */
+    asm("stg [%0]" : : "r"(p1));
+
+    *p1 = 0;
+    signal(SIGSEGV, pass);
+    *p2 = 0;
+
+    assert(0);
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 3d56e7c6ea..1c4ebe894c 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -19,4 +19,8 @@ AARCH64_TESTS += bti-1
 bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
 run-bti-1: QEMU += -cpu max,guarded_pages=on
 
+AARCH64_TESTS += mte-1 mte-2
+mte-%: CFLAGS += -O -g
+run-mte-%: QEMU += -cpu max
+
 TESTS:=$(AARCH64_TESTS)
-- 
2.17.2

Re: [Qemu-devel] [PATCH 17/17] tests/tcg/aarch64: Add mte smoke tests

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> ??? Requires a quite recent aarch64 assembler.  Use .inst instead?

How recent? Given buster is nearing release we could start using it as a
basis for the ARM cross compiler images. That is shipping gcc 8.2.0 as
of now.

>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/tcg/aarch64/mte-1.c         | 27 +++++++++++++++++++++
>  tests/tcg/aarch64/mte-2.c         | 39 +++++++++++++++++++++++++++++++
>  tests/tcg/aarch64/Makefile.target |  4 ++++
>  3 files changed, 70 insertions(+)
>  create mode 100644 tests/tcg/aarch64/mte-1.c
>  create mode 100644 tests/tcg/aarch64/mte-2.c
>
> diff --git a/tests/tcg/aarch64/mte-1.c b/tests/tcg/aarch64/mte-1.c
> new file mode 100644
> index 0000000000..740bf506f1
> --- /dev/null
> +++ b/tests/tcg/aarch64/mte-1.c
> @@ -0,0 +1,27 @@
> +/*
> + * Memory tagging, basic pass cases.
> + */
> +
> +#include <assert.h>
> +
> +asm(".arch armv8.5-a+memtag");
> +
> +int data[16 / sizeof(int)] __attribute__((aligned(16)));
> +
> +int main(int ac, char **av)
> +{
> +    int *p0 = data;
> +    int *p1, *p2;
> +    long c;
> +
> +    asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(1));
> +    assert(p1 != p0);
> +    asm("subp %0,%1,%2" : "=r"(c) : "r"(p0), "r"(p1));
> +    assert(c == 0);
> +
> +    asm("stg [%0]" : : "r"(p1));
> +    asm("ldg %0, [%1]" : "=r"(p2) : "r"(p0));
> +    assert(p1 == p2);
> +
> +    return 0;
> +}
> diff --git a/tests/tcg/aarch64/mte-2.c b/tests/tcg/aarch64/mte-2.c
> new file mode 100644
> index 0000000000..4d2004ab41
> --- /dev/null
> +++ b/tests/tcg/aarch64/mte-2.c
> @@ -0,0 +1,39 @@
> +/*
> + * Memory tagging, basic fail cases.
> + */
> +
> +#include <assert.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +
> +asm(".arch armv8.5-a+memtag");
> +
> +int data[16 / sizeof(int)] __attribute__((aligned(16)));
> +
> +void pass(int sig)
> +{
> +    exit(0);
> +}
> +
> +int main(int ac, char **av)
> +{
> +    int *p0 = data;
> +    int *p1, *p2;
> +    long excl = 1;
> +
> +    /* Create two differently tagged pointers.  */
> +    asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(excl));
> +    asm("gmi %0,%1,%0" : "+r"(excl) : "r" (p1));
> +    assert(excl != 1);
> +    asm("irg %0,%1,%2" : "=r"(p2) : "r"(p0), "r"(excl));
> +    assert(p1 != p2);
> +
> +    /* Store the tag from the first pointer.  */
> +    asm("stg [%0]" : : "r"(p1));
> +
> +    *p1 = 0;
> +    signal(SIGSEGV, pass);
> +    *p2 = 0;
> +
> +    assert(0);
> +}
> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
> index 3d56e7c6ea..1c4ebe894c 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -19,4 +19,8 @@ AARCH64_TESTS += bti-1
>  bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
>  run-bti-1: QEMU += -cpu max,guarded_pages=on
>
> +AARCH64_TESTS += mte-1 mte-2
> +mte-%: CFLAGS += -O -g
> +run-mte-%: QEMU += -cpu max
> +
>  TESTS:=$(AARCH64_TESTS)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 17/17] tests/tcg/aarch64: Add mte smoke tests

[Richard Henderson]

On 1/15/19 1:22 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> ??? Requires a quite recent aarch64 assembler.  Use .inst instead?
> 
> How recent? Given buster is nearing release we could start using it as a
> basis for the ARM cross compiler images. That is shipping gcc 8.2.0 as
> of now.

Not yet released.  The patches adding the support include

commit 3a0f69be5589d351453afebd0974992cff3ee4d1
Author: Sudakshina Das <sudi.das@arm.com>
Date:   Mon Nov 12 13:29:38 2018 +0000

    [BINUTILS, AARCH64, 8/8] Add data cache instructions
    for Memory Tagging Extension

but the most recent binutils release branch (2.31) is from June.


r~

Re: [Qemu-devel] [PATCH 01/17] target/arm: Add MTE_ACTIVE to tb_flags

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> When MTE is fully enabled, i.e. access to tags are enabled and
> tag checks affect the PE, then arrange to perform the check
> while stripping the TBI.
>
> The check is not yet implemented, just the plumbing to that point.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


> @@ -0,0 +1,32 @@
> +/*
> + * ARM v8.5-MemTag Operations
> + *
> + * Copyright (c) 2019 Linaro, Ltd.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.

Do you mean LGPL version 2.1 here, or GPL version 2?

> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "internals.h"
> +#include "exec/exec-all.h"
> +#include "exec/cpu_ldst.h"
> +#include "exec/helper-proto.h"
> +
> +
> +uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
> +{
> +    /* Only unchecked implemented so far.  */
> +    return sextract64(ptr, 0, 55);

Are you sure this is right? I think that unchecked accesses
should work the same as if MTE isn't active at all, ie
do all the stuff gen_top_byte_ignore() does. If you look
at the pseudocode for AArch64.MemSingle[]:
https://developer.arm.com/docs/ddi0596/b/shared-pseudocode-functions/aarch64-functionsmemory-pseudocode#AArch64.MemSingle.write.4

the address (hidden inside 'memaddrdesc' passed down to the
_Mem[] accessor isn't changed by any of the code guarded by
the "if HaveMTEExt()" conditional.

In fact both checked and unchecked accesses ought to do this to
get the vaddr to use from the input vaddr.

> +}
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index ee6f71c98f..0286507bae 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -339,7 +339,13 @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 src)
>  static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr)
>  {
>      TCGv_i64 clean = new_tmp_a64(s);
> -    gen_top_byte_ignore(s, clean, addr, s->tbid);
> +
> +    /* FIXME: SP+OFS is always unchecked.  */
> +    if (s->tbid && s->mte_active) {
> +        gen_helper_mte_check(clean, cpu_env, addr);
> +    } else {
> +        gen_top_byte_ignore(s, clean, addr, s->tbid);
> +    }
>      return clean;
>  }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 02/17] target/arm: Extract TCMA with ARMVAParameters

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/internals.h | 1 +
>  target/arm/helper.c    | 8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is TFSRE0_EL1, TFSR_EL1, TFSR_EL2, TFSR_EL3,
> RGSR_EL1, GCR_EL1, and PSTATE.TCO.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.h           |  5 +++++
>  target/arm/translate.h     | 11 ++++++++++
>  target/arm/helper.c        | 45 ++++++++++++++++++++++++++++++++++++++
>  target/arm/translate-a64.c | 11 ++++++++++
>  4 files changed, 72 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 22163c9c3f..c8b447e30a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -482,6 +482,11 @@ typedef struct CPUARMState {
>          uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
>          uint64_t vpidr_el2; /* Virtualization Processor ID Register */
>          uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */
> +#ifdef TARGET_AARCH64
> +        uint64_t tfsr_el[4]; /* tfsrel0_el1 is index 0.  */
> +        uint64_t gcr_el1;
> +        uint64_t rgsr_el1;
> +#endif

Are we going to add more fields inside this #ifdef or is it only
saving 12 words?

>      } cp15;
>
>      struct {
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 5a101e1c6d..a24757d3d7 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -204,6 +204,17 @@ static inline TCGv_i32 get_ahp_flag(void)
>      return ret;
>  }
>
> +/* Set bits within PSTATE.  */
> +static inline void set_pstate_bits(uint32_t bits)
> +{
> +    TCGv_i32 p = tcg_temp_new_i32();
> +
> +    tcg_gen_ld_i32(p, cpu_env, offsetof(CPUARMState, pstate));
> +    tcg_gen_ori_i32(p, p, bits);
> +    tcg_gen_st_i32(p, cpu_env, offsetof(CPUARMState, pstate));
> +    tcg_temp_free_i32(p);

Maybe assert() that all the bits in the input are in the
set that we actually store in env->pstate, to catch attempts
to set NZCV, nRW, etc this way ?

> +}
> +
>  /* Clear bits within PSTATE.  */
>  static inline void clear_pstate_bits(uint32_t bits)
>  {
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5a59fc4315..df43deb0f8 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5132,6 +5132,48 @@ static const ARMCPRegInfo pauth_reginfo[] = {
>        .fieldoffset = offsetof(CPUARMState, apib_key.hi) },
>      REGINFO_SENTINEL
>  };
> +
> +static uint64_t tco_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    return env->pstate & PSTATE_TCO;
> +}
> +
> +static void tco_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t val)
> +{
> +    env->pstate = (env->pstate & ~PSTATE_TCO) | (val & PSTATE_TCO);
> +}
> +
> +static const ARMCPRegInfo mte_reginfo[] = {
> +    { .name = "TFSRE0_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 6, .opc2 = 1,
> +      .access = PL1_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[0]) },
> +    { .name = "TFSR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 6, .crm = 5, .opc2 = 0,
> +      .access = PL1_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[1]) },
> +    { .name = "TFSR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 5, .opc2 = 0,
> +      .access = PL2_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[2]) },
> +    { .name = "TFSR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 6, .opc2 = 0,
> +      .access = PL3_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.tfsr_el[3]) },
> +    { .name = "RGSR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 5,
> +      .access = PL1_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.rgsr_el1) },
> +    { .name = "GCR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 6,
> +      .access = PL1_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.gcr_el1) },
> +    { .name = "TCO", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 0, .opc1 = 3, .crn = 4, .crm = 2, .opc2 = 7,

Shouldn't this have opc0 = 3 ?

> +      .type = ARM_CP_NO_RAW,
> +      .access = PL0_RW, .readfn = tco_read, .writefn = tco_write },
> +    REGINFO_SENTINEL

Missing GMID_EL1 ?

> +};
>  #endif
>
>  void register_cp_regs_for_features(ARMCPU *cpu)
> @@ -5923,6 +5965,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      if (cpu_isar_feature(aa64_pauth, cpu)) {
>          define_arm_cp_regs(cpu, pauth_reginfo);
>      }
> +    if (cpu_isar_feature(aa64_mte_insn_reg, cpu)) {
> +        define_arm_cp_regs(cpu, mte_reginfo);
> +    }
>  #endif
>  }
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 0286507bae..5c2577a9ac 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1668,6 +1668,17 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
>          s->base.is_jmp = DISAS_UPDATE;
>          break;
>
> +    case 0x1c: /* TCO */
> +        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
> +            goto do_unallocated;
> +        }
> +        if (crm & 1) {
> +            set_pstate_bits(PSTATE_TCO);
> +        } else {
> +            clear_pstate_bits(PSTATE_TCO);
> +        }
> +        break;

Don't we need to break the TB here or something to pick up
the new value of TCO when we generate code for a following
load or store ? (TCO is self-synchronizing so there is no
requirement for an ISB before it takes effect.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/17] target/arm: Implement ARMv8.5-MemTag

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Based-on: 20190110124951.15473-1-richard.henderson@linaro.org
> aka the TBID patch set, which itself is based on the BTI patch set.
>
> The full tree is available at
>
>   https://github.org/rth7680/qemu.git tgt-arm-mte
>
> This extension isl also spelled MTE in the ARM.
>
> This patch set only attempts to implement linux-user emulation.
> For system emulation, I still miss the new cache flushing insns (easy)
> and the out-of-band physical memory for the allocation tags (harder).
>
> From a few mis-steps in writing the test cases for the extension,
> I might suggest that some future kernel's userland ABI for this have
> TCR.TCMA0 = 1, so that legacy code that is *not* MTE aware can use
> a frame pointer without accidentally tripping left over stack tags.
> (As seen in patch 5, SP+OFF is unchecked per the ISA but FP+OFF is not.)
>
> OTOH, depending on the application, that does make it easier for an
> attack vector to clean the tag off the top of a pointer to bypass
> store checking.  So, tricky.

I'm working through review of this, but feel free to rebase on
current master (which has now got a pile of your other patches
in it, since I've just merged target-arm.next) without waiting
for me to finish going through it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/17] target/arm: Fill in helper_mte_check

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Implements the rules of "PE generation of Checked and
> Unchecked accesses" which aren't already covered by XXX.

What should the "XXX" be here ?

> Implements the rules of "PE handling of Tag Check Failure".
>
> Does not implement tag physical address space, so all
> operations reduce to unchecked so far.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/mte_helper.c | 80 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index a3226c44a4..6f4bc0aa04 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -25,8 +25,86 @@
>  #include "exec/helper-proto.h"
>
>
> +static int get_allocation_tag(CPUARMState *env, uint64_t ptr)
> +{
> +    /* Tag storage not implemented.  */
> +    return -1;
> +}
> +
> +static int allocation_tag_from_addr(uint64_t ptr)
> +{
> +    return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
> +}
> +
>  uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
>  {
> -    /* Only unchecked implemented so far.  */
> +    ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
> +    ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, true);
> +    int ptr_tag, mem_tag;
> +
> +    /*
> +     * If TBI is disabled, then the access is unchecked.
> +     * While we filtered out TBI0==0 && TBI1==0 in cpu_get_tb_cpu_state,
> +     * we did not save separate bits for TBI0 != TBI1.
> +     */
> +    if (!param.tbi) {
> +        /* Do not ignore the top byte.  */
> +        return ptr;
> +    }
> +
> +    /*
> +     * If TCMA is enabled, then physical tag 0 is unchecked.
> +     * Note the rules R0076 & R0077 are written with logical tags,
> +     * and we need the physical tag below anyway.
> +     */
> +    ptr_tag = allocation_tag_from_addr(ptr);
> +    if (param.tcma && ptr_tag == 0) {
> +        goto pass;
> +    }
> +
> +    /*
> +     * If an access is made to an address that does not provide tag storage,
> +     * the result is implementation defined (R0006).  We choose to treat the
> +     * access as unchecked.
> +     * This is similar to MemAttr != Tagged, which are also unchecked.
> +     */
> +    mem_tag = get_allocation_tag(env, ptr);
> +    if (mem_tag < 0) {
> +        goto pass;
> +    }
> +
> +    /* If the tags do not match, the tag check operation fails.  */
> +    if (ptr_tag != mem_tag) {
> +        int el = arm_current_el(env);
> +        int tcf;
> +
> +        /* Indicate the tag check fail, both async and sync reporting.  */
> +        env->cp15.tfsr_el[el] |= 1 << param.select;

We should only update the TFSR bits if we're not taking a fault
(ie if tcf == 2).

> +
> +        if (el == 0) {
> +            /* FIXME: ARMv8.1-VHE S2 translation regime.  */
> +            tcf = extract64(env->cp15.sctlr_el[1], 38, 2);
> +        } else {
> +            tcf = extract64(env->cp15.sctlr_el[el], 40, 2);
> +        }
> +        if (tcf == 1) {
> +            /* Tag check fail causes a synchronous exception.  */
> +            CPUState *cs = ENV_GET_CPU(env);
> +
> +            /*
> +             * In restore_state_to_opc, we set the exception syndrome
> +             * for the load or store operation.  Do that first so we
> +             * may overwrite that with the syndrome for the tag check.
> +             */
> +            cpu_restore_state(cs, GETPC(), true);
> +            env->exception.vaddress = ptr;
> +            raise_exception(env, EXCP_DATA_ABORT,
> +                            syn_data_abort_no_iss(el != 0, 0, 0, 0, 0, 0x11),
> +                            exception_target_el(env));
> +        }
> +    }
> +
> + pass:
> +    /* Unchecked, or tag check pass.  Ignore the top byte.  */
>      return sextract64(ptr, 0, 55);
>  }

Remarks from earlier patch still apply about the 'unchecked or check pass'
code path needing to do the usual TBI stuff.

thanks
-- PMM

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
<richard.henderson@linaro.org> 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 <richard.henderson@linaro.org>
> ---
>  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

Re: [Qemu-devel] [PATCH 06/17] target/arm: Implement the IRG instruction

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.h    |  1 +
>  target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++
>  target/arm/translate-a64.c |  7 +++++
>  3 files changed, 63 insertions(+)
>
> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
> index fa4c371a47..7a6051fdab 100644
> --- a/target/arm/helper-a64.h
> +++ b/target/arm/helper-a64.h
> @@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>
>  DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
> +DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 6f4bc0aa04..1878393fc4 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)
>      return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
>  }
>
> +/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */

I don't understand this comment -- neither the pseudocode
function nor this code refer to GCR_EL1.

> +static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
> +{
> +    if (exclude != 0xffff) {
> +        int i;
> +        for (i = 0; i < offset; ++i) {
> +            do {
> +                tag = (tag + 1) & 15;
> +            } while (exclude & (1 << tag));
> +        }
> +    }
> +    return tag;

This doesn't seem to do the same thing as ChooseNonExcludedTag()
for the offset==0 case, or for the exclude == 0xffff case.

> +}
> +
> +static int choose_random_nonexcluded_tag(CPUARMState *env, uint16_t exclude)
> +{
> +    /* Ignore GCR_EL1.RRND.  Always produce deterministic results.  */

This comment is trying to say that our IMPDEF choice for GCR_EL1.RRND==1
is "behave the same as if RRND==0", right? I think that would be
clearer as a comment at the callsite, because if you're following
along with the pseudocode you expect the IRG code to do an
"if RRND == 1 then { choose_random_nonexcluded_tag(); } else { ... }".

thanks
-- PMM

Re: [Qemu-devel] [PATCH 07/17] target/arm: Implement ADDG, SUBG instructions

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.h    |  2 ++
>  target/arm/internals.h     |  3 ++
>  target/arm/mte_helper.c    | 34 ++++++++++++++++++
>  target/arm/translate-a64.c | 71 ++++++++++++++++++++++++++------------
>  4 files changed, 87 insertions(+), 23 deletions(-)
>
> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
> index 7a6051fdab..47577207b2 100644
> --- a/target/arm/helper-a64.h
> +++ b/target/arm/helper-a64.h
> @@ -105,3 +105,5 @@ DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>
>  DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
>  DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
> +DEF_HELPER_FLAGS_4(addg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
> +DEF_HELPER_FLAGS_4(subg, TCG_CALL_NO_RWG_SE, i64, env, i64, i32, i32)
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 2922324f63..a5a249b001 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1002,4 +1002,7 @@ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el,
>      return sctlr != 0;
>  }
>
> +/* We associate one allocation tag per 16 bytes, the minimum.  */
> +#define LOG2_TAG_GRANULE 4
> +
>  #endif
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 1878393fc4..e2b1a5dd40 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -163,3 +163,37 @@ uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
>      }
>      return address_with_allocation_tag(rn, rtag);
>  }
> +
> +uint64_t HELPER(addg)(CPUARMState *env, uint64_t ptr,
> +                      uint32_t offset, uint32_t tag_offset)
> +{
> +    int el = arm_current_el(env);
> +    uint64_t sctlr = arm_sctlr(env, el);
> +    int rtag = 0;
> +
> +    if (allocation_tag_access_enabled(env, el, sctlr)) {
> +        int start_tag = allocation_tag_from_addr(ptr);
> +        uint16_t exclude = env->cp15.gcr_el1;

I'm not entirely convinced about this extraction of the 'exclude'
field from GCR_EL1 by silently assigning it to a uint16_t;
it's a bit non-obvious.

> +        rtag = choose_nonexcluded_tag(start_tag, tag_offset, exclude);
> +    }
> +
> +    offset <<= LOG2_TAG_GRANULE;
> +    return address_with_allocation_tag(ptr + offset, rtag);
> +}
> +
> +uint64_t HELPER(subg)(CPUARMState *env, uint64_t ptr,
> +                      uint32_t offset, uint32_t tag_offset)
> +{
> +    int el = arm_current_el(env);
> +    uint64_t sctlr = arm_sctlr(env, el);
> +    int rtag = 0;
> +
> +    if (allocation_tag_access_enabled(env, el, sctlr)) {
> +        int start_tag = allocation_tag_from_addr(ptr);
> +        uint16_t exclude = env->cp15.gcr_el1;
> +        rtag = choose_nonexcluded_tag(start_tag, tag_offset, exclude);
> +    }
> +
> +    offset <<= LOG2_TAG_GRANULE;
> +    return address_with_allocation_tag(ptr - offset, rtag);
> +}

You could have done the shift of the offset at translate time,
but I guess it doesn't make any difference given everything else
we're doing.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 08/17] target/arm: Implement the GMI instruction

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.h    | 1 +
>  target/arm/mte_helper.c    | 6 ++++++
>  target/arm/translate-a64.c | 6 ++++++
>  3 files changed, 13 insertions(+)


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 09/17] target/arm: Implement the SUBP instruction

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate-a64.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 6583ad93b1..98ff60c161 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5111,19 +5111,39 @@ static void handle_crc32(DisasContext *s,
>   */
>  static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
>  {
> -    unsigned int sf, rm, opcode, rn, rd;
> +    unsigned int sf, rm, opcode, rn, rd, setflag;
>      sf = extract32(insn, 31, 1);
> +    setflag = extract32(insn, 29, 1);
>      rm = extract32(insn, 16, 5);
>      opcode = extract32(insn, 10, 6);
>      rn = extract32(insn, 5, 5);
>      rd = extract32(insn, 0, 5);
>
> -    if (extract32(insn, 29, 1)) {
> +    if (setflag && opcode != 0) {
>          unallocated_encoding(s);
>          return;
>      }
>
>      switch (opcode) {
> +    case 0: /* SUBP(S) */
> +        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
> +            goto do_unallocated;
> +        } else {
> +            TCGv_i64 tcg_n, tcg_m, tcg_d;
> +
> +            tcg_n = read_cpu_reg_sp(s, rn, true);
> +            tcg_m = read_cpu_reg_sp(s, rm, true);
> +            tcg_gen_sextract_i64(tcg_n, tcg_n, 0, 55);
> +            tcg_gen_sextract_i64(tcg_m, tcg_m, 0, 55);

Shouldn't the lengths here be 56, not 55 ? We're doing the
sign-extend of bits [55:0] to 64 bits.

> +            tcg_d = cpu_reg(s, rd);
> +
> +            if (setflag) {
> +                gen_sub_CC(true, tcg_d, tcg_n, tcg_m);
> +            } else {
> +                tcg_gen_sub_i64(tcg_d, tcg_n, tcg_m);
> +            }
> +        }
> +        break;
>      case 2: /* UDIV */
>          handle_div(s, false, sf, rm, rn, rd);
>          break;
> --
> 2.17.2

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 10/17] target/arm: Implement LDG, STG, ST2G instructions

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.h    |   3 ++
>  target/arm/mte_helper.c    |  53 +++++++++++++++++++
>  target/arm/translate-a64.c | 106 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 162 insertions(+)
>

I'm going to skip review of this one, since it will need
updating for the STG/LDG spec change.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/17] target/arm: Implement the STGP instruction

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate-a64.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

Same remark as for patch 10.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 12/17] target/arm: Implement the LDGV and STGV instructions

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

These insns don't exist any more.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 13/17] target/arm: Set PSTATE.TCO on exception entry

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> R0085 specifies that exception handlers begin with tag checks overridden.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index df43deb0f8..1e9ccf0b2e 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8830,7 +8830,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>      qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
>                    env->elr_el[new_el]);
>
> -    pstate_write(env, PSTATE_DAIF | new_mode);
> +    pstate_write(env, PSTATE_DAIF | PSTATE_TCO | new_mode);
>      env->aarch64 = 1;
>      aarch64_restore_sp(env, new_el);

PSTATE_TCO being set doesn't affect codegen for non-MTE CPUs,
right?

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/17] target/arm: Implement ARMv8.5-MemTag

[Peter Maydell]

On Tue, 5 Feb 2019 at 19:42, Peter Maydell <peter.maydell@linaro.org> wrote:
> I'm working through review of this, but feel free to rebase on
> current master (which has now got a pile of your other patches
> in it, since I've just merged target-arm.next) without waiting
> for me to finish going through it.

OK, I've now finished review of it. I haven't looked at the
last few patches which are linux-user, because it doesn't
seem worth doing much review on them until we have at least
a proposed Linux userspace ABI for MemTag to compare them
against. (If we do want to do an implementation that precedes
the ABI being nailed down, we need to hide it behind a
defaults-to-off x-property.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 13/17] target/arm: Set PSTATE.TCO on exception entry

[Richard Henderson]

On 2/7/19 9:44 AM, Peter Maydell wrote:
> On Mon, 14 Jan 2019 at 01:12, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> R0085 specifies that exception handlers begin with tag checks overridden.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/arm/helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index df43deb0f8..1e9ccf0b2e 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -8830,7 +8830,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>      qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
>>                    env->elr_el[new_el]);
>>
>> -    pstate_write(env, PSTATE_DAIF | new_mode);
>> +    pstate_write(env, PSTATE_DAIF | PSTATE_TCO | new_mode);
>>      env->aarch64 = 1;
>>      aarch64_restore_sp(env, new_el);
> 
> PSTATE_TCO being set doesn't affect codegen for non-MTE CPUs,
> right?

TCO does not exist before MTE.  I shouldn't set it without MTE, I think, as
it's visible to the guest.

r~

Re: [Qemu-devel] [PATCH 01/17] target/arm: Add MTE_ACTIVE to tb_flags

[Richard Henderson]

On 2/5/19 11:06 AM, Peter Maydell wrote:
> On Mon, 14 Jan 2019 at 01:11, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> When MTE is fully enabled, i.e. access to tags are enabled and
>> tag checks affect the PE, then arrange to perform the check
>> while stripping the TBI.
>>
>> The check is not yet implemented, just the plumbing to that point.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
>> @@ -0,0 +1,32 @@
>> +/*
>> + * ARM v8.5-MemTag Operations
>> + *
>> + * Copyright (c) 2019 Linaro, Ltd.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
> 
> Do you mean LGPL version 2.1 here, or GPL version 2?

Exactly the same as all of the other Linaro contributed
files -- a confused mix.  ;-)

>> +uint64_t HELPER(mte_check)(CPUARMState *env, uint64_t ptr)
>> +{
>> +    /* Only unchecked implemented so far.  */
>> +    return sextract64(ptr, 0, 55);
> 
> Are you sure this is right?

Not exactly right, no.  I thought I had a fixme here, but that was the other
one you point out later.  Will fix.


r~

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Richard Henderson]

On 2/5/19 11:27 AM, Peter Maydell wrote:
>> +#ifdef TARGET_AARCH64
>> +        uint64_t tfsr_el[4]; /* tfsrel0_el1 is index 0.  */
>> +        uint64_t gcr_el1;
>> +        uint64_t rgsr_el1;
>> +#endif
> 
> Are we going to add more fields inside this #ifdef or is it only
> saving 12 words?

Just the 12 words here.  We've got plenty of other ifdefs though...

>> +/* Set bits within PSTATE.  */
>> +static inline void set_pstate_bits(uint32_t bits)
>> +{
>> +    TCGv_i32 p = tcg_temp_new_i32();
>> +
>> +    tcg_gen_ld_i32(p, cpu_env, offsetof(CPUARMState, pstate));
>> +    tcg_gen_ori_i32(p, p, bits);
>> +    tcg_gen_st_i32(p, cpu_env, offsetof(CPUARMState, pstate));
>> +    tcg_temp_free_i32(p);
> 
> Maybe assert() that all the bits in the input are in the
> set that we actually store in env->pstate, to catch attempts
> to set NZCV, nRW, etc this way ?

I suppose.  There's the clear_pstate_bits just below, which has a couple of users.

>> +      .type = ARM_CP_NO_RAW,
>> +      .access = PL0_RW, .readfn = tco_read, .writefn = tco_write },
>> +    REGINFO_SENTINEL
> 
> Missing GMID_EL1 ?

Err..  that's not in 00eac5, at least.

>> +    case 0x1c: /* TCO */
>> +        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
>> +            goto do_unallocated;
>> +        }
>> +        if (crm & 1) {
>> +            set_pstate_bits(PSTATE_TCO);
>> +        } else {
>> +            clear_pstate_bits(PSTATE_TCO);
>> +        }
>> +        break;
> 
> Don't we need to break the TB here or something to pick up
> the new value of TCO when we generate code for a following
> load or store?

Yep.  It's included in the (quite complex) MTE_ACTIVE.


r~

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Richard Henderson]

On 2/5/19 11:27 AM, Peter Maydell wrote:
>> +++ b/target/arm/translate-a64.c
>> @@ -1668,6 +1668,17 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
>>          s->base.is_jmp = DISAS_UPDATE;
>>          break;
>>
>> +    case 0x1c: /* TCO */
>> +        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
>> +            goto do_unallocated;
>> +        }
>> +        if (crm & 1) {
>> +            set_pstate_bits(PSTATE_TCO);
>> +        } else {
>> +            clear_pstate_bits(PSTATE_TCO);
>> +        }
>> +        break;
> Don't we need to break the TB here or something to pick up
> the new value of TCO when we generate code for a following
> load or store ? (TCO is self-synchronizing so there is no
> requirement for an ISB before it takes effect.)

Actually, we already break the TB here by default.


r~

Re: [Qemu-devel] [PATCH 06/17] target/arm: Implement the IRG instruction

[Richard Henderson]

On 2/7/19 8:47 AM, Peter Maydell wrote:
> On Mon, 14 Jan 2019 at 01:11, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/arm/helper-a64.h    |  1 +
>>  target/arm/mte_helper.c    | 55 ++++++++++++++++++++++++++++++++++++++
>>  target/arm/translate-a64.c |  7 +++++
>>  3 files changed, 63 insertions(+)
>>
>> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
>> index fa4c371a47..7a6051fdab 100644
>> --- a/target/arm/helper-a64.h
>> +++ b/target/arm/helper-a64.h
>> @@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
>>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>>
>>  DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
>> +DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
>> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
>> index 6f4bc0aa04..1878393fc4 100644
>> --- a/target/arm/mte_helper.c
>> +++ b/target/arm/mte_helper.c
>> @@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)
>>      return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
>>  }
>>
>> +/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in.  */
> 
> I don't understand this comment -- neither the pseudocode
> function nor this code refer to GCR_EL1.

That's changed since the first rev.  Now ChooseNonExcludedTag just has an
exclude argument, and the caller includes GCR_EL1.Exclude.

>> +static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
>> +{
>> +    if (exclude != 0xffff) {
>> +        int i;
>> +        for (i = 0; i < offset; ++i) {
>> +            do {
>> +                tag = (tag + 1) & 15;
>> +            } while (exclude & (1 << tag));
>> +        }
>> +    }
>> +    return tag;
> 
> This doesn't seem to do the same thing as ChooseNonExcludedTag()
> for the offset==0 case, or for the exclude == 0xffff case.

All of this has changed since the first rev too.

> This comment is trying to say that our IMPDEF choice for GCR_EL1.RRND==1
> is "behave the same as if RRND==0", right? I think that would be
> clearer as a comment at the callsite, because if you're following
> along with the pseudocode you expect the IRG code to do an
> "if RRND == 1 then { choose_random_nonexcluded_tag(); } else { ... }".

Thanks for the verbage.


r~

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Peter Maydell]

On Sun, 10 Feb 2019 at 01:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/5/19 11:27 AM, Peter Maydell wrote:
> >> +++ b/target/arm/translate-a64.c
> >> @@ -1668,6 +1668,17 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
> >>          s->base.is_jmp = DISAS_UPDATE;
> >>          break;
> >>
> >> +    case 0x1c: /* TCO */
> >> +        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
> >> +            goto do_unallocated;
> >> +        }
> >> +        if (crm & 1) {
> >> +            set_pstate_bits(PSTATE_TCO);
> >> +        } else {
> >> +            clear_pstate_bits(PSTATE_TCO);
> >> +        }
> >> +        break;
> > Don't we need to break the TB here or something to pick up
> > the new value of TCO when we generate code for a following
> > load or store ? (TCO is self-synchronizing so there is no
> > requirement for an ISB before it takes effect.)
>
> Actually, we already break the TB here by default.

Do we? I didn't see any code (apart from the handling
in the DAIFSet/Clear codepaths, which aren't used for TCO).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Richard Henderson]

On 2/10/19 1:40 PM, Peter Maydell wrote:
>> Actually, we already break the TB here by default.
> 
> Do we? I didn't see any code (apart from the handling
> in the DAIFSet/Clear codepaths, which aren't used for TCO).

At the start of the function:

    /* End the TB by default, chaining is ok.  */
    s->base.is_jmp = DISAS_TOO_MANY;

Since the change to TCO is from an immediate, the change to MTE_ACTIVE is also
constant, and so chaining will work.


r~

Re: [Qemu-devel] [PATCH 03/17] target/arm: Add MTE system registers

[Peter Maydell]

On Sun, 10 Feb 2019 at 22:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/10/19 1:40 PM, Peter Maydell wrote:
> >> Actually, we already break the TB here by default.
> >
> > Do we? I didn't see any code (apart from the handling
> > in the DAIFSet/Clear codepaths, which aren't used for TCO).
>
> At the start of the function:
>
>     /* End the TB by default, chaining is ok.  */
>     s->base.is_jmp = DISAS_TOO_MANY;

...that's only in the v2 patchset which you hadn't posted yet :-)

-- PMM