From 8a9a629bdb346b49d592d11367c2b6ba76702d52 Mon Sep 17 00:00:00 2001 From: Roger Chang Date: Fri, 17 Feb 2023 20:20:36 +0800 Subject: [PATCH] arch-riscv: Support PMP lock feature The lock feature will let M mode do memory permission check before R/W/X data. If the lock bit of pmpicfg set, then the pmpicfg and pmpaddri will ignore the update value later until CPU reset, and pmpaddri-1 will ignore if the TOR A field is set. The following is add in CL: 1. Add condition to run PMP check when any lock bit of pmp tables is set 2. Add PMP_LOCK bit check when try to update pmpaddr and pmpcfg 3. If there is no PMP entry matches and priviledge mode is M, no fault generated 4. If the address matches PMP entry, return no fault if priviledge mode is M and lock bit is not set For more details about PMP, please see RISC-V Spec Volumn II, Priviledge Archtecture, Ver 1.12, Section 3.7 Physical Memory Protection Change-Id: I3e7c5824d6c05f2ea928ee9ec7714f7271e4c58c Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/68057 Reviewed-by: Ayaz Akram Tested-by: kokoro Reviewed-by: Yu-hsin Wang Maintainer: Bobby Bruce --- src/arch/riscv/faults.cc | 10 ++++++ src/arch/riscv/isa.cc | 18 +++++++--- src/arch/riscv/pmp.cc | 75 ++++++++++++++++++++++++++++------------ src/arch/riscv/pmp.hh | 19 ++++++++-- 4 files changed, 93 insertions(+), 29 deletions(-) diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc index 3469c71252..940f7107ba 100644 --- a/src/arch/riscv/faults.cc +++ b/src/arch/riscv/faults.cc @@ -33,6 +33,8 @@ #include "arch/riscv/insts/static_inst.hh" #include "arch/riscv/isa.hh" +#include "arch/riscv/mmu.hh" +#include "arch/riscv/pmp.hh" #include "arch/riscv/regs/misc.hh" #include "arch/riscv/utility.hh" #include "cpu/base.hh" @@ -180,6 +182,14 @@ Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) tc->getIsaPtr()->newPCState(workload->getEntry()))); panic_if(!new_pc, "Failed create new PCState from ISA pointer"); tc->pcState(*new_pc); + + // Reset PMP Cfg + auto* mmu = dynamic_cast(tc->getMMUPtr()); + if (mmu == nullptr) { + warn("MMU is not Riscv MMU instance, we can't reset PMP"); + return; + } + mmu->getPMP()->pmpReset(); } void diff --git a/src/arch/riscv/isa.cc b/src/arch/riscv/isa.cc index 3809c61d63..d778957b9e 100644 --- a/src/arch/riscv/isa.cc +++ b/src/arch/riscv/isa.cc @@ -2,6 +2,7 @@ * Copyright (c) 2016 RISC-V Foundation * Copyright (c) 2016 The University of Virginia * Copyright (c) 2020 Barkhausen Institut + * Copyright (c) 2022 Google LLC * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -544,6 +545,8 @@ ISA::setMiscReg(RegIndex idx, RegVal val) // qemu seems to update the tables when // pmp addr regs are written (with the assumption // that cfg regs are already written) + RegVal res = 0; + RegVal old_val = readMiscRegNoEffect(idx); for (int i=0; i < regSize; i++) { @@ -554,10 +557,15 @@ ISA::setMiscReg(RegIndex idx, RegVal val) // Form pmp_index using the index i and // PMPCFG register number uint32_t pmp_index = i+(4*(idx-MISCREG_PMPCFG0)); - mmu->getPMP()->pmpUpdateCfg(pmp_index,cfg_val); + bool result = mmu->getPMP()->pmpUpdateCfg(pmp_index,cfg_val); + if (result) { + res |= ((RegVal)cfg_val << (8*i)); + } else { + res |= (old_val & (0xFF << (8*i))); + } } - setMiscRegNoEffect(idx, val); + setMiscRegNoEffect(idx, res); } break; case MISCREG_PMPADDR00 ... MISCREG_PMPADDR15: @@ -568,9 +576,9 @@ ISA::setMiscReg(RegIndex idx, RegVal val) auto mmu = dynamic_cast (tc->getMMUPtr()); uint32_t pmp_index = idx-MISCREG_PMPADDR00; - mmu->getPMP()->pmpUpdateAddr(pmp_index, val); - - setMiscRegNoEffect(idx, val); + if (mmu->getPMP()->pmpUpdateAddr(pmp_index, val)) { + setMiscRegNoEffect(idx, val); + } } break; diff --git a/src/arch/riscv/pmp.cc b/src/arch/riscv/pmp.cc index 77ef98f2d0..940af47686 100644 --- a/src/arch/riscv/pmp.cc +++ b/src/arch/riscv/pmp.cc @@ -1,5 +1,6 @@ /* * Copyright (c) 2021 The Regents of the University of California + * Copyright (c) 2023 Google LLC * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -27,7 +28,6 @@ */ #include "arch/riscv/pmp.hh" - #include "arch/generic/tlb.hh" #include "arch/riscv/faults.hh" #include "arch/riscv/isa.hh" @@ -47,7 +47,8 @@ namespace gem5 PMP::PMP(const Params ¶ms) : SimObject(params), pmpEntries(params.pmp_entries), - numRules(0) + numRules(0), + hasLockEntry(false) { pmpTable.resize(pmpEntries); } @@ -70,10 +71,7 @@ PMP::pmpCheck(const RequestPtr &req, BaseMMU::Mode mode, req->getPaddr()); } - // An access should be successful if there are - // no rules defined yet or we are in M mode (based - // on specs v1.10) - if (numRules == 0 || (pmode == RiscvISA::PrivilegeMode::PRV_M)) + if (numRules == 0) return NoFault; // match_index will be used to identify the pmp entry @@ -94,20 +92,19 @@ PMP::pmpCheck(const RequestPtr &req, BaseMMU::Mode mode, if ((match_index > -1) && (PMP_OFF != pmpGetAField(pmpTable[match_index].pmpCfg))) { - // check the RWX permissions from the pmp entry - uint8_t allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC; + uint8_t this_cfg = pmpTable[match_index].pmpCfg; - // i is the index of pmp table which matched - allowed_privs &= pmpTable[match_index].pmpCfg; - - if ((mode == BaseMMU::Mode::Read) && - (PMP_READ & allowed_privs)) { + if ((pmode == RiscvISA::PrivilegeMode::PRV_M) && + (PMP_LOCK & this_cfg) == 0) { + return NoFault; + } else if ((mode == BaseMMU::Mode::Read) && + (PMP_READ & this_cfg)) { return NoFault; } else if ((mode == BaseMMU::Mode::Write) && - (PMP_WRITE & allowed_privs)) { + (PMP_WRITE & this_cfg)) { return NoFault; } else if ((mode == BaseMMU::Mode::Execute) && - (PMP_EXEC & allowed_privs)) { + (PMP_EXEC & this_cfg)) { return NoFault; } else { if (req->hasVaddr()) { @@ -119,7 +116,9 @@ PMP::pmpCheck(const RequestPtr &req, BaseMMU::Mode mode, } } // if no entry matched and we are not in M mode return fault - if (req->hasVaddr()) { + if (pmode == RiscvISA::PrivilegeMode::PRV_M) { + return NoFault; + } else if (req->hasVaddr()) { return createAddrfault(req->getVaddr(), mode); } else { return createAddrfault(vaddr, mode); @@ -150,17 +149,19 @@ PMP::pmpGetAField(uint8_t cfg) } -void +bool PMP::pmpUpdateCfg(uint32_t pmp_index, uint8_t this_cfg) { DPRINTF(PMP, "Update pmp config with %u for pmp entry: %u \n", (unsigned)this_cfg, pmp_index); - - warn_if((PMP_LOCK & this_cfg), "pmp lock feature is not supported.\n"); - + if (pmpTable[pmp_index].pmpCfg & PMP_LOCK) { + DPRINTF(PMP, "Update pmp entry config %u failed because it locked\n", + pmp_index); + return false; + } pmpTable[pmp_index].pmpCfg = this_cfg; pmpUpdateRule(pmp_index); - + return true; } void @@ -170,6 +171,7 @@ PMP::pmpUpdateRule(uint32_t pmp_index) // pmpaddr/pmpcfg is written numRules = 0; + hasLockEntry = false; Addr prevAddr = 0; if (pmp_index >= 1) { @@ -209,15 +211,42 @@ PMP::pmpUpdateRule(uint32_t pmp_index) if (PMP_OFF != a_field) { numRules++; } + hasLockEntry |= ((pmpTable[i].pmpCfg & PMP_LOCK) != 0); + } + + if (hasLockEntry) { + DPRINTF(PMP, "Find lock entry\n"); } } void +PMP::pmpReset() +{ + for (uint32_t i = 0; i < pmpTable.size(); i++) { + pmpTable[i].pmpCfg &= ~(PMP_A_MASK | PMP_LOCK); + pmpUpdateRule(i); + } +} + +bool PMP::pmpUpdateAddr(uint32_t pmp_index, Addr this_addr) { DPRINTF(PMP, "Update pmp addr %#x for pmp entry %u \n", this_addr, pmp_index); + if (pmpTable[pmp_index].pmpCfg & PMP_LOCK) { + DPRINTF(PMP, "Update pmp entry %u failed because the lock bit set\n", + pmp_index); + return false; + } else if (pmp_index < pmpTable.size() - 1 && + ((pmpTable[pmp_index+1].pmpCfg & PMP_LOCK) != 0) && + pmpGetAField(pmpTable[pmp_index+1].pmpCfg) == PMP_TOR) { + DPRINTF(PMP, "Update pmp entry %u failed because the entry %u lock bit set" + "and A field is TOR\n", + pmp_index, pmp_index+1); + return false; + } + // just writing the raw addr in the pmp table // will convert it into a range, once cfg // reg is written @@ -225,6 +254,8 @@ PMP::pmpUpdateAddr(uint32_t pmp_index, Addr this_addr) for (int index = 0; index < pmpEntries; index++) { pmpUpdateRule(index); } + + return true; } bool @@ -247,7 +278,7 @@ PMP::shouldCheckPMP(RiscvISA::PrivilegeMode pmode, bool cond3 = (mode != BaseMMU::Execute && (status.mprv) && (status.mpp != RiscvISA::PrivilegeMode::PRV_M)); - return (cond1 || cond2 || cond3); + return (cond1 || cond2 || cond3 || hasLockEntry); } AddrRange diff --git a/src/arch/riscv/pmp.hh b/src/arch/riscv/pmp.hh index 1509646850..24cb4ad1ca 100644 --- a/src/arch/riscv/pmp.hh +++ b/src/arch/riscv/pmp.hh @@ -1,5 +1,6 @@ /* * Copyright (c) 2021 The Regents of the University of California + * Copyright (c) 2023 Google LLC * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -85,12 +86,18 @@ class PMP : public SimObject /** pmpcfg address range execute permission mask */ const uint8_t PMP_EXEC = 1 << 2; + /** pmpcfg A field mask */ + const uint8_t PMP_A_MASK = 3 << 3; + /** pmpcfg address range locked mask */ const uint8_t PMP_LOCK = 1 << 7; /** variable to keep track of active number of rules any time */ int numRules; + /** variable to keep track of any lock of entry */ + bool hasLockEntry; + /** single pmp entry struct*/ struct PmpEntry { @@ -127,8 +134,9 @@ class PMP : public SimObject * rule of corresponding pmp entry. * @param pmp_index pmp entry index. * @param this_cfg value to be written to pmpcfg. + * @returns true if update pmpicfg success */ - void pmpUpdateCfg(uint32_t pmp_index, uint8_t this_cfg); + bool pmpUpdateCfg(uint32_t pmp_index, uint8_t this_cfg); /** * pmpUpdateAddr updates the pmpaddr for a pmp @@ -136,8 +144,15 @@ class PMP : public SimObject * rule of corresponding pmp entry. * @param pmp_index pmp entry index. * @param this_addr value to be written to pmpaddr. + * @returns true if update pmpaddri success */ - void pmpUpdateAddr(uint32_t pmp_index, Addr this_addr); + bool pmpUpdateAddr(uint32_t pmp_index, Addr this_addr); + + /** + * pmpReset reset when reset signal in trigger from + * CPU. + */ + void pmpReset(); private: /**