Re: [Qemu-devel] Making cputlb.c operations safe for MTTCG

[Paolo Bonzini]

> > - tlb_set_page_with_attrs is also hard-ish to get right, but perhaps the
> > same idea of adding the callback last would work:
> >
> >     /* First set addr_write so that concurrent tlb_reset_dirty_range
> >      * finds a match.
> >      */
> >     te->addr_write = address;
> >     if (memory_region_is_ram(section->mr)) {
> >         if (cpu_physical_memory_is_clean(
> >                      memory_region_get_ram_addr(section->mr) + xlat)) {
> >             te->addr_write = address | TLB_NOTDIRTY;
> >         }
> >     }
> 
> Why not just write addr_write completely at the end?

I suspect that another thread can come around and make the page "not dirty",
while the CPU that "owns" the TLB has decided that the page is dirty and does
not set the bit.  So you have

    CPU thread                               other thread
    -----------------------------            -----------------------------------
    compute addr_write
                                             page is now clean
    write addr_write without TLB_NOTDIRTY

By the way, the same happens in victim_tlb_hit.  You have to recompute
TLB_NOTDIRTY after setting *tlb, something like:

    tmptlb = *vtlb;
    *vtlb = *tlb;

    if (tmptlb.addr_write & (TLB_INVALID_MASK|TLB_MMIO)) {
        *tlb = tmptlb;
    } else {
        /* First write TLB entry without TLB_NOTDIRTY.  */
        tmptlb.addr_write &= ~TLB_NOTDIRTY;
        *tlb = tmptlb;
        smp_wmb();
        /* Then set NOTDIRTY in tlb->addr_write if necessary.
         * IIRC the IOTLB lets you get back to the ram_addr, and
         * pass it to cpu_physical_memory_is_clean.
         */
        if (cpu_physical_memory_is_clean(tlb_get_ram_addr(&tmptlb))) {
            tlb->addr_write = tmptlb.addr_write | TLB_NOTDIRTY;
        }
    }

The silver lining is that tlb_set_dirty now does not need anymore to check
the victim TLB.

Paolo