Roger Chang has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/69497?usp=email ) Change subject: arch-riscv: Refactor the shouldCheckPMP function ...................................................................... arch-riscv: Refactor the shouldCheckPMP function The shouldCheckPMP can be simply with pmode != PRV_M since the privilege mode of memory is modified by TLB and Walker. The numRules check can done in shouldPMPCheck Change-Id: I842687674fed7bc4d88a9ba6b4c4d52c3459068f Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/69497 Maintainer: Bobby Bruce <bbruce@ucdavis.edu> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Yu-hsin Wang <yuhsingw@google.com> --- M src/arch/riscv/pmp.cc M src/arch/riscv/pmp.hh 2 files changed, 11 insertions(+), 28 deletions(-) Approvals: kokoro: Regressions pass Bobby Bruce: Looks good to me, approved Yu-hsin Wang: Looks good to me, approved diff --git a/src/arch/riscv/pmp.cc b/src/arch/riscv/pmp.cc index 6275104..8fa1ca3 100644 --- a/src/arch/riscv/pmp.cc +++ b/src/arch/riscv/pmp.cc @@ -59,7 +59,7 @@ Addr vaddr) { // First determine if pmp table should be consulted - if (!shouldCheckPMP(pmode, mode, tc)) + if (!shouldCheckPMP(pmode, tc)) return NoFault; if (req->hasVaddr()) { @@ -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; @@ -273,26 +270,14 @@ } bool -PMP::shouldCheckPMP(RiscvISA::PrivilegeMode pmode, - BaseMMU::Mode mode, ThreadContext *tc) +PMP::shouldCheckPMP(RiscvISA::PrivilegeMode pmode, 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 + // is modified by TLB. It can just simply check if + // the numRule is not zero, then return true if + // privilege mode is not M or has any lock entry + return numRules != 0 && ( + pmode != RiscvISA::PrivilegeMode::PRV_M || hasLockEntry); } AddrRange diff --git a/src/arch/riscv/pmp.hh b/src/arch/riscv/pmp.hh index 24cb4ad..ff8c4fc 100644 --- a/src/arch/riscv/pmp.hh +++ b/src/arch/riscv/pmp.hh @@ -118,7 +118,7 @@ * is allowed based on the pmp rules. * @param req memory request. * @param mode mode of request (read, write, execute). - * @param pmode current privilege mode of execution (U, S, M). + * @param pmode current privilege mode of memory (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 @@ -159,13 +159,11 @@ * This function is called during a memory * access to determine if the pmp table * should be consulted for this access. - * @param pmode current privilege mode of execution (U, S, M). - * @param mode mode of request (read, write, execute). + * @param pmode current privilege mode of memory (U, S, M). * @param tc thread context. * @return true or false. */ - bool shouldCheckPMP(RiscvISA::PrivilegeMode pmode, - BaseMMU::Mode mode, ThreadContext *tc); + bool shouldCheckPMP(RiscvISA::PrivilegeMode pmode, ThreadContext *tc); /** * createAddrfault creates an address fault -- 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: 5 Gerrit-Owner: Roger Chang <rogerycchang@google.com> Gerrit-Reviewer: Ayaz Akram <yazakram@ucdavis.edu> Gerrit-Reviewer: Bobby Bruce <bbruce@ucdavis.edu> Gerrit-Reviewer: Hoa Nguyen <hoanguyen@ucdavis.edu> Gerrit-Reviewer: Jui-min Lee <fcrh@google.com> Gerrit-Reviewer: Roger Chang <rogerycchang@google.com> Gerrit-Reviewer: Yu-hsin Wang <yuhsingw@google.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-MessageType: merged