[PATCH RFC v3 05/12] target/arm: removed TBI bits from MTE check logic

Gabriel Brookman posted 12 patches 1 month ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Peter Maydell <peter.maydell@linaro.org>
[PATCH RFC v3 05/12] target/arm: removed TBI bits from MTE check logic
Posted by Gabriel Brookman 1 month ago
Previously, the TBI bit was used to mediate whether MTE checks happened.
This dependency isn't described in the ARM ARM. See D10.4.1, Tag Checked
Memory Accesses, specifically.

Decoupling tag checking from TBI is required to correctly implement
canonical tag checking, which must function even when TBI is disabled.

Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
---
 target/arm/tcg/helper-a64.c |  9 +--------
 target/arm/tcg/hflags.c     | 11 ++++-------
 target/arm/tcg/mte_helper.c | 10 ----------
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ba1d775d81..f64025925d 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -1050,20 +1050,13 @@ static int mops_sizereg(uint32_t syndrome)
 }
 
 /*
- * Return true if TCMA and TBI bits mean we need to do MTE checks.
+ * Return true if the TCMA bit means we need to do MTE checks.
  * We only need to do this once per MOPS insn, not for every page.
  */
 static bool mte_checks_needed(uint64_t ptr, uint32_t desc)
 {
     int bit55 = extract64(ptr, 55, 1);
 
-    /*
-     * Note that tbi_check() returns true for "access checked" but
-     * tcma_check() returns true for "access unchecked".
-     */
-    if (!tbi_check(desc, bit55)) {
-        return false;
-    }
     return !tcma_check(desc, bit55, allocation_tag_from_addr(ptr));
 }
 
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index c4696af5d8..80d7ea9349 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -405,15 +405,13 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
         /*
          * Set MTE_ACTIVE if any access may be Checked, and leave clear
          * if all accesses must be Unchecked:
-         * 1) If no TBI, then there are no tags in the address to check,
-         * 2) If Tag Check Override, then all accesses are Unchecked,
-         * 3) If Tag Check Fail == 0, then Checked access have no effect,
-         * 4) If no Allocation Tag Access, then all accesses are Unchecked.
+         * 1) If Tag Check Override, then all accesses are Unchecked,
+         * 2) If Tag Check Fail == 0, then Checked access have no effect,
+         * 3) If no Allocation Tag Access, then all accesses are Unchecked.
          */
         if (allocation_tag_access_enabled(env, el, sctlr)) {
             DP_TBFLAG_A64(flags, ATA, 1);
-            if (tbid
-                && !(env->pstate & PSTATE_TCO)
+            if (!(env->pstate & PSTATE_TCO)
                 && (sctlr & (el == 0 ? SCTLR_TCF0 : SCTLR_TCF))) {
                 DP_TBFLAG_A64(flags, MTE_ACTIVE, 1);
                 if (!EX_TBFLAG_A64(flags, UNPRIV)) {
@@ -439,7 +437,6 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
         }
         /* And again for unprivileged accesses, if required.  */
         if (EX_TBFLAG_A64(flags, UNPRIV)
-            && tbid
             && !(env->pstate & PSTATE_TCO)
             && (sctlr & SCTLR_TCF0)
             && allocation_tag_access_enabled(env, 0, sctlr)) {
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index 942bd4103d..f0880991b6 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -819,11 +819,6 @@ static int mte_probe_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
     bit55 = extract64(ptr, 55, 1);
     *fault = ptr;
 
-    /* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
-    if (unlikely(!tbi_check(desc, bit55))) {
-        return -1;
-    }
-
     ptr_tag = allocation_tag_from_addr(ptr);
 
     if (tcma_check(desc, bit55, ptr_tag)) {
@@ -960,11 +955,6 @@ uint64_t HELPER(mte_check_zva)(CPUARMState *env, uint32_t desc, uint64_t ptr)
 
     bit55 = extract64(ptr, 55, 1);
 
-    /* If TBI is disabled, the access is unchecked, and ptr is not dirty. */
-    if (unlikely(!tbi_check(desc, bit55))) {
-        return ptr;
-    }
-
     ptr_tag = allocation_tag_from_addr(ptr);
 
     if (tcma_check(desc, bit55, ptr_tag)) {

-- 
2.52.0
Re: [PATCH RFC v3 05/12] target/arm: removed TBI bits from MTE check logic
Posted by Richard Henderson 1 month ago
On 1/6/26 05:14, Gabriel Brookman wrote:
> Previously, the TBI bit was used to mediate whether MTE checks happened.
> This dependency isn't described in the ARM ARM. See D10.4.1, Tag Checked
> Memory Accesses, specifically.

I disagree.

See R_DRGYL: "Logical Address Tagging is disabled for the VA range being accessed", and 
R_TQHWL: "TCR_ELx.TBIn".

I can agree that you might want to factor this logical address tagging enabled test, since 
with FEAT_MTE_NO_ADDRESS_TAGS it then includes TCR_ELx.MTXn.


r~