[PATCH v2 00/17] target-arm: Implement ARMv8.5-MemTag, user mode

[PATCH v2 00/17] target-arm: Implement ARMv8.5-MemTag, user mode

[Richard Henderson]

Version 1 was back in October:
https://patchew.org/QEMU/20191015163254.12041-1-richard.henderson@linaro.org/

Although that post claims there was an actual v1 in March 2019,
I can't locate it, so... whatever.

This version -- call it 2 -- is the first attempt to implement the
actual in-progress kernel abi, rather than making up a private abi:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=devel/mte-v4

Based-on: 20200603011317.473934-1-richard.henderson@linaro.org
("target/arm: Implement ARMv8.5-MemTag, system mode")
Based-on: 20200520172800.8499-1-richard.henderson@linaro.org
("linux-user: User support for AArch64 BTI")

The full tree is available at:
https://github.com/rth7680/qemu/tree/tgt-arm-mte-user


r~


Richard Henderson (17):
  tcg: Introduce target-specific page data for user-only
  linux-user: Introduce PAGE_ANON
  linux-user: Check for overflow in access_ok
  linux-user: Tidy VERIFY_READ/VERIFY_WRITE
  bsd-user: Tidy VERIFY_READ/VERIFY_WRITE
  linux-user: Do not use guest_addr_valid for h2g_valid
  linux-user: Fix guest_addr_valid vs reserved_va
  exec: Add support for TARGET_TAGGED_ADDRESSES
  linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE
  linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG
  linux-user/aarch64: Implement PROT_MTE
  linux-user/aarch64: Pass syndrome to EXC_*_ABORT
  linux-user/aarch64: Signal SEGV_MTESERR for sync tag check fault
  linux-user/aarch64: Signal SEGV_MTEAERR for async tag check error
  target/arm: Add allocation tag storage for user mode
  target/arm: Enable MTE for user-only
  tests/tcg/aarch64: Add mte smoke tests

 bsd-user/qemu.h                     |  9 ++--
 include/exec/cpu-all.h              | 23 ++++++---
 include/exec/cpu_ldst.h             | 40 +++++++++++-----
 linux-user/aarch64/target_signal.h  |  3 ++
 linux-user/aarch64/target_syscall.h | 13 +++++
 linux-user/qemu.h                   | 19 +++++---
 linux-user/syscall_defs.h           |  1 +
 target/arm/cpu-param.h              |  3 ++
 tests/tcg/aarch64/mte.h             | 54 +++++++++++++++++++++
 accel/tcg/translate-all.c           | 28 +++++++++++
 bsd-user/main.c                     |  5 ++
 linux-user/aarch64/cpu_loop.c       | 60 +++++++++++++++++++-----
 linux-user/main.c                   |  5 ++
 linux-user/mmap.c                   | 28 +++++++----
 linux-user/syscall.c                | 73 ++++++++++++++++++++++++++++-
 target/arm/cpu.c                    | 17 +++++++
 target/arm/mte_helper.c             | 39 ++++++++++++++-
 target/arm/tlb_helper.c             | 27 ++++++++---
 tests/tcg/aarch64/mte-1.c           | 25 ++++++++++
 tests/tcg/aarch64/mte-2.c           | 42 +++++++++++++++++
 tests/tcg/aarch64/mte-3.c           | 47 +++++++++++++++++++
 tests/tcg/aarch64/Makefile.target   |  4 ++
 22 files changed, 502 insertions(+), 63 deletions(-)
 create mode 100644 tests/tcg/aarch64/mte.h
 create mode 100644 tests/tcg/aarch64/mte-1.c
 create mode 100644 tests/tcg/aarch64/mte-2.c
 create mode 100644 tests/tcg/aarch64/mte-3.c

-- 
2.25.1

[PATCH v2 01/17] tcg: Introduce target-specific page data for user-only

[Richard Henderson]

This data can be allocated by page_alloc_target_data() and
released by page_set_flags(start, end, prot | PAGE_RESET).

This data will be used to hold tag memory for AArch64 MTE.

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

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 2bd023d692..e5f564fa1f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -266,15 +266,21 @@ extern intptr_t qemu_host_page_mask;
 #define PAGE_EXEC      0x0004
 #define PAGE_BITS      (PAGE_READ | PAGE_WRITE | PAGE_EXEC)
 #define PAGE_VALID     0x0008
-/* original state of the write flag (used when tracking self-modifying
-   code */
+/*
+ * Original state of the write flag (used when tracking self-modifying code)
+ */
 #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
+/*
+ * 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 0x0020
+/* For use with page_set_flags: page is being replaced; target_data cleared. */
+#define PAGE_RESET     0x0040
+
 #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
 /* Target-specific bits that will be used via page_get_flags().  */
 #define PAGE_TARGET_1  0x0080
@@ -289,6 +295,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 42ce1dfcff..a65bc1d4c2 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;
@@ -2525,6 +2526,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
@@ -2539,6 +2541,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;
@@ -2552,10 +2556,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 40f03e3174..f9832609b0 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -599,6 +599,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
         }
     }
  the_end1:
+    page_flags |= PAGE_RESET;
     page_set_flags(start, start + len, page_flags);
  the_end:
     trace_target_mmap_complete(start);
@@ -787,9 +788,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 7f6700c54e..d190fb1122 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4315,8 +4315,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_RESET | PAGE_READ |
+                   (shmflg & SHM_RDONLY ? 0 : PAGE_WRITE));
 
     for (i = 0; i < N_SHM_REGIONS; i++) {
         if (!shm_regions[i].in_use) {
-- 
2.25.1

Re: [PATCH v2 01/17] tcg: Introduce target-specific page data for user-only

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This data can be allocated by page_alloc_target_data() and
> released by page_set_flags(start, end, prot | PAGE_RESET).
>
> This data will be used to hold tag memory for AArch64 MTE.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

> @@ -289,6 +295,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);

Could we have a doc comment for any new function that's got
global scope, please?

>  #endif
>
>  CPUArchState *cpu_copy(CPUArchState *env);

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

Can a PageDesc validly have p->target_data != NULL but
p->flags with PAGE_VALID not set ?

It's not clear to me why for a !PAGE_VALID page which
has target_data already we return that pointer but
if it doesn't have any we don't allocate: either
"always allocate" or "always return NULL for non-valid pages"
would seem more self-consistent.

> @@ -787,9 +788,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.  */

Is this something we're going to address later in the patchset?

>          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();

thanks
-- PMM

Re: [PATCH v2 01/17] tcg: Introduce target-specific page data for user-only

[Richard Henderson]

On 6/25/20 9:20 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> This data can be allocated by page_alloc_target_data() and
>> released by page_set_flags(start, end, prot | PAGE_RESET).
>>
>> This data will be used to hold tag memory for AArch64 MTE.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
> 
>> @@ -289,6 +295,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);
> 
> Could we have a doc comment for any new function that's got
> global scope, please?
> 
>>  #endif
>>
>>  CPUArchState *cpu_copy(CPUArchState *env);
> 
>> +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;
> 
> Can a PageDesc validly have p->target_data != NULL but
> p->flags with PAGE_VALID not set ?

No.  But we can be called for a page that is not mapped (returning NULL) and
can be called for a page that already has associated data (returning the old
value).

> It's not clear to me why for a !PAGE_VALID page which
> has target_data already we return that pointer but
> if it doesn't have any we don't allocate: either
> "always allocate" or "always return NULL for non-valid pages"
> would seem more self-consistent.

I was expecting a non-valid page to have no data.  I will rearrange this to

    ret = NULL;
    if (p->flags & PAGE_VALID) {
        ret = p->target_data;
        if (!ret) {
            p->target_data = ret = g_malloc0(size);
        }
    }

which is probably clearer.

>> +        /* FIXME: Move page flags and target_data for each page.  */
> 
> Is this something we're going to address later in the patchset?

I had not, but I should.  Will fix.


r~

Re: [PATCH v2 01/17] tcg: Introduce target-specific page data for user-only

[Richard Henderson]

On 6/25/20 9:20 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
>> @@ -787,9 +788,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.  */
> 
> Is this something we're going to address later in the patchset?

I've removed the comment.

The mremap system call is not as general as I think it should be.  It only
applies to MAP_SHARED vmas and returns EINVAL on MAP_PRIVATE.  Therefore, at
least for the MTE usage of target_data, there cannot be any.


r~

[PATCH v2 02/17] linux-user: Introduce PAGE_ANON

[Richard Henderson]

Record whether the backing page is anonymous, or if it has file
backing.  This will allow us to get close to the Linux AArch64
ABI for MTE, which allows tag memory only on ram-backed VMAs.

The real ABI allows tag memory on files, when those files are
on ram-backed filesystems, such as tmpfs.  We will not be able
to implement that in QEMU linux-user.

Thankfully, anonymous memory for malloc arenas is the primary
consumer of this feature, so this restricted version should
still be of use.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h | 2 ++
 linux-user/mmap.c      | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e5f564fa1f..3cac7750e4 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -277,6 +277,8 @@ extern intptr_t qemu_host_page_mask;
 #define PAGE_WRITE_INV 0x0020
 /* For use with page_set_flags: page is being replaced; target_data cleared. */
 #define PAGE_RESET     0x0040
+/* For linux-user, indicates that the page is MAP_ANON. */
+#define PAGE_ANON      0x0080
 
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
 /* FIXME: Code that sets/uses this is broken and needs to go away.  */
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index f9832609b0..fdd55986a1 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -599,6 +599,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
         }
     }
  the_end1:
+    if (flags & MAP_ANONYMOUS) {
+        page_flags |= PAGE_ANON;
+    }
     page_flags |= PAGE_RESET;
     page_set_flags(start, start + len, page_flags);
  the_end:
-- 
2.25.1

Re: [PATCH v2 02/17] linux-user: Introduce PAGE_ANON

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Record whether the backing page is anonymous, or if it has file
> backing.  This will allow us to get close to the Linux AArch64
> ABI for MTE, which allows tag memory only on ram-backed VMAs.
>
> The real ABI allows tag memory on files, when those files are
> on ram-backed filesystems, such as tmpfs.  We will not be able
> to implement that in QEMU linux-user.
>
> Thankfully, anonymous memory for malloc arenas is the primary
> consumer of this feature, so this restricted version should
> still be of use.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

thanks
-- PMM

[PATCH v2 03/17] linux-user: Check for overflow in access_ok

[Richard Henderson]

Verify that addr + size - 1 does not wrap around.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index d36b18b678..2bf35e66ac 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -467,12 +467,19 @@ extern unsigned long guest_stack_size;
 #define VERIFY_READ 0
 #define VERIFY_WRITE 1 /* implies read access */
 
-static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
+static inline bool access_ok(int type, abi_ulong addr, abi_ulong size)
 {
-    return guest_addr_valid(addr) &&
-           (size == 0 || guest_addr_valid(addr + size - 1)) &&
-           page_check_range((target_ulong)addr, size,
-                            (type == VERIFY_READ) ? PAGE_READ : (PAGE_READ | PAGE_WRITE)) == 0;
+    if (!guest_addr_valid(addr)) {
+        return false;
+    }
+    if (size != 0 &&
+        (addr + size - 1 < addr ||
+         !guest_addr_valid(addr + size - 1))) {
+        return false;
+    }
+    return page_check_range((target_ulong)addr, size,
+                            (type == VERIFY_READ) ? PAGE_READ :
+                            (PAGE_READ | PAGE_WRITE)) == 0;
 }
 
 /* NOTE __get_user and __put_user use host pointers and don't check access.
-- 
2.25.1

Re: [PATCH v2 03/17] linux-user: Check for overflow in access_ok

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Verify that addr + size - 1 does not wrap around.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/qemu.h | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)

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

thanks
-- PMM

[PATCH v2 04/17] linux-user: Tidy VERIFY_READ/VERIFY_WRITE

[Richard Henderson]

These constants are only ever used with access_ok, and friends.
Rather than translating them to PAGE_* bits, let them equal
the PAGE_* bits to begin.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 2bf35e66ac..a8dde8c609 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -464,8 +464,8 @@ extern unsigned long guest_stack_size;
 
 /* user access */
 
-#define VERIFY_READ 0
-#define VERIFY_WRITE 1 /* implies read access */
+#define VERIFY_READ  PAGE_READ
+#define VERIFY_WRITE (PAGE_READ | PAGE_WRITE)
 
 static inline bool access_ok(int type, abi_ulong addr, abi_ulong size)
 {
@@ -477,9 +477,7 @@ static inline bool access_ok(int type, abi_ulong addr, abi_ulong size)
          !guest_addr_valid(addr + size - 1))) {
         return false;
     }
-    return page_check_range((target_ulong)addr, size,
-                            (type == VERIFY_READ) ? PAGE_READ :
-                            (PAGE_READ | PAGE_WRITE)) == 0;
+    return page_check_range((target_ulong)addr, size, type) == 0;
 }
 
 /* NOTE __get_user and __put_user use host pointers and don't check access.
-- 
2.25.1

Re: [PATCH v2 04/17] linux-user: Tidy VERIFY_READ/VERIFY_WRITE

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These constants are only ever used with access_ok, and friends.
> Rather than translating them to PAGE_* bits, let them equal
> the PAGE_* bits to begin.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

Side note: at some point we would ideally want to support
syscalls that checked for write-access-only (ie some of
our VERIFY_WRITE uses should really be VERIFY_READ_WRITE
and some should be a true VERIFY_WRITE):
https://bugs.launchpad.net/qemu/+bug/1779955

thanks
-- PMM

[PATCH v2 05/17] bsd-user: Tidy VERIFY_READ/VERIFY_WRITE

[Richard Henderson]

These constants are only ever used with access_ok, and friends.
Rather than translating them to PAGE_* bits, let them equal
the PAGE_* bits to begin.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 bsd-user/qemu.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index f8bb1e5459..4076adabd0 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -218,13 +218,12 @@ extern unsigned long x86_stack_size;
 
 /* user access */
 
-#define VERIFY_READ 0
-#define VERIFY_WRITE 1 /* implies read access */
+#define VERIFY_READ  PAGE_READ
+#define VERIFY_WRITE (PAGE_READ | PAGE_WRITE)
 
-static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
+static inline bool access_ok(int type, abi_ulong addr, abi_ulong size)
 {
-    return page_check_range((target_ulong)addr, size,
-                            (type == VERIFY_READ) ? PAGE_READ : (PAGE_READ | PAGE_WRITE)) == 0;
+    return page_check_range((target_ulong)addr, size, type) == 0;
 }
 
 /* NOTE __get_user and __put_user use host pointers and don't check access. */
-- 
2.25.1

Re: [PATCH v2 05/17] bsd-user: Tidy VERIFY_READ/VERIFY_WRITE

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These constants are only ever used with access_ok, and friends.
> Rather than translating them to PAGE_* bits, let them equal
> the PAGE_* bits to begin.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  bsd-user/qemu.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

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

(We really should either get bsd-user up to speed and
maintained or just delete it...)

thanks
-- PMM

[PATCH v2 06/17] linux-user: Do not use guest_addr_valid for h2g_valid

[Richard Henderson]

This is the only use of guest_addr_valid that does not begin
with a guest address, but a host address being transformed to
a guest address.

We will shortly adjust guest_addr_valid to handle guest memory
tags, and the host address should not be subjected to that.

Move h2g_valid adjacent to the other h2g macros.

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

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index c14a48f65e..3930362e20 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -77,15 +77,16 @@ typedef uint64_t abi_ptr;
 #else
 #define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
 #endif
-#define h2g_valid(x) guest_addr_valid((unsigned long)(x) - guest_base)
 
 static inline int guest_range_valid(unsigned long start, unsigned long len)
 {
     return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
 }
 
+#define h2g_valid(x)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)
+
 #define h2g_nocheck(x) ({ \
-    unsigned long __ret = (unsigned long)(x) - guest_base; \
+    uintptr_t __ret = (uintptr_t)(x) - guest_base; \
     (abi_ptr)__ret; \
 })
 
-- 
2.25.1

Re: [PATCH v2 06/17] linux-user: Do not use guest_addr_valid for h2g_valid

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is the only use of guest_addr_valid that does not begin
> with a guest address, but a host address being transformed to
> a guest address.
>
> We will shortly adjust guest_addr_valid to handle guest memory
> tags, and the host address should not be subjected to that.
>
> Move h2g_valid adjacent to the other h2g macros.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu_ldst.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index c14a48f65e..3930362e20 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -77,15 +77,16 @@ typedef uint64_t abi_ptr;
>  #else
>  #define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
>  #endif
> -#define h2g_valid(x) guest_addr_valid((unsigned long)(x) - guest_base)
>
>  static inline int guest_range_valid(unsigned long start, unsigned long len)
>  {
>      return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
>  }
>
> +#define h2g_valid(x)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)

The old implementation returns true for
HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
(because there's a different definition of guest_addr_valid() there)
but this one does a range check even in that case.

> +
>  #define h2g_nocheck(x) ({ \
> -    unsigned long __ret = (unsigned long)(x) - guest_base; \
> +    uintptr_t __ret = (uintptr_t)(x) - guest_base; \
>      (abi_ptr)__ret; \
>  })

Why the type change? This seems unrelated.

thanks
-- PMM

Re: [PATCH v2 06/17] linux-user: Do not use guest_addr_valid for h2g_valid

[Richard Henderson]

On 6/25/20 9:34 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> This is the only use of guest_addr_valid that does not begin
>> with a guest address, but a host address being transformed to
>> a guest address.
>>
>> We will shortly adjust guest_addr_valid to handle guest memory
>> tags, and the host address should not be subjected to that.
>>
>> Move h2g_valid adjacent to the other h2g macros.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/exec/cpu_ldst.h | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
>> index c14a48f65e..3930362e20 100644
>> --- a/include/exec/cpu_ldst.h
>> +++ b/include/exec/cpu_ldst.h
>> @@ -77,15 +77,16 @@ typedef uint64_t abi_ptr;
>>  #else
>>  #define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
>>  #endif
>> -#define h2g_valid(x) guest_addr_valid((unsigned long)(x) - guest_base)
>>
>>  static inline int guest_range_valid(unsigned long start, unsigned long len)
>>  {
>>      return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
>>  }
>>
>> +#define h2g_valid(x)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)
> 
> The old implementation returns true for
> HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> (because there's a different definition of guest_addr_valid() there)
> but this one does a range check even in that case.

It's part and parcel with patch 1, wherein we are in fact attempting to limit
the guest address space to GUEST_ADDR_MAX.

That's why I put patch 1 first, so the behaviour change happens there.

>>  #define h2g_nocheck(x) ({ \
>> -    unsigned long __ret = (unsigned long)(x) - guest_base; \
>> +    uintptr_t __ret = (uintptr_t)(x) - guest_base; \
>>      (abi_ptr)__ret; \
>>  })
> 
> Why the type change? This seems unrelated.

Dropped.  Though at some point we should purge unsigned long, as there is
always a clearer type to use.


r~

Re: [PATCH v2 06/17] linux-user: Do not use guest_addr_valid for h2g_valid

[Richard Henderson]

On 7/11/20 12:30 PM, Richard Henderson wrote:
>> The old implementation returns true for
>> HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
>> (because there's a different definition of guest_addr_valid() there)
>> but this one does a range check even in that case.
> 
> It's part and parcel with patch 1, wherein we are in fact attempting to limit
> the guest address space to GUEST_ADDR_MAX.
> 
> That's why I put patch 1 first, so the behaviour change happens there.

Ho hum.  I've just realized the messages are sorted oddly in the mbox here, and
that the behaviour change is actually coming later in patch 7.

So, to summarize, I am intending a change here, it's just a matter of sorting
things so that one thing happens at a time.


r~

[PATCH v2 07/17] linux-user: Fix guest_addr_valid vs reserved_va

[Richard Henderson]

We must always use GUEST_ADDR_MAX, because even 32-bit hosts can
use -R <reserved_va> to restrict the memory address of the guest.

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

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 3930362e20..066cf33f70 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -72,11 +72,10 @@ typedef uint64_t abi_ptr;
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 #define g2h(x) ((void *)((unsigned long)(abi_ptr)(x) + guest_base))
 
-#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
-#define guest_addr_valid(x) (1)
-#else
-#define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
-#endif
+static inline bool guest_addr_valid(abi_ptr x)
+{
+    return x <= GUEST_ADDR_MAX;
+}
 
 static inline int guest_range_valid(unsigned long start, unsigned long len)
 {
-- 
2.25.1

Re: [PATCH v2 07/17] linux-user: Fix guest_addr_valid vs reserved_va

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We must always use GUEST_ADDR_MAX, because even 32-bit hosts can
> use -R <reserved_va> to restrict the memory address of the guest.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu_ldst.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Doesn't this run into trouble with the arm32 commpage?
The reserved_va is set there to 0xffff0000 (stopping
at the commpage), but the addresses within the commpage
themselves are still valid guest addresses.

thanks
-- PMM

Re: [PATCH v2 07/17] linux-user: Fix guest_addr_valid vs reserved_va

[Richard Henderson]

On 6/25/20 9:37 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> We must always use GUEST_ADDR_MAX, because even 32-bit hosts can
>> use -R <reserved_va> to restrict the memory address of the guest.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/exec/cpu_ldst.h | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> Doesn't this run into trouble with the arm32 commpage?
> The reserved_va is set there to 0xffff0000 (stopping
> at the commpage), but the addresses within the commpage
> themselves are still valid guest addresses.

Not really.  The commpage is Special, and gets allocated differently.  Normal
binaries work, e.g. our standard busybox ls.

I would imagine the corner case that doesn't work is that you couldn't issue a
syscall to the commpage, e.g.

    write(1, 0xfffff000, 1);

because the commpage is now outside the normal address space.

But given that it only matters with an explicit -R command-line option, this
falls into the Well Don't Do That Then category. This is a generic option, and
works as expected with other 32-bit guests.


r~

[PATCH v2 08/17] exec: Add support for TARGET_TAGGED_ADDRESSES

[Richard Henderson]

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 | 22 ++++++++++++++++++----
 target/arm/cpu-param.h  |  3 +++
 bsd-user/main.c         |  5 +++++
 linux-user/main.c       |  5 +++++
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 066cf33f70..d84847dc70 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
 
+#ifdef TARGET_TAGGED_ADDRESSES
+extern abi_ptr untagged_addr_mask;
+static inline abi_ptr untagged_addr(abi_ptr x)
+{
+    return x & untagged_addr_mask;
+}
+#else
+static inline abi_ptr untagged_addr(abi_ptr x) { return x; }
+#endif
+
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
-#define g2h(x) ((void *)((unsigned long)(abi_ptr)(x) + guest_base))
+static inline void *g2h(abi_ptr x)
+{
+    return (void *)(uintptr_t)(untagged_addr(x) + guest_base);
+}
 
 static inline bool guest_addr_valid(abi_ptr x)
 {
-    return x <= GUEST_ADDR_MAX;
+    return untagged_addr(x) <= GUEST_ADDR_MAX;
 }
 
-static inline int guest_range_valid(unsigned long start, unsigned long len)
+static inline bool guest_range_valid(abi_ptr 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)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 6321385b46..f922aa0650 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -20,6 +20,9 @@
 
 #ifdef CONFIG_USER_ONLY
 #define TARGET_PAGE_BITS 12
+# ifdef TARGET_AARCH64
+#  define TARGET_TAGGED_ADDRESSES
+# endif
 #else
 /*
  * ARMv7 and later CPUs have 4K pages minimum, but ARMv5 and v6
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0bfe46cff9..87c99fc6c2 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -45,6 +45,11 @@ unsigned long guest_base;
 bool have_guest_base;
 unsigned long reserved_va;
 
+#ifdef TARGET_TAGGED_ADDRESSES
+/* Default to no tagged addresses, i.e. all pointer bits valid.  */
+abi_ptr untagged_addr_mask = -1;
+#endif
+
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release;
 extern char **environ;
diff --git a/linux-user/main.c b/linux-user/main.c
index 3597e99bb1..8fcca3f5f1 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -103,6 +103,11 @@ static int last_log_mask;
 
 unsigned long reserved_va;
 
+#ifdef TARGET_TAGGED_ADDRESSES
+/* Default to no tagged addresses, i.e. all pointer bits valid.  */
+abi_ptr untagged_addr_mask = -1;
+#endif
+
 static void usage(int exitcode);
 
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
-- 
2.25.1

Re: [PATCH v2 08/17] exec: Add support for TARGET_TAGGED_ADDRESSES

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, 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 | 22 ++++++++++++++++++----
>  target/arm/cpu-param.h  |  3 +++
>  bsd-user/main.c         |  5 +++++
>  linux-user/main.c       |  5 +++++
>  4 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 066cf33f70..d84847dc70 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
>
> +#ifdef TARGET_TAGGED_ADDRESSES
> +extern abi_ptr untagged_addr_mask;
> +static inline abi_ptr untagged_addr(abi_ptr x)
> +{
> +    return x & untagged_addr_mask;
> +}
> +#else
> +static inline abi_ptr untagged_addr(abi_ptr x) { return x; }
> +#endif
> +
>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
> -#define g2h(x) ((void *)((unsigned long)(abi_ptr)(x) + guest_base))
> +static inline void *g2h(abi_ptr x)
> +{
> +    return (void *)(uintptr_t)(untagged_addr(x) + guest_base);
> +}
>
>  static inline bool guest_addr_valid(abi_ptr x)
>  {
> -    return x <= GUEST_ADDR_MAX;
> +    return untagged_addr(x) <= GUEST_ADDR_MAX;
>  }
>
> -static inline int guest_range_valid(unsigned long start, unsigned long len)
> +static inline bool guest_range_valid(abi_ptr start, abi_ulong len)

More type changes...

>  {
> -    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)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)

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

thanks
-- PMM

[PATCH v2 09/17] linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE

[Richard Henderson]

This is the prctl bit that controls whether syscalls accept tagged
addresses.  See Documentation/arm64/tagged-address-abi.rst in the
linux kernel.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/target_syscall.h |  4 ++++
 linux-user/syscall.c                | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h
index 995e475c73..5fb0bf4a5d 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -29,4 +29,8 @@ struct target_pt_regs {
 # define TARGET_PR_PAC_APDBKEY   (1 << 3)
 # define TARGET_PR_PAC_APGAKEY   (1 << 4)
 
+#define TARGET_PR_SET_TAGGED_ADDR_CTRL 55
+#define TARGET_PR_GET_TAGGED_ADDR_CTRL 56
+# define TARGET_PR_TAGGED_ADDR_ENABLE  (1UL << 0)
+
 #endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d190fb1122..e4da53c5b3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10460,6 +10460,29 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
                 }
             }
             return -TARGET_EINVAL;
+        case TARGET_PR_SET_TAGGED_ADDR_CTRL:
+            {
+                abi_ulong valid_mask = TARGET_PR_TAGGED_ADDR_ENABLE;
+
+                if ((arg2 & ~valid_mask) || arg3 || arg4 || arg5) {
+                    return -TARGET_EINVAL;
+                }
+                untagged_addr_mask = (arg2 & TARGET_PR_TAGGED_ADDR_ENABLE
+                                      ? MAKE_64BIT_MASK(0, 56) : -1);
+                return 0;
+            }
+        case TARGET_PR_GET_TAGGED_ADDR_CTRL:
+            {
+                abi_long ret = 0;
+
+                if (arg2 || arg3 || arg4 || arg5) {
+                    return -TARGET_EINVAL;
+                }
+                if (~untagged_addr_mask != 0) {
+                    ret |= TARGET_PR_TAGGED_ADDR_ENABLE;
+                }
+                return ret;
+            }
 #endif /* AARCH64 */
         case PR_GET_SECCOMP:
         case PR_SET_SECCOMP:
-- 
2.25.1

Re: [PATCH v2 09/17] linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is the prctl bit that controls whether syscalls accept tagged
> addresses.  See Documentation/arm64/tagged-address-abi.rst in the
> linux kernel.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/target_syscall.h |  4 ++++
>  linux-user/syscall.c                | 23 +++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
>

> +        case TARGET_PR_SET_TAGGED_ADDR_CTRL:
> +            {
> +                abi_ulong valid_mask = TARGET_PR_TAGGED_ADDR_ENABLE;
> +
> +                if ((arg2 & ~valid_mask) || arg3 || arg4 || arg5) {
> +                    return -TARGET_EINVAL;
> +                }
> +                untagged_addr_mask = (arg2 & TARGET_PR_TAGGED_ADDR_ENABLE
> +                                      ? MAKE_64BIT_MASK(0, 56) : -1);
> +                return 0;
> +            }

The spec says the tagged-address control is thread-scoped, not process-wide.

thanks
-- PMM

[PATCH v2 10/17] linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG

[Richard Henderson]

These prctl fields are required for the function of MTE.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/target_syscall.h |  9 ++++++
 linux-user/syscall.c                | 46 +++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h
index 5fb0bf4a5d..779ca0c805 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -32,5 +32,14 @@ struct target_pt_regs {
 #define TARGET_PR_SET_TAGGED_ADDR_CTRL 55
 #define TARGET_PR_GET_TAGGED_ADDR_CTRL 56
 # define TARGET_PR_TAGGED_ADDR_ENABLE  (1UL << 0)
+/* MTE tag check fault modes */
+# define TARGET_PR_MTE_TCF_SHIFT       1
+# define TARGET_PR_MTE_TCF_NONE        (0UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_SYNC        (1UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_ASYNC       (2UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_MASK        (3UL << TARGET_PR_MTE_TCF_SHIFT)
+/* MTE tag inclusion mask */
+# define TARGET_PR_MTE_TAG_SHIFT       3
+# define TARGET_PR_MTE_TAG_MASK        (0xffffUL << TARGET_PR_MTE_TAG_SHIFT)
 
 #endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e4da53c5b3..9cf3ce872d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10463,10 +10463,47 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         case TARGET_PR_SET_TAGGED_ADDR_CTRL:
             {
                 abi_ulong valid_mask = TARGET_PR_TAGGED_ADDR_ENABLE;
+                CPUARMState *env = cpu_env;
+                ARMCPU *cpu = env_archcpu(env);
+
+                if (cpu_isar_feature(aa64_mte, cpu)) {
+                    valid_mask |= TARGET_PR_MTE_TCF_MASK;
+                    valid_mask |= TARGET_PR_MTE_TAG_MASK;
+                }
 
                 if ((arg2 & ~valid_mask) || arg3 || arg4 || arg5) {
                     return -TARGET_EINVAL;
                 }
+
+                if (cpu_isar_feature(aa64_mte, cpu)) {
+                    switch (arg2 & TARGET_PR_MTE_TCF_MASK) {
+                    case TARGET_PR_MTE_TCF_NONE:
+                    case TARGET_PR_MTE_TCF_SYNC:
+                    case TARGET_PR_MTE_TCF_ASYNC:
+                        break;
+                    default:
+                        return -EINVAL;
+                    }
+
+                    /*
+                     * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+                     * Note that the syscall values are consistent with hw.
+                     */
+                    env->cp15.sctlr_el[1] =
+                        deposit64(env->cp15.sctlr_el[1], 38, 2,
+                                  arg2 >> TARGET_PR_MTE_TCF_SHIFT);
+
+                    /*
+                     * Write PR_MTE_TAG to GCR_EL1[Exclude].
+                     * Note that the syscall uses an include mask,
+                     * and hardware uses an exclude mask -- invert.
+                     */
+                    env->cp15.gcr_el1 =
+                        deposit64(env->cp15.gcr_el1, 0, 16,
+                                  ~arg2 >> TARGET_PR_MTE_TAG_SHIFT);
+                    arm_rebuild_hflags(env);
+                }
+
                 untagged_addr_mask = (arg2 & TARGET_PR_TAGGED_ADDR_ENABLE
                                       ? MAKE_64BIT_MASK(0, 56) : -1);
                 return 0;
@@ -10474,6 +10511,8 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         case TARGET_PR_GET_TAGGED_ADDR_CTRL:
             {
                 abi_long ret = 0;
+                CPUARMState *env = cpu_env;
+                ARMCPU *cpu = env_archcpu(env);
 
                 if (arg2 || arg3 || arg4 || arg5) {
                     return -TARGET_EINVAL;
@@ -10481,6 +10520,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
                 if (~untagged_addr_mask != 0) {
                     ret |= TARGET_PR_TAGGED_ADDR_ENABLE;
                 }
+                if (cpu_isar_feature(aa64_mte, cpu)) {
+                    /* See above. */
+                    ret |= (extract64(env->cp15.sctlr_el[1], 38, 2)
+                            << TARGET_PR_MTE_TCF_SHIFT);
+                    ret = deposit64(ret, TARGET_PR_MTE_TAG_SHIFT, 16,
+                                    ~env->cp15.gcr_el1);
+                }
                 return ret;
             }
 #endif /* AARCH64 */
-- 
2.25.1

Re: [PATCH v2 10/17] linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These prctl fields are required for the function of MTE.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

This API doesn't seem to be in the upstream kernel yet, right?

> +                    /*
> +                     * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
> +                     * Note that the syscall values are consistent with hw.
> +                     */
> +                    env->cp15.sctlr_el[1] =
> +                        deposit64(env->cp15.sctlr_el[1], 38, 2,
> +                                  arg2 >> TARGET_PR_MTE_TCF_SHIFT);

This actually will be per-thread since each linux-user thread has
its own CPU. You probably need to do something to make it be
inherited across clone and fork, though (assuming those are
the required semantics).

thanks
-- PMM

Re: [PATCH v2 10/17] linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG

[Richard Henderson]

On 6/25/20 11:50 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> These prctl fields are required for the function of MTE.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> This API doesn't seem to be in the upstream kernel yet, right?

It has finally landed in 5.10.
Thus the long delay since my last revision.

>> +                    /*
>> +                     * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
>> +                     * Note that the syscall values are consistent with hw.
>> +                     */
>> +                    env->cp15.sctlr_el[1] =
>> +                        deposit64(env->cp15.sctlr_el[1], 38, 2,
>> +                                  arg2 >> TARGET_PR_MTE_TCF_SHIFT);
> 
> This actually will be per-thread since each linux-user thread has
> its own CPU. You probably need to do something to make it be
> inherited across clone and fork, though (assuming those are
> the required semantics).

FWIW, these are a per-thread quantities.

>From arch/arm64/kernel/mte.c:
> void mte_thread_switch(struct task_struct *next)
> {
>         if (!system_supports_mte())
>                 return;
> 
>         /* avoid expensive SCTLR_EL1 accesses if no change */
>         if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
>                 update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
>         update_gcr_el1_excl(next->thread.gcr_user_incl);
> }

I don't think I have to do anything special wrt fork/clone, as env->cp15 will
get copied by our own cpu_copy().


r~

[PATCH v2 11/17] linux-user/aarch64: Implement PROT_MTE

[Richard Henderson]

Remember the PROT_MTE bit as PAGE_TARGET_2.
Otherwise this does not yet have effect.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h    |  1 +
 linux-user/syscall_defs.h |  1 +
 linux-user/mmap.c         | 20 ++++++++++++--------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 3cac7750e4..7ff10a8b08 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -286,6 +286,7 @@ extern intptr_t qemu_host_page_mask;
 #endif
 /* Target-specific bits that will be used via page_get_flags().  */
 #define PAGE_TARGET_1  0x0080
+#define PAGE_TARGET_2  0x0100
 
 #if defined(CONFIG_USER_ONLY)
 void page_dump(FILE *f);
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 36bdafb3f1..f2bfa3b17f 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1196,6 +1196,7 @@ struct target_winsize {
 
 #ifdef TARGET_AARCH64
 #define TARGET_PROT_BTI         0x10
+#define TARGET_PROT_MTE         0x20
 #endif
 
 /* Common */
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index fdd55986a1..b5618c40bd 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -84,18 +84,22 @@ static int validate_prot_to_pageflags(int *host_prot, int prot)
                | (prot & PROT_EXEC ? PROT_READ : 0);
 
 #ifdef TARGET_AARCH64
-    /*
-     * The PROT_BTI bit is only accepted if the cpu supports the feature.
-     * Since this is the unusual case, don't bother checking unless
-     * the bit has been requested.  If set and valid, record the bit
-     * within QEMU's page_flags as PAGE_TARGET_1.
-     */
-    if (prot & TARGET_PROT_BTI) {
+    {
         ARMCPU *cpu = ARM_CPU(thread_cpu);
-        if (cpu_isar_feature(aa64_bti, cpu)) {
+        /*
+         * The PROT_BTI bit is only accepted if the cpu supports the feature.
+         * If set and valid, record the bit within QEMU's page_flags
+         * as PAGE_TARGET_1.
+         */
+        if ((prot & TARGET_PROT_BTI) && cpu_isar_feature(aa64_bti, cpu)) {
             valid |= TARGET_PROT_BTI;
             page_flags |= PAGE_TARGET_1;
         }
+        /* Similarly for the PROT_MTE bit; set PAGE_TARGET_2. */
+        if ((prot & TARGET_PROT_MTE) && cpu_isar_feature(aa64_mte, cpu)) {
+            valid |= TARGET_PROT_MTE;
+            page_flags |= PAGE_TARGET_2;
+        }
     }
 #endif
 
-- 
2.25.1

Re: [PATCH v2 11/17] linux-user/aarch64: Implement PROT_MTE

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Remember the PROT_MTE bit as PAGE_TARGET_2.
> Otherwise this does not yet have effect.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu-all.h    |  1 +
>  linux-user/syscall_defs.h |  1 +
>  linux-user/mmap.c         | 20 ++++++++++++--------
>  3 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 3cac7750e4..7ff10a8b08 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -286,6 +286,7 @@ extern intptr_t qemu_host_page_mask;
>  #endif
>  /* Target-specific bits that will be used via page_get_flags().  */
>  #define PAGE_TARGET_1  0x0080
> +#define PAGE_TARGET_2  0x0100

This is the same as the bsd-user-only PAGE_RESERVED,
which seems unnecessarily confusing given we're not hurting
for available bits.

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

thanks
-- PMM

[PATCH v2 12/17] linux-user/aarch64: Pass syndrome to EXC_*_ABORT

[Richard Henderson]

A proper syndrome is required to fill in the proper si_code.
Use page_get_flags to determine permission vs translation
for user-only.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/cpu_loop.c | 23 ++++++++++++++++++++---
 target/arm/tlb_helper.c       | 27 +++++++++++++++++++++------
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 3cca637bb9..ce56e15be2 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -75,7 +75,7 @@
 void cpu_loop(CPUARMState *env)
 {
     CPUState *cs = env_cpu(env);
-    int trapnr;
+    int trapnr, ec, fsc;
     abi_long ret;
     target_siginfo_t info;
 
@@ -116,9 +116,26 @@ void cpu_loop(CPUARMState *env)
         case EXCP_DATA_ABORT:
             info.si_signo = TARGET_SIGSEGV;
             info.si_errno = 0;
-            /* XXX: check env->error_code */
-            info.si_code = TARGET_SEGV_MAPERR;
             info._sifields._sigfault._addr = env->exception.vaddress;
+
+            /* We should only arrive here with EC in {DATAABORT, INSNABORT}. */
+            ec = extract32(env->exception.syndrome, 26, 6);
+            assert(ec == 0x24 || ec == 0x20);
+
+            /* Both EC have the same format for FSC, or close enough. */
+            fsc = extract32(env->exception.syndrome, 0, 6);
+            switch (fsc) {
+            case 0x04 ... 0x07: /* Translation fault, level {0-3} */
+                info.si_code = TARGET_SEGV_MAPERR;
+                break;
+            case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */
+            case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
+                info.si_code = TARGET_SEGV_ACCERR;
+                break;
+            default:
+                g_assert_not_reached();
+            }
+
             queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
             break;
         case EXCP_UNALIGNED:
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index d4e6d37f4f..35d84ac24f 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -10,8 +10,6 @@
 #include "internals.h"
 #include "exec/exec-all.h"
 
-#if !defined(CONFIG_USER_ONLY)
-
 static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
                                             unsigned int target_el,
                                             bool same_el, bool ea,
@@ -51,6 +49,8 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
     return syn;
 }
 
+#if !defined(CONFIG_USER_ONLY)
+
 static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr,
                                             MMUAccessType access_type,
                                             int mmu_idx, ARMMMUFaultInfo *fi)
@@ -138,13 +138,28 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     ARMCPU *cpu = ARM_CPU(cs);
 
 #ifdef CONFIG_USER_ONLY
+    int flags, fsc, exc;
+    unsigned syn;
+
     cpu->env.exception.vaddress = address;
-    if (access_type == MMU_INST_FETCH) {
-        cs->exception_index = EXCP_PREFETCH_ABORT;
+
+    flags = page_get_flags(useronly_clean_ptr(address));
+    if (flags & PAGE_VALID) {
+        fsc = 0xf; /* permission fault, level 3 */
     } else {
-        cs->exception_index = EXCP_DATA_ABORT;
+        fsc = 0x7; /* translation fault, level 3 */
     }
-    cpu_loop_exit_restore(cs, retaddr);
+
+    cpu_restore_state(cs, retaddr, true);
+    if (access_type == MMU_INST_FETCH) {
+        exc = EXCP_PREFETCH_ABORT;
+        syn = syn_insn_abort(0, 0, 0, fsc);
+    } else {
+        exc = EXCP_DATA_ABORT;
+        syn = merge_syn_data_abort(cpu->env.exception.syndrome, 1, 0, 0, 0,
+                                   access_type == MMU_DATA_STORE, fsc);
+    }
+    raise_exception(&cpu->env, exc, syn, 1);
 #else
     hwaddr phys_addr;
     target_ulong page_size;
-- 
2.25.1

Re: [PATCH v2 12/17] linux-user/aarch64: Pass syndrome to EXC_*_ABORT

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> A proper syndrome is required to fill in the proper si_code.
> Use page_get_flags to determine permission vs translation
> for user-only.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/cpu_loop.c | 23 ++++++++++++++++++++---
>  target/arm/tlb_helper.c       | 27 +++++++++++++++++++++------
>  2 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 3cca637bb9..ce56e15be2 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -75,7 +75,7 @@
>  void cpu_loop(CPUARMState *env)
>  {
>      CPUState *cs = env_cpu(env);
> -    int trapnr;
> +    int trapnr, ec, fsc;
>      abi_long ret;
>      target_siginfo_t info;
>
> @@ -116,9 +116,26 @@ void cpu_loop(CPUARMState *env)
>          case EXCP_DATA_ABORT:
>              info.si_signo = TARGET_SIGSEGV;
>              info.si_errno = 0;
> -            /* XXX: check env->error_code */
> -            info.si_code = TARGET_SEGV_MAPERR;
>              info._sifields._sigfault._addr = env->exception.vaddress;
> +
> +            /* We should only arrive here with EC in {DATAABORT, INSNABORT}. */
> +            ec = extract32(env->exception.syndrome, 26, 6);

 ec = syn_get_ec(env->exception.syndrome);

(though it's in internals.h which implies moving it I guess).

> +            assert(ec == 0x24 || ec == 0x20);

assert(ec == EC_DATAABORT || ec == EC_INSNABORT);

> +
> +            /* Both EC have the same format for FSC, or close enough. */
> +            fsc = extract32(env->exception.syndrome, 0, 6);
> +            switch (fsc) {
> +            case 0x04 ... 0x07: /* Translation fault, level {0-3} */
> +                info.si_code = TARGET_SEGV_MAPERR;
> +                break;
> +            case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */
> +            case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
> +                info.si_code = TARGET_SEGV_ACCERR;
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }
> +
>              queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
>              break;
>          case EXCP_UNALIGNED:
> diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
> index d4e6d37f4f..35d84ac24f 100644
> --- a/target/arm/tlb_helper.c
> +++ b/target/arm/tlb_helper.c
> @@ -10,8 +10,6 @@
>  #include "internals.h"
>  #include "exec/exec-all.h"
>
> -#if !defined(CONFIG_USER_ONLY)
> -
>  static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
>                                              unsigned int target_el,
>                                              bool same_el, bool ea,
> @@ -51,6 +49,8 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
>      return syn;
>  }
>
> +#if !defined(CONFIG_USER_ONLY)
> +
>  static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr,
>                                              MMUAccessType access_type,
>                                              int mmu_idx, ARMMMUFaultInfo *fi)
> @@ -138,13 +138,28 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      ARMCPU *cpu = ARM_CPU(cs);
>
>  #ifdef CONFIG_USER_ONLY
> +    int flags, fsc, exc;
> +    unsigned syn;
> +
>      cpu->env.exception.vaddress = address;
> -    if (access_type == MMU_INST_FETCH) {
> -        cs->exception_index = EXCP_PREFETCH_ABORT;
> +
> +    flags = page_get_flags(useronly_clean_ptr(address));
> +    if (flags & PAGE_VALID) {
> +        fsc = 0xf; /* permission fault, level 3 */
>      } else {
> -        cs->exception_index = EXCP_DATA_ABORT;
> +        fsc = 0x7; /* translation fault, level 3 */
>      }
> -    cpu_loop_exit_restore(cs, retaddr);
> +
> +    cpu_restore_state(cs, retaddr, true);
> +    if (access_type == MMU_INST_FETCH) {
> +        exc = EXCP_PREFETCH_ABORT;
> +        syn = syn_insn_abort(0, 0, 0, fsc);
> +    } else {
> +        exc = EXCP_DATA_ABORT;
> +        syn = merge_syn_data_abort(cpu->env.exception.syndrome, 1, 0, 0, 0,
> +                                   access_type == MMU_DATA_STORE, fsc);
> +    }
> +    raise_exception(&cpu->env, exc, syn, 1);

Should we call arm_deliver_fault() the way the other series
does for linux-user alignment faults ?

thanks
-- PMM

[PATCH v2 13/17] linux-user/aarch64: Signal SEGV_MTESERR for sync tag check fault

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/target_signal.h | 2 ++
 linux-user/aarch64/cpu_loop.c      | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/linux-user/aarch64/target_signal.h b/linux-user/aarch64/target_signal.h
index ddd73169f0..777fb667fe 100644
--- a/linux-user/aarch64/target_signal.h
+++ b/linux-user/aarch64/target_signal.h
@@ -21,5 +21,7 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_SEGV_MTESERR  9  /* Synchronous ARM MTE exception */
+
 #define TARGET_ARCH_HAS_SETUP_FRAME
 #endif /* AARCH64_TARGET_SIGNAL_H */
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index ce56e15be2..41a68a57bc 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -132,6 +132,9 @@ void cpu_loop(CPUARMState *env)
             case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
                 info.si_code = TARGET_SEGV_ACCERR;
                 break;
+            case 0x11: /* Synchronous Tag Check Fault */
+                info.si_code = TARGET_SEGV_MTESERR;
+                break;
             default:
                 g_assert_not_reached();
             }
-- 
2.25.1

[PATCH v2 14/17] linux-user/aarch64: Signal SEGV_MTEAERR for async tag check error

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/target_signal.h |  1 +
 linux-user/aarch64/cpu_loop.c      | 34 +++++++++++++++++++++---------
 target/arm/mte_helper.c            | 10 +++++++++
 3 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/linux-user/aarch64/target_signal.h b/linux-user/aarch64/target_signal.h
index 777fb667fe..18013e1b23 100644
--- a/linux-user/aarch64/target_signal.h
+++ b/linux-user/aarch64/target_signal.h
@@ -21,6 +21,7 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_SEGV_MTEAERR  8  /* Asynchronous ARM MTE error */
 #define TARGET_SEGV_MTESERR  9  /* Synchronous ARM MTE exception */
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 41a68a57bc..1b2f2b4239 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -71,6 +71,21 @@
         put_user_u16(__x, (gaddr));                     \
     })
 
+static bool check_mte_async_fault(CPUARMState *env, target_siginfo_t *info)
+{
+    if (likely(env->cp15.tfsr_el[0] == 0)) {
+        return false;
+    }
+
+    env->cp15.tfsr_el[0] = 0;
+    info->si_signo = TARGET_SIGSEGV;
+    info->si_errno = 0;
+    info->_sifields._sigfault._addr = 0;
+    info->si_code = TARGET_SEGV_MTEAERR;
+    queue_signal(env, info->si_signo, QEMU_SI_FAULT, info);
+    return true;
+}
+
 /* AArch64 main loop */
 void cpu_loop(CPUARMState *env)
 {
@@ -87,15 +102,13 @@ void cpu_loop(CPUARMState *env)
 
         switch (trapnr) {
         case EXCP_SWI:
-            ret = do_syscall(env,
-                             env->xregs[8],
-                             env->xregs[0],
-                             env->xregs[1],
-                             env->xregs[2],
-                             env->xregs[3],
-                             env->xregs[4],
-                             env->xregs[5],
-                             0, 0);
+            if (check_mte_async_fault(env, &info)) {
+                ret = -TARGET_ERESTARTSYS;
+            } else {
+                ret = do_syscall(env, env->xregs[8], env->xregs[0],
+                                 env->xregs[1], env->xregs[2], env->xregs[3],
+                                 env->xregs[4], env->xregs[5], 0, 0);
+            }
             if (ret == -TARGET_ERESTARTSYS) {
                 env->pc -= 4;
             } else if (ret != -TARGET_QEMU_ESIGRETURN) {
@@ -103,7 +116,8 @@ void cpu_loop(CPUARMState *env)
             }
             break;
         case EXCP_INTERRUPT:
-            /* just indicate that signals should be handled asap */
+            /* Just indicate that signals should be handled asap. */
+            check_mte_async_fault(env, &info);
             break;
         case EXCP_UDEF:
             info.si_signo = TARGET_SIGILL;
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index b15fa665df..835b6d1ded 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -542,6 +542,16 @@ static void mte_check_fail(CPUARMState *env, int mmu_idx,
             select = 0;
         }
         env->cp15.tfsr_el[el] |= 1 << select;
+#ifdef CONFIG_USER_ONLY
+        /*
+         * Stand in for a timer irq, setting _TIF_MTE_ASYNC_FAULT,
+         * which then sends a SIGSEGV when the thread is next scheduled.
+         * This cpu will return to the main loop at the end of the TB,
+         * which is rather sooner than "normal".  But the alternative
+         * is waiting until the next syscall.
+         */
+        qemu_cpu_kick(env_cpu(env));
+#endif
         break;
 
     default:
-- 
2.25.1

[PATCH v2 15/17] target/arm: Add allocation tag storage for user mode

[Richard Henderson]

Use the now-saved PAGE_ANON and PAGE_TARGET_2 bits,
and the per-page saved data.

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

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 835b6d1ded..0911cebd36 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -76,8 +76,33 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
                                    int tag_size, uintptr_t ra)
 {
 #ifdef CONFIG_USER_ONLY
-    /* Tag storage not implemented.  */
-    return NULL;
+    uint64_t clean_ptr = useronly_clean_ptr(ptr);
+    int flags = page_get_flags(clean_ptr);
+    uint8_t *tags;
+    uintptr_t index;
+
+    if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE : PAGE_READ))) {
+        /* SIGSEGV */
+        arm_cpu_tlb_fill(env_cpu(env), ptr, ptr_size, ptr_access,
+                         ptr_mmu_idx, false, ra);
+        g_assert_not_reached();
+    }
+
+    /* Require both MAP_ANON and PROT_MTE for the page. */
+    if (!(flags & PAGE_ANON) || !(flags & PAGE_TARGET_2)) {
+        return NULL;
+    }
+
+    tags = page_get_target_data(clean_ptr);
+    if (tags == NULL) {
+        size_t alloc_size = TARGET_PAGE_SIZE >> (LOG2_TAG_GRANULE + 1);
+        tags = page_alloc_target_data(clean_ptr, alloc_size);
+        assert(tags != NULL);
+    }
+
+    index = extract32(ptr, LOG2_TAG_GRANULE + 1,
+                      TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1);
+    return tags + index;
 #else
     uintptr_t index;
     CPUIOTLBEntry *iotlbentry;
-- 
2.25.1

[PATCH v2 16/17] target/arm: Enable MTE for user-only

[Richard Henderson]

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

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 37b7cc2c9c..6395918524 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -39,6 +39,7 @@
 #include "kvm_arm.h"
 #include "disas/capstone.h"
 #include "fpu/softfloat.h"
+#include "qemu/guest-random.h"
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -191,6 +192,22 @@ static void arm_cpu_reset(DeviceState *dev)
         /* Enable all PAC keys.  */
         env->cp15.sctlr_el[1] |= (SCTLR_EnIA | SCTLR_EnIB |
                                   SCTLR_EnDA | SCTLR_EnDB);
+        /* Enable MTE */
+        if (cpu_isar_feature(aa64_mte, cpu)) {
+            /* Enable tag access, but leave TCF0 as No Effect (0). */
+            env->cp15.sctlr_el[1] |= SCTLR_ATA0;
+            /*
+             * Exclude all tags, so that tag 0 is always used.
+             * This corresponds to Linux current->thread.gcr_incl = 0.
+             */
+            env->cp15.gcr_el1 = 0xffff;
+            /* If RGSR == 0, NextRandomTagBit will always produce 0.  */
+            do {
+                qemu_guest_getrandom_nofail(&env->cp15.rgsr_el1,
+                                            sizeof(env->cp15.rgsr_el1));
+                env->cp15.rgsr_el1 &= 0xffff0fu;
+            } while (env->cp15.rgsr_el1 == 0);
+        }
         /* and to the FP/Neon instructions */
         env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3);
         /* and to the SVE instructions */
-- 
2.25.1

[PATCH v2 17/17] tests/tcg/aarch64: Add mte smoke tests

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/aarch64/mte.h           | 54 +++++++++++++++++++++++++++++++
 tests/tcg/aarch64/mte-1.c         | 25 ++++++++++++++
 tests/tcg/aarch64/mte-2.c         | 42 ++++++++++++++++++++++++
 tests/tcg/aarch64/mte-3.c         | 47 +++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  4 +++
 5 files changed, 172 insertions(+)
 create mode 100644 tests/tcg/aarch64/mte.h
 create mode 100644 tests/tcg/aarch64/mte-1.c
 create mode 100644 tests/tcg/aarch64/mte-2.c
 create mode 100644 tests/tcg/aarch64/mte-3.c

diff --git a/tests/tcg/aarch64/mte.h b/tests/tcg/aarch64/mte.h
new file mode 100644
index 0000000000..7bcc416e6a
--- /dev/null
+++ b/tests/tcg/aarch64/mte.h
@@ -0,0 +1,54 @@
+#include <assert.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+
+#ifndef PR_SET_TAGGED_ADDR_CTRL
+# define PR_SET_TAGGED_ADDR_CTRL  55
+#endif
+#ifndef PR_TAGGED_ADDR_ENABLE
+# define PR_TAGGED_ADDR_ENABLE    (1UL << 0)
+#endif
+#ifndef PR_MTE_TCF_SHIFT
+# define PR_MTE_TCF_SHIFT         1
+# define PR_MTE_TCF_NONE          (0UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_SYNC          (1UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_ASYNC         (2UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TAG_SHIFT         3
+#endif
+
+#ifndef PROT_MTE
+# define PROT_MTE 0x20
+#endif
+
+#ifndef SEGV_MTEAERR
+# define SEGV_MTEAERR    8
+# define SEGV_MTESERR    9
+#endif
+
+static void enable_mte(int tcf)
+{
+    int r = prctl(PR_SET_TAGGED_ADDR_CTRL,
+                  PR_TAGGED_ADDR_ENABLE | tcf | (0xfffe << PR_MTE_TAG_SHIFT),
+                  0, 0, 0);
+    if (r < 0) {
+        perror("PR_SET_TAGGED_ADDR_CTRL");
+        exit(2);
+    }
+}
+
+static void *alloc_mte_page(void)
+{
+    void *p = mmap(NULL, getpagesize(),
+                   PROT_READ | PROT_WRITE | PROT_MTE,
+                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    if (p == MAP_FAILED) {
+        perror("mmap PROT_MTE");
+        exit(2);
+    }
+    return p;
+}
diff --git a/tests/tcg/aarch64/mte-1.c b/tests/tcg/aarch64/mte-1.c
new file mode 100644
index 0000000000..e62c842e85
--- /dev/null
+++ b/tests/tcg/aarch64/mte-1.c
@@ -0,0 +1,25 @@
+/*
+ * Memory tagging, basic pass cases.
+ */
+
+#include "mte.h"
+
+int main(int ac, char **av)
+{
+    int *p0, *p1, *p2;
+    long c;
+
+    enable_mte(PR_MTE_TCF_NONE);
+    p0 = alloc_mte_page();
+
+    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, [%0]" : : "r"(p1));
+    asm("ldg %0, [%1]" : "=r"(p2) : "r"(p0), "0"(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..68b5708462
--- /dev/null
+++ b/tests/tcg/aarch64/mte-2.c
@@ -0,0 +1,42 @@
+/*
+ * Memory tagging, basic fail cases.
+ */
+
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+    assert(info->si_code == SEGV_MTESERR);
+    exit(0);
+}
+
+int main(int ac, char **av)
+{
+    struct sigaction sa;
+    int *p0, *p1, *p2;
+    long excl = 1;
+
+    enable_mte(PR_MTE_TCF_SYNC);
+    p0 = alloc_mte_page();
+
+    /* 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, [%0]" : : "r"(p1));
+
+    *p1 = 0;
+
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_sigaction = pass;
+    sa.sa_flags = SA_SIGINFO;
+    sigaction(SIGSEGV, &sa, NULL);
+
+    *p2 = 0;
+
+    abort();
+}
diff --git a/tests/tcg/aarch64/mte-3.c b/tests/tcg/aarch64/mte-3.c
new file mode 100644
index 0000000000..6f034cdb01
--- /dev/null
+++ b/tests/tcg/aarch64/mte-3.c
@@ -0,0 +1,47 @@
+/*
+ * Memory tagging, basic fail cases.
+ */
+
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+    assert(info->si_code == SEGV_MTEAERR);
+    exit(0);
+}
+
+int main(int ac, char **av)
+{
+    struct sigaction sa;
+    int *p0, *p1, *p2;
+    long excl = 1;
+
+    enable_mte(PR_MTE_TCF_ASYNC);
+    p0 = alloc_mte_page();
+
+    /* 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, [%0]" : : "r"(p1));
+
+    *p1 = 0;
+
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_sigaction = pass;
+    sa.sa_flags = SA_SIGINFO;
+    sigaction(SIGSEGV, &sa, NULL);
+
+    /*
+     * Signal for async error will happen eventually.
+     * For a real kernel this should be after the next IRQ (e.g. timer).
+     * For qemu linux-user, we kick the cpu and exit at the next TB.
+     * In either case, loop until this happens (or killed by timeout).
+     */
+    *p2 = 0;
+    while (1);
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index cf84787eb6..2269c264df 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -31,6 +31,10 @@ bti-%: CFLAGS += -mbranch-protection=standard
 bti-%: LDFLAGS += -nostdlib
 endif
 
+# MTE Tests
+AARCH64_TESTS += mte-1 mte-2 mte-3
+mte-%: CFLAGS += -march=armv8.5-a+memtag
+
 # Semihosting smoke test for linux-user
 AARCH64_TESTS += semihosting
 run-semihosting: semihosting
-- 
2.25.1

Re: [PATCH v2 00/17] target-arm: Implement ARMv8.5-MemTag, user mode

[Peter Maydell]

On Fri, 5 Jun 2020 at 05:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Version 1 was back in October:
> https://patchew.org/QEMU/20191015163254.12041-1-richard.henderson@linaro.org/
>
> Although that post claims there was an actual v1 in March 2019,
> I can't locate it, so... whatever.
>
> This version -- call it 2 -- is the first attempt to implement the
> actual in-progress kernel abi, rather than making up a private abi:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=devel/mte-v4
>
> Based-on: 20200603011317.473934-1-richard.henderson@linaro.org
> ("target/arm: Implement ARMv8.5-MemTag, system mode")
> Based-on: 20200520172800.8499-1-richard.henderson@linaro.org
> ("linux-user: User support for AArch64 BTI")
>
> The full tree is available at:
> https://github.com/rth7680/qemu/tree/tgt-arm-mte-user

I've reviewed most of this series; the later parts look ok
but they're implementations of an ABI that hasn't yet gone
into the kernel so I haven't bothered to look closely at the
details.

thanks
-- PMM