qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: qemu-ppc@nongnu.org
Cc: "Greg Kurz" <groug@kaod.org>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	qemu-devel@nongnu.org, "Cédric Le Goater" <clg@fr.ibm.com>,
	"David Gibson" <david@gibson.dropbear.id.au>
Subject: [PATCH 1/2] target/ppc: Fix slbia TLB invalidation gap
Date: Wed, 18 Mar 2020 14:41:34 +1000	[thread overview]
Message-ID: <20200318044135.851716-1-npiggin@gmail.com> (raw)

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
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 <npiggin@gmail.com>
---
 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,
-- 
2.23.0



             reply	other threads:[~2020-03-18  4:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-18  4:41 Nicholas Piggin [this message]
2020-03-18  4:41 ` [PATCH 2/2] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation Nicholas Piggin
2020-03-18 17:08   ` [EXTERNAL] " Cédric Le Goater
2020-03-18 20:46     ` Benjamin Herrenschmidt
2020-03-19  2:42       ` Nicholas Piggin
2020-03-19  5:22       ` David Gibson
2020-03-18 16:45 ` [EXTERNAL] [PATCH 1/2] target/ppc: Fix slbia TLB invalidation gap Cédric Le Goater
2020-03-19  2:24   ` Nicholas Piggin
2020-03-18 16:52 ` Greg Kurz
2020-03-19  5:20   ` David Gibson
2020-03-19  5:19 ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200318044135.851716-1-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=clg@fr.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).