Hi Richard,
On 9/13/23 22:30, Richard Henderson wrote:
On 9/13/23 10:19, Helge Deller wrote:
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?
The vaddr name is confusing (since it implies virtual address, which
the return from probe_access is not) as are the casts, which are
unnecessary.
Still, I think my code isn't as wrong as you said.
But I tend to agree with you on this:
Frankly, the current tcg_gen_atomic_xchg_reg is as optimized as
you'll be able to do.
tcg_gen_atomic_xchg_reg() seems to generate on x86-64:
0000000000525160 <helper_atomic_xchgl_be>:
525160: 53 push %rbx
525161: 4c 8b 44 24 08 mov 0x8(%rsp),%r8
525166: 89 d3 mov %edx,%ebx
525168: 89 ca mov %ecx,%edx
52516a: b9 04 00 00 00 mov $0x4,%ecx
52516f: e8 1c a6 ff ff call 51f790 <atomic_mmu_lookup>
525174: 48 89 c2 mov %rax,%rdx
525177: 89 d8 mov %ebx,%eax
525179: 0f c8 bswap %eax
52517b: 87 02 xchg %eax,(%rdx)
52517d: 5b pop %rbx
52517e: 0f c8 bswap %eax
525180: c3 ret
and atomic_mmu_lookup() is basically the same as probe_access(),
so there is probably no gain in my patch.
Please ignore my patch.
Thank you!
Helge