• E
    tcg: consistently access cpu->tb_jmp_cache atomically · f3ced3c5
    Emilio G. Cota 提交于
    Some code paths can lead to atomic accesses racing with memset()
    on cpu->tb_jmp_cache, which can result in torn reads/writes
    and is undefined behaviour in C11.
    
    These torn accesses are unlikely to show up as bugs, but from code
    inspection they seem possible. For example, tb_phys_invalidate does:
        /* remove the TB from the hash list */
        h = tb_jmp_cache_hash_func(tb->pc);
        CPU_FOREACH(cpu) {
            if (atomic_read(&cpu->tb_jmp_cache[h]) == tb) {
                atomic_set(&cpu->tb_jmp_cache[h], NULL);
            }
        }
    Here atomic_set might race with a concurrent memset (such as the
    ones scheduled via "unsafe" async work, e.g. tlb_flush_page) and
    therefore we might end up with a torn pointer (or who knows what,
    because we are under undefined behaviour).
    
    This patch converts parallel accesses to cpu->tb_jmp_cache to use
    atomic primitives, thereby bringing these accesses back to defined
    behaviour. The price to pay is to potentially execute more instructions
    when clearing cpu->tb_jmp_cache, but given how infrequently they happen
    and the small size of the cache, the performance impact I have measured
    is within noise range when booting debian-arm.
    
    Note that under "safe async" work (e.g. do_tb_flush) we could use memset
    because no other vcpus are running. However I'm keeping these accesses
    atomic as well to keep things simple and to avoid confusing analysis
    tools such as ThreadSanitizer.
    Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
    Reviewed-by: NRichard Henderson <rth@twiddle.net>
    Signed-off-by: NEmilio G. Cota <cota@braap.org>
    Message-Id: <1497486973-25845-1-git-send-email-cota@braap.org>
    Signed-off-by: NRichard Henderson <rth@twiddle.net>
    f3ced3c5
cpu.h 31.8 KB