From 72e3c9687234405c45e52b09ed27fb9b871e78ab Mon Sep 17 00:00:00 2001 From: Nils Asmussen Date: Fri, 1 May 2020 10:27:01 +0200 Subject: [PATCH] arch-riscv: be prepared for CSR changes during PT walk. 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 Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/arch/riscv/pagetable_walker.cc | 23 +++++++++++++---------- src/arch/riscv/pagetable_walker.hh | 3 +++ src/arch/riscv/tlb.cc | 17 +++++++++++++---- src/arch/riscv/tlb.hh | 7 ++++--- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/arch/riscv/pagetable_walker.cc b/src/arch/riscv/pagetable_walker.cc index 02d2d945b..6ec118d56 100644 --- a/src/arch/riscv/pagetable_walker.cc +++ b/src/arch/riscv/pagetable_walker.cc @@ -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(1) << VADDR_BITS) - 1); - - SATP satp = tc->readMiscReg(MISCREG_SATP); - assert(satp.mode == AddrXlateMode::SV39); + vaddr &= (static_cast(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(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); diff --git a/src/arch/riscv/pagetable_walker.hh b/src/arch/riscv/pagetable_walker.hh index 2d509284e..60826a055 100644 --- a/src/arch/riscv/pagetable_walker.hh +++ b/src/arch/riscv/pagetable_walker.hh @@ -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; diff --git a/src/arch/riscv/tlb.cc b/src/arch/riscv/tlb.cc index 294d73ff5..c8b1a4c6f 100644 --- a/src/arch/riscv/tlb.cc +++ b/src/arch/riscv/tlb.cc @@ -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(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. diff --git a/src/arch/riscv/tlb.hh b/src/arch/riscv/tlb.hh index a7e8d61f6..7ed662817 100644 --- a/src/arch/riscv/tlb.hh +++ b/src/arch/riscv/tlb.hh @@ -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); }; } -- 2.30.2