diff mbox series

[v4,05/12] target/arm: Fix unaligned checks for mte_check1, mte_probe1

Message ID 20210406174031.64299-6-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm mte fixes | expand

Commit Message

Richard Henderson April 6, 2021, 5:40 p.m. UTC 
We were incorrectly assuming that only the first byte of an MTE access
is checked against the tags.  But per the ARM, unaligned accesses are
pre-decomposed into single-byte accesses.  So by the time we reach the
actual MTE check in the ARM pseudocode, all accesses are aligned.

We cannot tell a priori whether or not a given scalar access is aligned,
therefore we must at least check.  Use mte_probe_int, which is already
set up for checking multiple granules.

Buglink: https://bugs.launchpad.net/bugs/1921948
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/mte_helper.c | 109 +++++++++++++---------------------------
 1 file changed, 35 insertions(+), 74 deletions(-)

-- 
2.25.1

Comments

Alex Bennée April 8, 2021, 9:05 a.m. UTC | #1 
Richard Henderson <richard.henderson@linaro.org> writes:

> We were incorrectly assuming that only the first byte of an MTE access

> is checked against the tags.  But per the ARM, unaligned accesses are

> pre-decomposed into single-byte accesses.  So by the time we reach the

> actual MTE check in the ARM pseudocode, all accesses are aligned.

>

> We cannot tell a priori whether or not a given scalar access is aligned,

> therefore we must at least check.  Use mte_probe_int, which is already

> set up for checking multiple granules.

>

> Buglink: https://bugs.launchpad.net/bugs/1921948

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Tested-by: Alex Bennée <alex.bennee@linaro.org>


(tested with hand crafted kunit test)

> ---

>  target/arm/mte_helper.c | 109 +++++++++++++---------------------------

>  1 file changed, 35 insertions(+), 74 deletions(-)

>

> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c

> index 144bfa4a51..619c4b9351 100644

> --- a/target/arm/mte_helper.c

> +++ b/target/arm/mte_helper.c

> @@ -617,80 +617,6 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,

>      }

>  }

>  

> -/*

> - * Perform an MTE checked access for a single logical or atomic access.

> - */

> -static bool mte_probe1_int(CPUARMState *env, uint32_t desc, uint64_t ptr,

> -                           uintptr_t ra, int bit55)

> -{

> -    int mem_tag, mmu_idx, ptr_tag, size;

> -    MMUAccessType type;

> -    uint8_t *mem;

> -

> -    ptr_tag = allocation_tag_from_addr(ptr);

> -

> -    if (tcma_check(desc, bit55, ptr_tag)) {

> -        return true;

> -    }

> -

> -    mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);

> -    type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;

> -    size = FIELD_EX32(desc, MTEDESC, ESIZE);

> -

> -    mem = allocation_tag_mem(env, mmu_idx, ptr, type, size,

> -                             MMU_DATA_LOAD, 1, ra);

> -    if (!mem) {

> -        return true;

> -    }

> -

> -    mem_tag = load_tag1(ptr, mem);

> -    return ptr_tag == mem_tag;

> -}

> -

> -/*

> - * No-fault version of mte_check1, to be used by SVE for MemSingleNF.

> - * Returns false if the access is Checked and the check failed.  This

> - * is only intended to probe the tag -- the validity of the page must

> - * be checked beforehand.

> - */

> -bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)

> -{

> -    int bit55 = extract64(ptr, 55, 1);

> -

> -    /* If TBI is disabled, the access is unchecked. */

> -    if (unlikely(!tbi_check(desc, bit55))) {

> -        return true;

> -    }

> -

> -    return mte_probe1_int(env, desc, ptr, 0, bit55);

> -}

> -

> -uint64_t mte_check1(CPUARMState *env, uint32_t desc,

> -                    uint64_t ptr, uintptr_t ra)

> -{

> -    int 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;

> -    }

> -

> -    if (unlikely(!mte_probe1_int(env, desc, ptr, ra, bit55))) {

> -        mte_check_fail(env, desc, ptr, ra);

> -    }

> -

> -    return useronly_clean_ptr(ptr);

> -}

> -

> -uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)

> -{

> -    return mte_check1(env, desc, ptr, GETPC());

> -}

> -

> -/*

> - * Perform an MTE checked access for multiple logical accesses.

> - */

> -

>  /**

>   * checkN:

>   * @tag: tag memory to test

> @@ -882,6 +808,41 @@ uint64_t HELPER(mte_checkN)(CPUARMState *env, uint32_t desc, uint64_t ptr)

>      return mte_checkN(env, desc, ptr, GETPC());

>  }

>  

> +uint64_t mte_check1(CPUARMState *env, uint32_t desc,

> +                    uint64_t ptr, uintptr_t ra)

> +{

> +    uint64_t fault;

> +    uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);

> +    int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);

> +

> +    if (unlikely(ret == 0)) {

> +        mte_check_fail(env, desc, fault, ra);

> +    } else if (ret < 0) {

> +        return ptr;

> +    }

> +    return useronly_clean_ptr(ptr);

> +}

> +

> +uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)

> +{

> +    return mte_check1(env, desc, ptr, GETPC());

> +}

> +

> +/*

> + * No-fault version of mte_check1, to be used by SVE for MemSingleNF.

> + * Returns false if the access is Checked and the check failed.  This

> + * is only intended to probe the tag -- the validity of the page must

> + * be checked beforehand.

> + */

> +bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)

> +{

> +    uint64_t fault;

> +    uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);

> +    int ret = mte_probe_int(env, desc, ptr, 0, total, &fault);

> +

> +    return ret != 0;

> +}

> +

>  /*

>   * Perform an MTE checked access for DC_ZVA.

>   */



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 144bfa4a51..619c4b9351 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -617,80 +617,6 @@  static void mte_check_fail(CPUARMState *env, uint32_t desc,
     }
 }
 
-/*
- * Perform an MTE checked access for a single logical or atomic access.
- */
-static bool mte_probe1_int(CPUARMState *env, uint32_t desc, uint64_t ptr,
-                           uintptr_t ra, int bit55)
-{
-    int mem_tag, mmu_idx, ptr_tag, size;
-    MMUAccessType type;
-    uint8_t *mem;
-
-    ptr_tag = allocation_tag_from_addr(ptr);
-
-    if (tcma_check(desc, bit55, ptr_tag)) {
-        return true;
-    }
-
-    mmu_idx = FIELD_EX32(desc, MTEDESC, MIDX);
-    type = FIELD_EX32(desc, MTEDESC, WRITE) ? MMU_DATA_STORE : MMU_DATA_LOAD;
-    size = FIELD_EX32(desc, MTEDESC, ESIZE);
-
-    mem = allocation_tag_mem(env, mmu_idx, ptr, type, size,
-                             MMU_DATA_LOAD, 1, ra);
-    if (!mem) {
-        return true;
-    }
-
-    mem_tag = load_tag1(ptr, mem);
-    return ptr_tag == mem_tag;
-}
-
-/*
- * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
- * Returns false if the access is Checked and the check failed.  This
- * is only intended to probe the tag -- the validity of the page must
- * be checked beforehand.
- */
-bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
-{
-    int bit55 = extract64(ptr, 55, 1);
-
-    /* If TBI is disabled, the access is unchecked. */
-    if (unlikely(!tbi_check(desc, bit55))) {
-        return true;
-    }
-
-    return mte_probe1_int(env, desc, ptr, 0, bit55);
-}
-
-uint64_t mte_check1(CPUARMState *env, uint32_t desc,
-                    uint64_t ptr, uintptr_t ra)
-{
-    int 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;
-    }
-
-    if (unlikely(!mte_probe1_int(env, desc, ptr, ra, bit55))) {
-        mte_check_fail(env, desc, ptr, ra);
-    }
-
-    return useronly_clean_ptr(ptr);
-}
-
-uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
-{
-    return mte_check1(env, desc, ptr, GETPC());
-}
-
-/*
- * Perform an MTE checked access for multiple logical accesses.
- */
-
 /**
  * checkN:
  * @tag: tag memory to test
@@ -882,6 +808,41 @@  uint64_t HELPER(mte_checkN)(CPUARMState *env, uint32_t desc, uint64_t ptr)
     return mte_checkN(env, desc, ptr, GETPC());
 }
 
+uint64_t mte_check1(CPUARMState *env, uint32_t desc,
+                    uint64_t ptr, uintptr_t ra)
+{
+    uint64_t fault;
+    uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
+    int ret = mte_probe_int(env, desc, ptr, ra, total, &fault);
+
+    if (unlikely(ret == 0)) {
+        mte_check_fail(env, desc, fault, ra);
+    } else if (ret < 0) {
+        return ptr;
+    }
+    return useronly_clean_ptr(ptr);
+}
+
+uint64_t HELPER(mte_check1)(CPUARMState *env, uint32_t desc, uint64_t ptr)
+{
+    return mte_check1(env, desc, ptr, GETPC());
+}
+
+/*
+ * No-fault version of mte_check1, to be used by SVE for MemSingleNF.
+ * Returns false if the access is Checked and the check failed.  This
+ * is only intended to probe the tag -- the validity of the page must
+ * be checked beforehand.
+ */
+bool mte_probe1(CPUARMState *env, uint32_t desc, uint64_t ptr)
+{
+    uint64_t fault;
+    uint32_t total = FIELD_EX32(desc, MTEDESC, ESIZE);
+    int ret = mte_probe_int(env, desc, ptr, 0, total, &fault);
+
+    return ret != 0;
+}
+
 /*
  * Perform an MTE checked access for DC_ZVA.
  */