On 9/13/23 18:55, Richard Henderson wrote:
On 9/13/23 07:47, Helge Deller wrote:
+        haddr = (uint32_t *)((uintptr_t)vaddr);
+        old = *haddr;

This is horribly incorrect, both for user-only and system mode.

Richard, thank you for the review!
But would you mind explaining why this is incorrect?
I thought the "vaddr = probe_access()" calculates the host address, so
shouldn't it be the right address?

+        /* if already zero, do not write 0 again to reduce memory presssure */
+        if (old == 0) {
+            return 0;
+        }
+        old = qatomic_xchg(haddr, (uint32_t) 0);

You're also dropping the required host memory barrier.

?

Frankly, the current tcg_gen_atomic_xchg_reg is as optimized as you'll be able to do.  I 
really doubt the "avoid write 0" is measurable at all.

Helge

Reply via email to