Re: [PATCH 1/3] accel/tcg: Unify big- and little- endian atomic ops

[Richard Henderson]

On 8/21/25 21:33, Danila Zhebryakov wrote:
  ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, vaddr addr,
                                ABI_TYPE cmpv, ABI_TYPE newv,
                                MemOpIdx oi, uintptr_t retaddr)
  {
+    bool need_bswap = get_memop(oi) & MO_BSWAP;
      DATA_TYPE *haddr = atomic_mmu_lookup(env_cpu(env), addr, oi,
-                                         DATA_SIZE, retaddr);
-    DATA_TYPE ret;
-
+                                         DATA_SIZE, retaddr, &need_bswap);
+    DATA_TYPE ret, ret_e;
+    if (need_bswap) {

I'd rather you do all of the bswap detection in the helper, not put part of it here and 
then modify the partial value.


diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 87e14bde4f..841b54e41d 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1808,7 +1808,7 @@ static bool mmu_lookup(CPUState *cpu, vaddr addr, 
MemOpIdx oi,
   * or io operations to proceed.  Return the host address.
   */
  static void *atomic_mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
-                               int size, uintptr_t retaddr)
+                               int size, uintptr_t retaddr, bool *need_bswap)
  {
      uintptr_t mmu_idx = get_mmuidx(oi);
      MemOp mop = get_memop(oi);
@@ -1894,6 +1894,14 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr 
addr, MemOpIdx oi,
          notdirty_write(cpu, addr, size, full, retaddr);
      }
  
+    if (unlikely(tlb_addr & TLB_BSWAP)) {
+        assert(!( (  full->slow_flags[MMU_DATA_STORE]
+            ^ full->slow_flags[MMU_DATA_LOAD ])
+            & TLB_BSWAP));

You can skip the assert. It's impossible to set a page with differing values.

+
+        *need_bswap = !need_bswap;
+    }

This becomes

      mop ^= MO_BSWAP;
    }
    *need_bswap = mop & MO_BSWAP;


r~