Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST

[Richard Henderson]

On 07/14/2017 11:01 AM, Aurelien Jarno wrote:
+    if (parallel_cpus) {
+        int mask = 0;
+#if !defined(CONFIG_ATOMIC64)
+        mask = -8;
+#elif !defined(CONFIG_ATOMIC128)
+        mask = -16;
+#endif
+        if (((4 << fc) | (1 << sc)) & mask) {
+            cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+        }
+    }

This doesn't look correct. For a 16-byte store, ie sc = 4, and with
ATOMIC128 support, ie mask = -16, the condition is true.

That's WITHOUT atomic128 support that mask = -16.
If we have atomic128, then mask = 0.

+            if (parallel_cpus) {
+#ifdef CONFIG_USER_ONLY
+                uint32_t *haddr = g2h(a1);
+                ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
+#else
+                TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
+                ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
+#endif

Not a problem with the patch itself, but how complicated would it be to
make helper_atomic_cmpxchgl_be_mmu (just like the o version) available
also in user mode? That would make less #ifdef in the backends.

Dunno.  Probably not too bad.  I have wondered about doing that, as these sorts 
of functions are quite ugly to write.


r~