qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] target/riscv: Avoid leaking "no translation" TLB entries


From: Palmer Dabbelt
Subject: [PATCH] target/riscv: Avoid leaking "no translation" TLB entries
Date: Wed, 30 Mar 2022 09:59:13 -0700

The ISA doesn't allow bare mappings to be cached, as the caches are
translations and bare mppings are not translated.  We cache these
translations in QEMU in order to utilize the TLB code, but that leaks
out to the guest.

Suggested-by: phantom@zju.edu.cn # no name in the From field
Fixes: 1e0d985fa9 ("target/riscv: Only flush TLB if SATP.ASID changes")
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

---

Another way to fix this would be to utilize a MMU index that cooresponds
to no ASID to hold these direct mappings, but given that we're not
currently taking advantage of ASIDs for translation performance that
would be a larger chunk of work.  This causes a Linux boot regression,
so the band-aid seems appropriate.

I think the original version of this was also more broadly broken, in
that changing to ASID 0 would allow old mappings, but I might be missing
something there.  I seem to remember ASID 0 as having been special at
some point, but it's not in the ISA as it stands so maybe I'm just
crazy.

This, when applied on top of Alistair's riscv-to-apply.next, boots my
for-next (which is very close to Linus' master).
---
 target/riscv/csr.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0606cd0ea8..cabef5a20b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1844,7 +1844,7 @@ static RISCVException read_satp(CPURISCVState *env, int 
csrno,
 static RISCVException write_satp(CPURISCVState *env, int csrno,
                                  target_ulong val)
 {
-    target_ulong vm, mask, asid;
+    target_ulong vm, mask;
 
     if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
         return RISCV_EXCP_NONE;
@@ -1853,20 +1853,22 @@ static RISCVException write_satp(CPURISCVState *env, 
int csrno,
     if (riscv_cpu_mxl(env) == MXL_RV32) {
         vm = validate_vm(env, get_field(val, SATP32_MODE));
         mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
-        asid = (val ^ env->satp) & SATP32_ASID;
     } else {
         vm = validate_vm(env, get_field(val, SATP64_MODE));
         mask = (val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
-        asid = (val ^ env->satp) & SATP64_ASID;
     }
 
     if (vm && mask) {
         if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
             return RISCV_EXCP_ILLEGAL_INST;
         } else {
-            if (asid) {
-                tlb_flush(env_cpu(env));
-            }
+           /*
+            * The ISA defines SATP.MODE=Bare as "no translation", but we still
+            * pass these through QEMU's TLB emulation as it improves
+            * performance.  Flushing the TLB on SATP writes with paging
+            * enabled avoids leaking those invalid cached mappings.
+            */
+            tlb_flush(env_cpu(env));
             env->satp = val;
         }
     }
-- 
2.34.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]