On Mon, Oct 08, 2018 at 14:57:18 +0100, Alex Bennée wrote: > Emilio G. Cota <c...@braap.org> writes: > > The readers that do not hold tlb_lock must use atomic reads when > > reading .addr_write, since this field can be updated by other threads; > > the conversion to atomic reads is done in the next patch. > > We don't enforce this for the TCG code - but rely on the backend ISA's > to avoid torn reads from updates from cputlb that could invalidate an > entry.
We do enforce it though; the TLB reads we emit in TCG backend code are appropriately sized to guarantee atomic reads. > > -/* For atomic correctness when running MTTCG we need to use the right > > - * primitives when copying entries */ > > -static inline void copy_tlb_helper(CPUTLBEntry *d, CPUTLBEntry *s, > > - bool atomic_set) > > +/* Called with tlb_lock held */ > > +static inline void copy_tlb_helper_locked(CPUTLBEntry *d, const > > CPUTLBEntry *s) > > { > > -#if TCG_OVERSIZED_GUEST > > *d = *s; > > In general I'm happy with the patch set but what ensures that this > always DRT with respect to the TCG code reads that race with it? copy_tlb_helper is only called by the "owner" CPU, so it cannot race with TCG code (i.e. the owner thread cannot race with itself). I wanted to add an assert_cpu_is_self(cpu) here, but that needs a CPUState pointer. Maybe I should just get rid of the function? All the callers have the assert, so that might make the code clearer. Thanks, Emilio