[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 2/6] cputlb: do not evict invalid entries to the v
From: |
Emilio G. Cota |
Subject: |
Re: [Qemu-devel] [RFC 2/6] cputlb: do not evict invalid entries to the vtlb |
Date: |
Mon, 8 Oct 2018 10:42:42 -0400 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
On Sun, Oct 07, 2018 at 19:09:01 -0700, Richard Henderson wrote:
> On 10/6/18 2:45 PM, Emilio G. Cota wrote:
> > Currently we evict an entry to the victim TLB when it doesn't match
> > the current address. But it could be that there's no match because
> > the current entry is invalid. Do not evict the entry to the vtlb
> > in that case.
> >
> > This change will help us keep track of the TLB's use rate.
> >
> > Signed-off-by: Emilio G. Cota <address@hidden>
> > ---
> > include/exec/cpu-all.h | 14 ++++++++++++++
> > accel/tcg/cputlb.c | 2 +-
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> > index 117d2fbbca..d938dedafc 100644
> > --- a/include/exec/cpu-all.h
> > +++ b/include/exec/cpu-all.h
> > @@ -362,6 +362,20 @@ static inline bool tlb_hit(target_ulong tlb_addr,
> > target_ulong addr)
> > return tlb_hit_page(tlb_addr, addr & TARGET_PAGE_MASK);
> > }
> >
> > +/**
> > + * tlb_is_valid - return true if at least one of the addresses is valid
> > + * @te: pointer to CPUTLBEntry
> > + *
> > + * This is useful when we don't have a particular address to compare
> > against,
> > + * and we just want to know whether any entry holds valid data.
> > + */
> > +static inline bool tlb_is_valid(const CPUTLBEntry *te)
> > +{
> > + return !(te->addr_read & TLB_INVALID_MASK) ||
> > + !(te->addr_write & TLB_INVALID_MASK) ||
> > + !(te->addr_code & TLB_INVALID_MASK);
> > +}
>
> No, I think you misunderstand.
>
> First, TLB_INVALID_MASK is only ever set for addr_write, in response to
> PAGE_WRITE_INV. Second, an entry that is invalid for write is still valid for
> read+exec. So there is benefit to swapping it out to the victim cache.
>
> This is used by the s390x target to make the "lowpage" read-only, which is a
> special architected 512 byte range within pages 0 and 1. This is done by
> forcing writes, but not reads, back through tlb_fill.
Aah I see. The point is to avoid pushing to the victim cache
an entry that is all invalid, not just partially invalid.
Thanks for the clarification!
Emilio
[Qemu-devel] [RFC 1/6] (XXX) cputlb: separate MMU allocation + run-time sizing, Emilio G. Cota, 2018/10/06