[PATCH v4 07/13] target/arm: ldg on canonical tag loads the tag

Gabriel Brookman posted 13 patches 1 month ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Laurent Vivier <laurent@vivier.eu>
[PATCH v4 07/13] target/arm: ldg on canonical tag loads the tag
Posted by Gabriel Brookman 1 month ago
According to ARM ARM, section "Memory Tagging Region Types", loading
tags from canonically tagged regions should use the canonical tags, not
allocation tags.

Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
---
 target/arm/tcg/mte_helper.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index b54fbd11c0..07797aecf9 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -313,6 +313,11 @@ uint64_t HELPER(ldg)(CPUARMState *env, uint64_t ptr, uint64_t xt)
     /* Load if page supports tags. */
     if (mem) {
         rtag = load_tag1(ptr, mem);
+    } else {
+        uint64_t bit55 = extract64(ptr, 55, 1);
+        if (canonical_tagging_enabled(env, bit55)) {
+            rtag = 0xF * bit55;
+        }
     }
 
     return address_with_allocation_tag(xt, rtag);
@@ -463,8 +468,10 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr)
     void *tag_mem;
     uint64_t ret;
     int shift;
+    bool bit55;
 
     ptr = QEMU_ALIGN_DOWN(ptr, gm_bs_bytes);
+    bit55 = extract64(ptr, 55, 1);
 
     /* Trap if accessing an invalid page.  */
     tag_mem = allocation_tag_mem(env, mmu_idx, ptr, MMU_DATA_LOAD,
@@ -472,6 +479,34 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr)
 
     /* The tag is squashed to zero if the page does not support tags.  */
     if (!tag_mem) {
+        /* Load canonical value if mtx is set (untagged memory region) */
+        if (canonical_tagging_enabled(env, bit55)) {
+            switch (gm_bs) {
+            case 3:
+                /* 32 bytes -> 2 tags -> 8 result bits */
+                ret = -(uint8_t)bit55;
+                break;
+            case 4:
+                /* 64 bytes -> 4 tags -> 16 result bits */
+                ret = -(uint16_t)bit55;
+                break;
+            case 5:
+                /* 128 bytes -> 8 tags -> 32 result bits */
+                ret = -(uint32_t)bit55;
+                break;
+            case 6:
+                /* 256 bytes -> 16 tags -> 64 result bits */
+                return -(uint64_t)bit55;
+            default:
+                /*
+                 * CPU configured with unsupported/invalid gm blocksize.
+                 * This is detected early in arm_cpu_realizefn.
+                 */
+                g_assert_not_reached();
+            }
+            shift = extract64(ptr, LOG2_TAG_GRANULE, 4) * 4;
+            return ret << shift;
+        }
         return 0;
     }
 

-- 
2.52.0
Re: [PATCH v4 07/13] target/arm: ldg on canonical tag loads the tag
Posted by Richard Henderson 3 days, 22 hours ago
On 3/10/26 08:59, Gabriel Brookman wrote:
> According to ARM ARM, section "Memory Tagging Region Types", loading
> tags from canonically tagged regions should use the canonical tags, not
> allocation tags.
> 
> Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
> ---
>   target/arm/tcg/mte_helper.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
> index b54fbd11c0..07797aecf9 100644
> --- a/target/arm/tcg/mte_helper.c
> +++ b/target/arm/tcg/mte_helper.c
> @@ -313,6 +313,11 @@ uint64_t HELPER(ldg)(CPUARMState *env, uint64_t ptr, uint64_t xt)
>       /* Load if page supports tags. */
>       if (mem) {
>           rtag = load_tag1(ptr, mem);
> +    } else {
> +        uint64_t bit55 = extract64(ptr, 55, 1);
> +        if (canonical_tagging_enabled(env, bit55)) {
> +            rtag = 0xF * bit55;
> +        }
>       }

Rather than look up MTX again, just pass it down from the translator with a new argument.

> @@ -472,6 +479,34 @@ uint64_t HELPER(ldgm)(CPUARMState *env, uint64_t ptr)
>   
>       /* The tag is squashed to zero if the page does not support tags.  */
>       if (!tag_mem) {
> +        /* Load canonical value if mtx is set (untagged memory region) */
> +        if (canonical_tagging_enabled(env, bit55)) {

Likewise.

> +            switch (gm_bs) {
> +            case 3:
> +                /* 32 bytes -> 2 tags -> 8 result bits */
> +                ret = -(uint8_t)bit55;
> +                break;
> +            case 4:
> +                /* 64 bytes -> 4 tags -> 16 result bits */
> +                ret = -(uint16_t)bit55;
> +                break;

This doesn't do what you expect, because uint8_t and uint16_t promote to int.

   ret = (uint8_t)-bit55;

will promote bool to int, negate, then truncate to {0, 0xff}.
Whether or not that's better than

   ret = bit55 ? 0xff : 0;

I don't know off-hand.


r~