diff mbox series

[v3,11/21] exec: Add support for TARGET_TAGGED_ADDRESSES

Message ID 20210115224645.1196742-12-richard.henderson@linaro.org
State New
Headers show
Series target-arm: Implement ARMv8.5-MemTag, user mode | expand

Commit Message

Richard Henderson Jan. 15, 2021, 10:46 p.m. UTC
The AArch64 Linux ABI has always enabled TBI, but has historically
required that pointer tags be removed before a syscall.  This has
changed in the lead-up to ARMv8.5-MTE, in a way that affects the
ABI generically and not specifically to MTE.

This patch allows the target to indicate that (1) there are tags
and (2) whether or not they should be taken into account at the
syscall level.

Adjust g2h, guest_addr_valid, and guest_range_valid to ignore
pointer tags, similar to how TIF_TAGGED_ADDR alters __range_ok
in the arm64 kernel source.

The prctl syscall is not not yet updated, so this change by itself
has no visible effect.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu_ldst.h | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Peter Maydell Jan. 22, 2021, 2:13 p.m. UTC | #1
On Fri, 15 Jan 2021 at 22:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The AArch64 Linux ABI has always enabled TBI, but has historically
> required that pointer tags be removed before a syscall.  This has
> changed in the lead-up to ARMv8.5-MTE, in a way that affects the
> ABI generically and not specifically to MTE.
>
> This patch allows the target to indicate that (1) there are tags
> and (2) whether or not they should be taken into account at the
> syscall level.
>
> Adjust g2h, guest_addr_valid, and guest_range_valid to ignore
> pointer tags, similar to how TIF_TAGGED_ADDR alters __range_ok
> in the arm64 kernel source.
>
> The prctl syscall is not not yet updated, so this change by itself
> has no visible effect.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu_ldst.h | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index e62f4fba00..1df9b93e59 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -69,17 +69,31 @@ typedef uint64_t abi_ptr;
>  #define TARGET_ABI_FMT_ptr "%"PRIx64
>  #endif
>
> +static inline abi_ptr untagged_addr(abi_ptr x)
> +{
> +#ifdef TARGET_TAGGED_ADDRESSES
> +    if (current_cpu) {
> +        return cpu_untagged_addr(current_cpu, x);
> +    }
> +#endif
> +    return x;
> +}

The current_cpu global is a nasty hack and I don't like seeing
new usages of it. In particular, it's very difficult to
analyse in what places this will get called when current_cpu is
NULL and whether it's always OK to not clean the tag in that
situation.

thanks
-- PMM
Richard Henderson Jan. 26, 2021, 5:10 p.m. UTC | #2
On 1/22/21 4:13 AM, Peter Maydell wrote:
> On Fri, 15 Jan 2021 at 22:47, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> The AArch64 Linux ABI has always enabled TBI, but has historically
>> required that pointer tags be removed before a syscall.  This has
>> changed in the lead-up to ARMv8.5-MTE, in a way that affects the
>> ABI generically and not specifically to MTE.
>>
>> This patch allows the target to indicate that (1) there are tags
>> and (2) whether or not they should be taken into account at the
>> syscall level.
>>
>> Adjust g2h, guest_addr_valid, and guest_range_valid to ignore
>> pointer tags, similar to how TIF_TAGGED_ADDR alters __range_ok
>> in the arm64 kernel source.
>>
>> The prctl syscall is not not yet updated, so this change by itself
>> has no visible effect.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/exec/cpu_ldst.h | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
>> index e62f4fba00..1df9b93e59 100644
>> --- a/include/exec/cpu_ldst.h
>> +++ b/include/exec/cpu_ldst.h
>> @@ -69,17 +69,31 @@ typedef uint64_t abi_ptr;
>>  #define TARGET_ABI_FMT_ptr "%"PRIx64
>>  #endif
>>
>> +static inline abi_ptr untagged_addr(abi_ptr x)
>> +{
>> +#ifdef TARGET_TAGGED_ADDRESSES
>> +    if (current_cpu) {
>> +        return cpu_untagged_addr(current_cpu, x);
>> +    }
>> +#endif
>> +    return x;
>> +}
> 
> The current_cpu global is a nasty hack and I don't like seeing
> new usages of it. In particular, it's very difficult to
> analyse in what places this will get called when current_cpu is
> NULL and whether it's always OK to not clean the tag in that
> situation.

Well, that'll be a really lot of changes to add cpu/env as an argument to
get_user et al.

Let's see how easily coccinelle can fix em all up for me...


r~
diff mbox series

Patch

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index e62f4fba00..1df9b93e59 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -69,17 +69,31 @@  typedef uint64_t abi_ptr;
 #define TARGET_ABI_FMT_ptr "%"PRIx64
 #endif
 
+static inline abi_ptr untagged_addr(abi_ptr x)
+{
+#ifdef TARGET_TAGGED_ADDRESSES
+    if (current_cpu) {
+        return cpu_untagged_addr(current_cpu, x);
+    }
+#endif
+    return x;
+}
+
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
-#define g2h(x) ((void *)((uintptr_t)(abi_ptr)(x) + guest_base))
+static inline void *g2h(abi_ulong x)
+{
+    return (void *)((uintptr_t)untagged_addr(x) + guest_base);
+}
 
 static inline bool guest_addr_valid(abi_ulong x)
 {
-    return x <= GUEST_ADDR_MAX;
+    return untagged_addr(x) <= GUEST_ADDR_MAX;
 }
 
 static inline bool guest_range_valid(abi_ulong start, abi_ulong len)
 {
-    return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
+    return len - 1 <= GUEST_ADDR_MAX &&
+           untagged_addr(start) <= GUEST_ADDR_MAX - len + 1;
 }
 
 #define h2g_valid(x) \