Pranith Kumar <bobby.pr...@gmail.com> writes:
> Hi Alex,
>
> This patch is causing some build errors on a 32-bit box:
>
> In file included from /home/pranith/qemu/include/exec/exec-all.h:44:0,
> from /home/pranith/qemu/cputlb.c:23:
> /home/pranith/qemu/cputlb.c: In function
> ‘tlb_flush_page_by_mmuidx_async_work’:
> /home/pranith/qemu/cputlb.c:54:36: error: format ‘%x’ expects argument of
> type ‘unsigned int’, but argument 5 has type ‘long unsigned int’
> [-Werror=format=]
> qemu_log_mask(CPU_LOG_MMU, "%s: " fmt, __func__, \
> ^
> /home/pranith/qemu/include/qemu/log.h:94:22: note: in definition of macro
> ‘qemu_log_mask’
> qemu_log(FMT, ## __VA_ARGS__); \
> ^~~
> /home/pranith/qemu/cputlb.c:286:5: note: in expansion of macro ‘tlb_debug’
> tlb_debug("page:%d addr:"TARGET_FMT_lx" mmu_idx%" PRIxPTR "\n",
> ^~~~~~~~~
> /home/pranith/qemu/cputlb.c:57:25: error: format ‘%x’ expects argument of
> type ‘unsigned int’, but argument 6 has type ‘long unsigned int’
> [-Werror=format=]
> fprintf(stderr, "%s: " fmt, __func__, ## __VA_ARGS__); \
> ^
> /home/pranith/qemu/cputlb.c:286:5: note: in expansion of macro ‘tlb_debug’
> tlb_debug("page:%d addr:"TARGET_FMT_lx" mmu_idx%" PRIxPTR "\n",
> ^~~~~~~~~
> cc1: all warnings being treated as errors
>
> Thanks,
>
<snip>
>> +/*
>> + * Dirty write flag handling
>> + *
>> + * When the TCG code writes to a location it looks up the address in
>> + * the TLB and uses that data to compute the final address. If any of
>> + * the lower bits of the address are set then the slow path is forced.
>> + * There are a number of reasons to do this but for normal RAM the
>> + * most usual is detecting writes to code regions which may invalidate
>> + * generated code.
>> + *
>> + * Because we want other vCPUs to respond to changes straight away we
>> + * update the te->addr_write field atomically. If the TLB entry has
>> + * been changed by the vCPU in the mean time we skip the update.
>> + */
>> +
>> +static void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
>> uintptr_t length)
>> {
>> - uintptr_t addr;
>> + /* paired with atomic_mb_set in tlb_set_page_with_attrs */
>> + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write);
>> + uintptr_t addr = orig_addr;
>>
>> - if (tlb_is_dirty_ram(tlb_entry)) {
>> - addr = (tlb_entry->addr_write & TARGET_PAGE_MASK) +
>> tlb_entry->addend;
>> + if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_NOTDIRTY)) == 0) {
>> + addr &= TARGET_PAGE_MASK;
>> + addr += atomic_read(&tlb_entry->addend);
>> if ((addr - start) < length) {
>> - tlb_entry->addr_write |= TLB_NOTDIRTY;
>> + uintptr_t notdirty_addr = orig_addr | TLB_NOTDIRTY;
>> + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr,
>> notdirty_addr);
>> }
>> }
>> }
Even worse than that we trip up the atomic.h QEMU_BUILD_BUG_ON with the
atomic_cmpxchg. Now I believe we can use atomic_cmpxchg__nocheck without
too much issue on x86 but we'll need to #ifdef it on detection of wide
atomics.
--
Alex Bennée
