On Wed, Mar 18, 2020 at 05:52:32PM +0100, Greg Kurz wrote: 65;5803;1c> On Wed, 18 Mar 2020 14:41:34 +1000 > Nicholas Piggin <npig...@gmail.com> wrote: > > > slbia must invalidate TLBs even if it does not remove a valid SLB > > entry, because slbmte can overwrite valid entries without removing > > their TLBs. > > > > As the architecture says, slbia invalidates all lookaside information, > > not conditionally based on if it removed valid entries. > > > > It does not seem possible for POWER8 or earlier Linux kernels to hit > > this bug because it never changes its kernel SLB translations, and it > > should always have valid entries if any accesses are made to usespace > > s/usespace/userspace
Corrected in my tree, thanks.
>
> > regions. However other operating systems which may modify SLB entry 0
> > or do more fancy things with segments might be affected.
> >
> > When POWER9 slbia support is added in the next patch, this becomes a
> > real problem because some new slbia variants don't invalidate all
> > non-zero entries.
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
>
> LGTM
>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
>
> > target/ppc/mmu-hash64.c | 21 +++++++++++++++------
> > 1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index 34f6009b1e..373d44de74 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -100,20 +100,29 @@ void helper_slbia(CPUPPCState *env)
> > PowerPCCPU *cpu = env_archcpu(env);
> > int n;
> >
> > + /*
> > + * slbia must always flush all TLB (which is equivalent to ERAT in ppc
> > + * architecture). Matching on SLB_ESID_V is not good enough, because
> > slbmte
> > + * can overwrite a valid SLB without flushing its lookaside
> > information.
> > + *
> > + * It would be possible to keep the TLB in synch with the SLB by
> > flushing
> > + * when a valid entry is overwritten by slbmte, and therefore slbia
> > would
> > + * not have to flush unless it evicts a valid SLB entry. However it is
> > + * expected that slbmte is more common than slbia, and slbia is usually
> > + * going to evict valid SLB entries, so that tradeoff is unlikely to
> > be a
> > + * good one.
> > + */
> > +
> > /* XXX: Warning: slbia never invalidates the first segment */
> > for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
> > ppc_slb_t *slb = &env->slb[n];
> >
> > if (slb->esid & SLB_ESID_V) {
> > slb->esid &= ~SLB_ESID_V;
> > - /*
> > - * XXX: given the fact that segment size is 256 MB or 1TB,
> > - * and we still don't have a tlb_flush_mask(env, n, mask)
> > - * in QEMU, we just invalidate all TLBs
> > - */
> > - env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
> > }
> > }
> > +
> > + env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
> > }
> >
> > static void __helper_slbie(CPUPPCState *env, target_ulong addr,
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
