arch-riscv: be prepared for CSR changes during PT walk.
authorNils Asmussen <nils.asmussen@barkhauseninstitut.org>
Fri, 1 May 2020 08:27:01 +0000 (10:27 +0200)
committerNils Asmussen <nils.asmussen@barkhauseninstitut.org>
Sat, 2 May 2020 05:04:50 +0000 (05:04 +0000)
If the address space is changed (by writing to SATP), it can happen that
a page table walk is in progress. Previously, this failed if the ASID
changed, because we then used the wrong ASID for the lookup.

This commit makes sure that we don't access CSRs after the beginning of
the walk by loading SATP, STATUS, and PRV at the beginning.

Change-Id: I8c184c7ae7dd44d78e881bb5ec8d430dd480849c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/28447
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
src/arch/riscv/pagetable_walker.cc
src/arch/riscv/pagetable_walker.hh
src/arch/riscv/tlb.cc
src/arch/riscv/tlb.hh

index 02d2d945b28b2b050f9d7341f13e010637c53618..6ec118d569634199135a9096743be538b6124b61 100644 (file)
@@ -184,6 +184,11 @@ Walker::WalkerState::initState(ThreadContext * _tc,
     tc = _tc;
     mode = _mode;
     timing = _isTiming;
+    // fetch these now in case they change during the walk
+    status = tc->readMiscReg(MISCREG_STATUS);
+    pmode = walker->tlb->getMemPriv(tc, mode);
+    satp = tc->readMiscReg(MISCREG_SATP);
+    assert(satp.mode == AddrXlateMode::SV39);
 }
 
 void
@@ -303,7 +308,8 @@ Walker::WalkerState::stepWalk(PacketPtr &write)
         if (pte.r || pte.x) {
             // step 5: leaf PTE
             doEndWalk = true;
-            fault = walker->tlb->checkPermissions(tc, entry.vaddr, mode, pte);
+            fault = walker->tlb->checkPermissions(status, pmode,
+                                                  entry.vaddr, mode, pte);
 
             // step 6
             if (fault == NoFault) {
@@ -413,10 +419,7 @@ Walker::WalkerState::endWalk()
 void
 Walker::WalkerState::setupWalk(Addr vaddr)
 {
-    vaddr &= ((static_cast<Addr>(1) << VADDR_BITS) - 1);
-
-    SATP satp = tc->readMiscReg(MISCREG_SATP);
-    assert(satp.mode == AddrXlateMode::SV39);
+    vaddr &= (static_cast<Addr>(1) << VADDR_BITS) - 1;
 
     Addr shift = PageShift + LEVEL_BITS * 2;
     Addr idx = (vaddr >> shift) & LEVEL_MASK;
@@ -483,12 +486,12 @@ Walker::WalkerState::recvPacket(PacketPtr pkt)
              * permissions violations, so we'll need the return value as
              * well.
              */
-            bool delayedResponse;
-            Fault fault = walker->tlb->doTranslate(req, tc, NULL, mode,
-                                                   delayedResponse);
-            assert(!delayedResponse);
+            Addr vaddr = req->getVaddr();
+            vaddr &= (static_cast<Addr>(1) << VADDR_BITS) - 1;
+            Addr paddr = walker->tlb->translateWithTLB(vaddr, satp.asid, mode);
+            req->setPaddr(paddr);
             // Let the CPU continue.
-            translation->finish(fault, req, tc, mode);
+            translation->finish(NoFault, req, tc, mode);
         } else {
             // There was a fault during the walk. Let the CPU know.
             translation->finish(timingFault, req, tc, mode);
index 2d509284e1686daa3b05a0ac6c6ac74d39b50921..60826a055ccfa7abf5a901991dc14a03868f2917 100644 (file)
@@ -100,6 +100,9 @@ namespace RiscvISA
             Fault timingFault;
             TLB::Translation * translation;
             BaseTLB::Mode mode;
+            SATP satp;
+            STATUS status;
+            PrivilegeMode pmode;
             bool functional;
             bool timing;
             bool retrying;
index 294d73ff56b73890aa7dd206bce7582a96e12630..c8b1a4c6f4cecd8fc28ebe62b2ffde9a15c933db 100644 (file)
@@ -213,7 +213,8 @@ TLB::remove(size_t idx)
 }
 
 Fault
-TLB::checkPermissions(ThreadContext *tc, Addr vaddr, Mode mode, PTESv39 pte)
+TLB::checkPermissions(STATUS status, PrivilegeMode pmode, Addr vaddr,
+                      Mode mode, PTESv39 pte)
 {
     Fault fault = NoFault;
 
@@ -232,8 +233,6 @@ TLB::checkPermissions(ThreadContext *tc, Addr vaddr, Mode mode, PTESv39 pte)
 
     if (fault == NoFault) {
         // check pte.u
-        STATUS status = tc->readMiscReg(MISCREG_STATUS);
-        PrivilegeMode pmode = getMemPriv(tc, mode);
         if (pmode == PrivilegeMode::PRV_U && !pte.u) {
             DPRINTF(TLB, "PTE is not user accessible, raising PF\n");
             fault = createPagefault(vaddr, mode);
@@ -260,6 +259,14 @@ TLB::createPagefault(Addr vaddr, Mode mode)
     return std::make_shared<AddressFault>(vaddr, code);
 }
 
+Addr
+TLB::translateWithTLB(Addr vaddr, uint16_t asid, Mode mode)
+{
+    TlbEntry *e = lookup(vaddr, asid, mode, false);
+    assert(e != nullptr);
+    return e->paddr << PageShift | (vaddr & mask(e->logBytes));
+}
+
 Fault
 TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
                  Translation *translation, Mode mode, bool &delayed)
@@ -281,7 +288,9 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
         assert(e != nullptr);
     }
 
-    Fault fault = checkPermissions(tc, vaddr, mode, e->pte);
+    STATUS status = tc->readMiscReg(MISCREG_STATUS);
+    PrivilegeMode pmode = getMemPriv(tc, mode);
+    Fault fault = checkPermissions(status, pmode, vaddr, mode, e->pte);
     if (fault != NoFault) {
         // if we want to write and it isn't writable, do a page table walk
         // again to update the dirty flag.
index a7e8d61f62e297976e2000a4cf142b6ceb2f1118..7ed66281783248bab28b4d67e794f8b701fe3734 100644 (file)
@@ -88,7 +88,7 @@ class TLB : public BaseTLB
     void flushAll() override;
     void demapPage(Addr vaddr, uint64_t asn) override;
 
-    Fault checkPermissions(ThreadContext *tc, Addr vaddr,
+    Fault checkPermissions(STATUS status, PrivilegeMode pmode, Addr vaddr,
                            Mode mode, PTESv39 pte);
     Fault createPagefault(Addr vaddr, Mode mode);
 
@@ -100,8 +100,7 @@ class TLB : public BaseTLB
 
     void regStats() override;
 
-    Fault doTranslate(const RequestPtr &req, ThreadContext *tc,
-                      Translation *translation, Mode mode, bool &delayed);
+    Addr translateWithTLB(Addr vaddr, uint16_t asid, Mode mode);
 
     Fault translateAtomic(const RequestPtr &req,
                           ThreadContext *tc, Mode mode) override;
@@ -122,6 +121,8 @@ class TLB : public BaseTLB
 
     Fault translate(const RequestPtr &req, ThreadContext *tc,
                     Translation *translation, Mode mode, bool &delayed);
+    Fault doTranslate(const RequestPtr &req, ThreadContext *tc,
+                      Translation *translation, Mode mode, bool &delayed);
 };
 
 }