[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/4] cputlb: serialize tlb updates with env->
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/4] cputlb: serialize tlb updates with env->tlb_lock |
Date: |
Mon, 08 Oct 2018 15:30:28 +0100 |
User-agent: |
mu4e 1.1.0; emacs 26.1.50 |
Emilio G. Cota <address@hidden> writes:
> On Mon, Oct 08, 2018 at 14:57:18 +0100, Alex Bennée wrote:
>> Emilio G. Cota <address@hidden> 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.
I'm happy keeping the function and just expanding the comment:
/* Called with tlb_lock held and only ever from the vCPU context */
Reviewed-by: Alex Bennée <address@hidden>
>
> Thanks,
>
> Emilio
--
Alex Bennée
[Qemu-devel] [PATCH v3 1/4] exec: introduce tlb_init, Emilio G. Cota, 2018/10/05