On Mon, Oct 08, 2018 at 14:57:18 +0100, Alex Bennée wrote:
> Emilio G. Cota <[email protected]> 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
