On 12 September 2017 at 05:23, Richard Henderson <r...@twiddle.net> wrote: > On 09/11/2017 02:37 PM, Peter Maydell wrote: >> On 11 September 2017 at 21:49, Philippe Mathieu-Daudé <f4...@amsat.org> >> wrote: >>> this fixes building for ppc64 on ppc32 (changed in 5964fca8a12c): >>> >>> qemu/tcg/ppc/tcg-target.inc.c: In function 'tb_target_set_jmp_target': >>> qemu/include/qemu/compiler.h:86:30: error: static assertion failed: "not >>> expecting: sizeof(*(uint64_t *)jmp_addr) > ATOMIC_REG_SIZE" >>> QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \ >>> ^ >>> qemu/tcg/ppc/tcg-target.inc.c:1377:9: note: in expansion of macro >>> 'atomic_set' >>> atomic_set((uint64_t *)jmp_addr, pair); >>> ^ >>> >>> Suggested-by: Richard Henderson <r...@twiddle.net> >>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >>> --- >>> This fixes Shippable builds, see: >>> https://app.shippable.com/github/qemu/qemu/runs/434/10/console >>> >>> tcg/ppc/tcg-target.inc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c >>> index 21d764c102..0417901289 100644 >>> --- a/tcg/ppc/tcg-target.inc.c >>> +++ b/tcg/ppc/tcg-target.inc.c >>> @@ -1374,7 +1374,7 @@ void tb_target_set_jmp_target(uintptr_t tc_ptr, >>> uintptr_t jmp_addr, >>> pair = (uint64_t)i2 << 32 | i1; >>> #endif >>> >>> - atomic_set((uint64_t *)jmp_addr, pair); >>> + atomic_set__nocheck((uint64_t *)jmp_addr, pair); >>> flush_icache_range(jmp_addr, jmp_addr + 8); >>> } else { >>> intptr_t diff = addr - jmp_addr; >> >> Can you explain why this is the right thing? On the >> face of it it looks correct to insist that we don't >> try to do an atomic set of something that's bigger >> than the host can actually handle... > > It is the correct thing because ppc32 is handled earlier in the function; only > ppc64 can reach here, therefore a 64-bit atomic_set is always available. > > However, I wrote the function intending to minimize the ifdefs so that we can > be sure that it all compiles -- especially the ppc32 bits which I cannot test > on gcc cfarm machines. I didn't think about the fact that ppc32 could not > compile the _Static_assert within the 64-bit atomic_set here in the ppc64 > section.
Ah, I see. Can we have a comment about why the __nocheck is ok here, then, please? thanks -- PMM
