Re: [PATCH 2/3] linux-user/arm: Implement __kernel_cmpxchg with host atomics

[Laurent Vivier]

Le 22/03/2022 à 12:46, Laurent Vivier a écrit :
Le 14/03/2022 à 05:43, Richard Henderson a écrit :
The existing implementation using start/end_exclusive
does not provide atomicity across processes.
Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
  linux-user/arm/cpu_loop.c | 85 +++++++++++++++++++++++++++------------
  1 file changed, 60 insertions(+), 25 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index a0e43b261c..0122bb34f7 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -75,7 +75,65 @@
          put_user_u16(__x, (gaddr));                     \
      })
-/* Commpage handling -- there is no commpage for AArch64 */
+/*
+ * Similar to code in accel/tcg/user-exec.c, but outside the execution loop.
+ * Must be called with mmap_lock.
+ */
+static void *atomic_mmu_lookup(CPUArchState *env, uint32_t addr, int size)
+{
+    int need_flags = PAGE_READ | PAGE_WRITE_ORG | PAGE_VALID;
+    int page_flags;
+
+    /* Enforce guest required alignment.  */
+    if (unlikely(addr & (size - 1))) {
+        force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, addr);
+        return NULL;
+    }
+
+    page_flags = page_get_flags(addr);
+    if (unlikely((page_flags & need_flags) != need_flags)) {
+        force_sig_fault(TARGET_SIGSEGV,
+                        page_flags & PAGE_VALID ?
+                        TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR, addr);
+        return NULL;
+    }
+
+    return g2h(env_cpu(env), addr);
+}
+
+/*
+ * See the Linux kernel's Documentation/arm/kernel_user_helpers.rst
+ * Input:
+ * r0 = oldval
+ * r1 = newval
+ * r2 = pointer to target value
+ *
+ * Output:
+ * r0 = 0 if *ptr was changed, non-0 if no exchange happened
+ * C set if *ptr was changed, clear if no exchange happened
+ */
+static void arm_kernel_cmpxchg32_helper(CPUARMState *env)
+{
+    uint32_t oldval, newval, val, addr, cpsr, *host_addr;
+
+    oldval = env->regs[0];
+    newval = env->regs[1];
+    addr = env->regs[2];
+
+    mmap_lock();
+    host_addr = atomic_mmu_lookup(env, addr, 8);
+    if (!host_addr) {
+        mmap_unlock();
+        return;
+    }
+
+    val = qatomic_cmpxchg__nocheck(host_addr, oldval, newval);
+    mmap_unlock();
+
+    cpsr = (val == oldval) * CPSR_C;
+    cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
+    env->regs[0] = cpsr ? 0 : -1;
+}
  /*
   * See the Linux kernel's Documentation/arm/kernel_user_helpers.txt
@@ -153,36 +211,13 @@ static int
  do_kernel_trap(CPUARMState *env)
  {
      uint32_t addr;
-    uint32_t cpsr;
-    uint32_t val;
      switch (env->regs[15]) {
      case 0xffff0fa0: /* __kernel_memory_barrier */
          smp_mb();
          break;
      case 0xffff0fc0: /* __kernel_cmpxchg */
-         /* XXX: This only works between threads, not between processes.
-            It's probably possible to implement this with native host
-            operations. However things like ldrex/strex are much harder so
-            there's not much point trying.  */
-        start_exclusive();
-        cpsr = cpsr_read(env);
-        addr = env->regs[2];
-        /* FIXME: This should SEGV if the access fails.  */
-        if (get_user_u32(val, addr))
-            val = ~env->regs[0];
-        if (val == env->regs[0]) {
-            val = env->regs[1];
-            /* FIXME: Check for segfaults.  */
-            put_user_u32(val, addr);
-            env->regs[0] = 0;
-            cpsr |= CPSR_C;
-        } else {
-            env->regs[0] = -1;
-            cpsr &= ~CPSR_C;
-        }
-        cpsr_write(env, cpsr, CPSR_C, CPSRWriteByInstr);
-        end_exclusive();
+        arm_kernel_cmpxchg32_helper(env);
          break;
      case 0xffff0fe0: /* __kernel_get_tls */
          env->regs[0] = cpu_get_tls(env);

Applied to my linux-user-for-7.0 branch.



I have removed this patch and the following one from the branch because it hangs when executed in an 
armhf/bionic chroot the following example:

cat > /tmp/hello.go <<EOF
package main

import "fmt"

func main() {
        fmt.Println("Hello Google!")
}
EOF

go run /tmp/hello.go

Thanks,
Laurent