Nils Asmussen has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/28447 )
Change subject: arch-riscv: be prepared for CSR changes during PT walk.
......................................................................
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 <bbr...@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/riscv/pagetable_walker.cc
M src/arch/riscv/pagetable_walker.hh
M src/arch/riscv/tlb.cc
M src/arch/riscv/tlb.hh
4 files changed, 33 insertions(+), 17 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/riscv/pagetable_walker.cc
b/src/arch/riscv/pagetable_walker.cc
index 02d2d94..6ec118d 100644
--- a/src/arch/riscv/pagetable_walker.cc
+++ b/src/arch/riscv/pagetable_walker.cc
@@ -184,6 +184,11 @@
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 @@
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 @@
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 @@
* 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);
diff --git a/src/arch/riscv/pagetable_walker.hh
b/src/arch/riscv/pagetable_walker.hh
index 2d50928..60826a0 100644
--- a/src/arch/riscv/pagetable_walker.hh
+++ b/src/arch/riscv/pagetable_walker.hh
@@ -100,6 +100,9 @@
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 294d73f..c8b1a4c 100644
--- a/src/arch/riscv/tlb.cc
+++ b/src/arch/riscv/tlb.cc
@@ -213,7 +213,8 @@
}
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 @@
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 @@
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 @@
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 a7e8d61..7ed6628 100644
--- a/src/arch/riscv/tlb.hh
+++ b/src/arch/riscv/tlb.hh
@@ -88,7 +88,7 @@
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 @@
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 @@
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);
};
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/28447
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I8c184c7ae7dd44d78e881bb5ec8d430dd480849c
Gerrit-Change-Number: 28447
Gerrit-PatchSet: 2
Gerrit-Owner: Nils Asmussen <nils.asmus...@barkhauseninstitut.org>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Nils Asmussen <nils.asmus...@barkhauseninstitut.org>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
