| Message ID | 20170320153441.2181-2-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | MTTCG regression fixes for 2.9-rc1 | expand |
On 20/03/2017 16:34, Alex Bennée wrote: > static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, target_ulong vaddr) > { > +#if TCG_OVERSIZED_GUEST > if (tlb_entry->addr_write == (vaddr | TLB_NOTDIRTY)) { > tlb_entry->addr_write = vaddr; > } > +#else > + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write); atomic_read is enough, since we don't care at all about cases where the address doesn't match. Otherwise Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo > + if (orig_addr == (vaddr | TLB_NOTDIRTY)) { > + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, vaddr); > + } > +#endif > }
Paolo Bonzini <pbonzini@redhat.com> writes: > On 20/03/2017 16:34, Alex Bennée wrote: >> static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, target_ulong vaddr) >> { >> +#if TCG_OVERSIZED_GUEST >> if (tlb_entry->addr_write == (vaddr | TLB_NOTDIRTY)) { >> tlb_entry->addr_write = vaddr; >> } >> +#else >> + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write); > > atomic_read is enough, since we don't care at all about cases where the > address doesn't match. Otherwise Good catch. Will fix for my pullreq > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Paolo > >> + if (orig_addr == (vaddr | TLB_NOTDIRTY)) { >> + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, vaddr); >> + } >> +#endif >> } -- Alex Bennée
On 03/21/2017 01:34 AM, Alex Bennée wrote: > This was an oversight when the rest of cputlb was being updated. As > before it falls back to the non-atomic version when the host can't > support wider-than-bus atomics. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > cputlb.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/cputlb.c b/cputlb.c > index f5d056cc08..0d52e45dfd 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -540,9 +540,17 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length) > > static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, target_ulong vaddr) > { > +#if TCG_OVERSIZED_GUEST > if (tlb_entry->addr_write == (vaddr | TLB_NOTDIRTY)) { > tlb_entry->addr_write = vaddr; > } > +#else > + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write); > + > + if (orig_addr == (vaddr | TLB_NOTDIRTY)) { > + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, vaddr); > + } What's the state machine here? How can the per-cpu tlb change in a way other than dirty->clean / clean->dirty? AFAIK, we shouldn't be swapping out the tlb entry to somthing completely different. So how does cmpxchg improve over atomic_write? r~
diff --git a/cputlb.c b/cputlb.c index f5d056cc08..0d52e45dfd 100644 --- a/cputlb.c +++ b/cputlb.c @@ -540,9 +540,17 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length) static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, target_ulong vaddr) { +#if TCG_OVERSIZED_GUEST if (tlb_entry->addr_write == (vaddr | TLB_NOTDIRTY)) { tlb_entry->addr_write = vaddr; } +#else + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write); + + if (orig_addr == (vaddr | TLB_NOTDIRTY)) { + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, vaddr); + } +#endif } /* update the TLB corresponding to virtual page vaddr
This was an oversight when the rest of cputlb was being updated. As before it falls back to the non-atomic version when the host can't support wider-than-bus atomics. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- cputlb.c | 8 ++++++++ 1 file changed, 8 insertions(+) -- 2.11.0