Roger Chang has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/69497?usp=email )
Change subject: arch-riscv: Fix the behavior of pmp
......................................................................
arch-riscv: Fix the behavior of pmp
1. Refactor the shouldCheckPMP function
2. Fix the AddrRange of pmp region. the contains of
AddrRange(start, end) will be valid if the address y is in
start <= y < end. It should not minus 1 in end parameter
Change-Id: I842687674fed7bc4d88a9ba6b4c4d52c3459068f
---
M src/arch/riscv/pmp.cc
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/src/arch/riscv/pmp.cc b/src/arch/riscv/pmp.cc
index 49dc7ba..eaac8b6 100644
--- a/src/arch/riscv/pmp.cc
+++ b/src/arch/riscv/pmp.cc
@@ -71,9 +71,6 @@
req->getPaddr());
}
- if (numRules == 0)
- return NoFault;
-
// match_index will be used to identify the pmp entry
// which matched for the given address
int match_index = -1;
@@ -83,9 +80,9 @@
for (int i = 0; i < pmpTable.size(); i++) {
AddrRange pmp_range = pmpTable[i].pmpAddr;
if (pmp_range.contains(req->getPaddr()) &&
- pmp_range.contains(req->getPaddr() + req->getSize())) {
+ pmp_range.contains(req->getPaddr() + req->getSize() - 1)) {
// according to specs address is only matched,
- // when (addr) and (addr + request_size) are both
+ // when (addr) and (addr + request_size - 1) are both
// within the pmp range
match_index = i;
}
@@ -197,11 +194,11 @@
break;
case PMP_TOR:
// top of range mode
- this_range = AddrRange(prevAddr << 2, (this_addr << 2) - 1);
+ this_range = AddrRange(prevAddr << 2, (this_addr << 2));
break;
case PMP_NA4:
// naturally aligned four byte region
- this_range = AddrRange(this_addr << 2, (this_addr + 4) - 1);
+ this_range = AddrRange(this_addr << 2, (this_addr + 4));
break;
case PMP_NAPOT:
// naturally aligned power of two region, >= 8 bytes
@@ -246,7 +243,7 @@
}
DPRINTF(PMP, "Update pmp addr %#x for pmp entry %u \n",
- this_addr, pmp_index);
+ (this_addr << 2), pmp_index);
if (pmpTable[pmp_index].pmpCfg & PMP_LOCK) {
DPRINTF(PMP, "Update pmp entry %u failed because the lock bit
set\n",
@@ -276,23 +273,12 @@
PMP::shouldCheckPMP(RiscvISA::PrivilegeMode pmode,
BaseMMU::Mode mode, ThreadContext *tc)
{
- // instruction fetch in S and U mode
- bool cond1 = (mode == BaseMMU::Execute &&
- (pmode != RiscvISA::PrivilegeMode::PRV_M));
-
- // data access in S and U mode when MPRV in mstatus is clear
- RiscvISA::STATUS status =
- tc->readMiscRegNoEffect(RiscvISA::MISCREG_STATUS);
- bool cond2 = (mode != BaseMMU::Execute &&
- (pmode != RiscvISA::PrivilegeMode::PRV_M)
- && (!status.mprv));
-
- // data access in any mode when MPRV bit in mstatus is set
- // and the MPP field in mstatus is S or U
- bool cond3 = (mode != BaseMMU::Execute && (status.mprv)
- && (status.mpp != RiscvISA::PrivilegeMode::PRV_M));
-
- return (cond1 || cond2 || cond3 || hasLockEntry);
+ // The privilege mode of memory read and write access
+ // is modified by TLB, it can just simply check if
+ // the privilege mode is M or the numRules is zero
+ // or has lock entry
+ return (pmode != RiscvISA::PrivilegeMode::PRV_M ||
+ numRules == 0 || hasLockEntry);
}
AddrRange
@@ -303,7 +289,7 @@
return this_range;
} else {
uint64_t t1 = ctz64(~pmpaddr);
- uint64_t range = (std::pow(2,t1+3))-1;
+ uint64_t range = (std::pow(2,t1+3));
// pmpaddr reg encodes bits 55-2 of a
// 56 bit physical address for RV64
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/69497?usp=email
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: I842687674fed7bc4d88a9ba6b4c4d52c3459068f
Gerrit-Change-Number: 69497
Gerrit-PatchSet: 1
Gerrit-Owner: Roger Chang <rogerycch...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
