Ayaz Akram has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/46279 )
Change subject: arch-riscv: add pma/pmp checks during page table walks
......................................................................
arch-riscv: add pma/pmp checks during page table walks
This change adds pma/pmp checks when page table entries
are accessed by hardware page table walker.
Change-Id: I161aad514bb7421e61a8c56af088c73969837704
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/46279
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/riscv/pagetable_walker.cc
M src/arch/riscv/pmp.cc
M src/arch/riscv/pmp.hh
3 files changed, 111 insertions(+), 68 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; 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 8dadd96..0a0c719 100644
--- a/src/arch/riscv/pagetable_walker.cc
+++ b/src/arch/riscv/pagetable_walker.cc
@@ -295,76 +295,101 @@
DPRINTF(PageTableWalker, "Got level%d PTE: %#x\n", level, pte);
- // step 2: TODO check PMA and PMP
+ // step 2:
+ // Performing PMA/PMP checks on physical address of PTE
- // step 3:
- if (!pte.v || (!pte.r && pte.w)) {
- doEndWalk = true;
- DPRINTF(PageTableWalker, "PTE invalid, raising PF\n");
- fault = pageFault(pte.v);
- }
- else {
- // step 4:
- if (pte.r || pte.x) {
- // step 5: leaf PTE
+ walker->pma->check(read->req);
+ // Effective privilege mode for pmp checks for page table
+ // walks is S mode according to specs
+ fault = walker->pmp->pmpCheck(read->req, mode,
+ RiscvISA::PrivilegeMode::PRV_S, tc, entry.vaddr);
+
+ if (fault == NoFault) {
+ // step 3:
+ if (!pte.v || (!pte.r && pte.w)) {
doEndWalk = true;
- fault = walker->tlb->checkPermissions(status, pmode,
- entry.vaddr, mode, pte);
-
- // step 6
- if (fault == NoFault) {
- if (level >= 1 && pte.ppn0 != 0) {
- DPRINTF(PageTableWalker,
- "PTE has misaligned PPN, raising PF\n");
- fault = pageFault(true);
- }
- else if (level == 2 && pte.ppn1 != 0) {
- DPRINTF(PageTableWalker,
- "PTE has misaligned PPN, raising PF\n");
- fault = pageFault(true);
- }
- }
-
- if (fault == NoFault) {
- // step 7
- if (!pte.a) {
- pte.a = 1;
- doWrite = true;
- }
- if (!pte.d && mode == TLB::Write) {
- pte.d = 1;
- doWrite = true;
- }
- // TODO check if this violates a PMA or PMP
-
- // step 8
- entry.logBytes = PageShift + (level * LEVEL_BITS);
- entry.paddr = pte.ppn;
- entry.vaddr &= ~((1 << entry.logBytes) - 1);
- entry.pte = pte;
- // put it non-writable into the TLB to detect writes and
redo
- // the page table walk in order to update the dirty flag.
- if (!pte.d && mode != TLB::Write)
- entry.pte.w = 0;
- doTLBInsert = true;
- }
+ DPRINTF(PageTableWalker, "PTE invalid, raising PF\n");
+ fault = pageFault(pte.v);
}
else {
- level--;
- if (level < 0) {
- DPRINTF(PageTableWalker, "No leaf PTE found, raising
PF\n");
+ // step 4:
+ if (pte.r || pte.x) {
+ // step 5: leaf PTE
doEndWalk = true;
- fault = pageFault(true);
- }
- else {
- Addr shift = (PageShift + LEVEL_BITS * level);
- Addr idx = (entry.vaddr >> shift) & LEVEL_MASK;
- nextRead = (pte.ppn << PageShift) + (idx * sizeof(pte));
- nextState = Translate;
+ fault = walker->tlb->checkPermissions(status, pmode,
+ entry.vaddr, mode,
pte);
+
+ // step 6
+ if (fault == NoFault) {
+ if (level >= 1 && pte.ppn0 != 0) {
+ DPRINTF(PageTableWalker,
+ "PTE has misaligned PPN, raising PF\n");
+ fault = pageFault(true);
+ }
+ else if (level == 2 && pte.ppn1 != 0) {
+ DPRINTF(PageTableWalker,
+ "PTE has misaligned PPN, raising PF\n");
+ fault = pageFault(true);
+ }
+ }
+
+ if (fault == NoFault) {
+ // step 7
+ if (!pte.a) {
+ pte.a = 1;
+ doWrite = true;
+ }
+ if (!pte.d && mode == TLB::Write) {
+ pte.d = 1;
+ doWrite = true;
+ }
+ // Performing PMA/PMP checks
+
+ if (doWrite) {
+
+ // this read will eventually become write
+ // if doWrite is True
+
+ walker->pma->check(read->req);
+
+ fault = walker->pmp->pmpCheck(read->req,
+ mode, pmode, tc, entry.vaddr);
+
+ }
+ // perform step 8 only if pmp checks pass
+ if (fault == NoFault) {
+
+ // step 8
+ entry.logBytes = PageShift + (level * LEVEL_BITS);
+ entry.paddr = pte.ppn;
+ entry.vaddr &= ~((1 << entry.logBytes) - 1);
+ entry.pte = pte;
+ // put it non-writable into the TLB to detect
+ // writes and redo the page table walk in order
+ // to update the dirty flag.
+ if (!pte.d && mode != TLB::Write)
+ entry.pte.w = 0;
+ doTLBInsert = true;
+ }
+ }
+ } else {
+ level--;
+ if (level < 0) {
+ DPRINTF(PageTableWalker, "No leaf PTE found,"
+ "raising PF\n");
+ doEndWalk = true;
+ fault = pageFault(true);
+ } else {
+ Addr shift = (PageShift + LEVEL_BITS * level);
+ Addr idx = (entry.vaddr >> shift) & LEVEL_MASK;
+ nextRead = (pte.ppn << PageShift) + (idx *
sizeof(pte));
+ nextState = Translate;
+ }
}
}
+ } else {
+ doEndWalk = true;
}
-
PacketPtr oldRead = read;
Request::Flags flags = oldRead->req->getFlags();
diff --git a/src/arch/riscv/pmp.cc b/src/arch/riscv/pmp.cc
index c0e53bf..af93a14 100644
--- a/src/arch/riscv/pmp.cc
+++ b/src/arch/riscv/pmp.cc
@@ -51,14 +51,21 @@
Fault
PMP::pmpCheck(const RequestPtr &req, BaseTLB::Mode mode,
- RiscvISA::PrivilegeMode pmode, ThreadContext *tc)
+ RiscvISA::PrivilegeMode pmode, ThreadContext *tc,
+ Addr vaddr)
{
// First determine if pmp table should be consulted
if (!shouldCheckPMP(pmode, mode, tc))
return NoFault;
- DPRINTF(PMP, "Checking pmp permissions for va: %#x , pa: %#x\n",
- req->getVaddr(), req->getPaddr());
+ if (req->hasVaddr()) {
+ DPRINTF(PMP, "Checking pmp permissions for va: %#x , pa: %#x\n",
+ req->getVaddr(), req->getPaddr());
+ }
+ else { // this access is corresponding to a page table walk
+ DPRINTF(PMP, "Checking pmp permissions for pa: %#x\n",
+ req->getPaddr());
+ }
// An access should be successful if there are
// no rules defined yet or we are in M mode (based
@@ -100,12 +107,20 @@
(PMP_EXEC & allowed_privs)) {
return NoFault;
} else {
- return createAddrfault(req->getVaddr(), mode);
+ if (req->hasVaddr()) {
+ return createAddrfault(req->getVaddr(), mode);
+ } else {
+ return createAddrfault(vaddr, mode);
+ }
}
}
}
// if no entry matched and we are not in M mode return fault
- return createAddrfault(req->getVaddr(), mode);
+ if (req->hasVaddr()) {
+ return createAddrfault(req->getVaddr(), mode);
+ } else {
+ return createAddrfault(vaddr, mode);
+ }
}
Fault
diff --git a/src/arch/riscv/pmp.hh b/src/arch/riscv/pmp.hh
index 3051a6a..aceed05 100644
--- a/src/arch/riscv/pmp.hh
+++ b/src/arch/riscv/pmp.hh
@@ -109,10 +109,13 @@
* @param mode mode of request (read, write, execute).
* @param pmode current privilege mode of execution (U, S, M).
* @param tc thread context.
+ * @param vaddr optional parameter to pass vaddr of original
+ * request for which a page table walk is consulted by pmp unit
* @return Fault.
*/
Fault pmpCheck(const RequestPtr &req, BaseTLB::Mode mode,
- RiscvISA::PrivilegeMode pmode, ThreadContext *tc);
+ RiscvISA::PrivilegeMode pmode, ThreadContext *tc,
+ Addr vaddr = 0);
/**
* pmpUpdateCfg updates the pmpcfg for a pmp
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/46279
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: I161aad514bb7421e61a8c56af088c73969837704
Gerrit-Change-Number: 46279
Gerrit-PatchSet: 4
Gerrit-Owner: Ayaz Akram <yazak...@ucdavis.edu>
Gerrit-Reviewer: Ayaz Akram <yazak...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
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