Skip to content

Commit

Permalink
cpu-o3: Fix tlb priv race when handling trap
Browse files Browse the repository at this point in the history
Previously, `fault->invoke()` updates priv immediately, TLB also
translate using new priv. There still might be some instructions going
to IEW at the moment. This is because interrupt traps when ROB is empty,
but somehow these instructions are not in ROB yet.
Such data race on processor PRIV causes mem accesses to bypass addr
translation, thereby accessing a non-existent paddr, which would not be
filtered out because GEM5 thinks they are PIO addrs.
This fix makes TLB use old privilege mode until squashing signal is
sent to IEW.

Change-Id: I016e81069d090269afe4e40f14396cd9163f9ccd
  • Loading branch information
notlqr committed Oct 16, 2024
1 parent 73679ae commit f71099d
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/arch/generic/mmu.hh
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ class BaseMMU : public SimObject
translateFunctional(const RequestPtr &req, ThreadContext *tc,
Mode mode);

virtual void setOldPriv(ThreadContext *tc) {
panic("BaseMMU::setOldPriv() not implemented");
}

virtual void useNewPriv(ThreadContext *tc) {
panic("BaseMMU::useNewPriv() not implemented");
}

class MMUTranslationGen : public TranslationGen
{
private:
Expand Down
12 changes: 12 additions & 0 deletions src/arch/riscv/mmu.hh
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ class MMU : public BaseMMU
{
return static_cast<TLB*>(dtb)->pmp;
}

virtual void
setOldPriv(ThreadContext *tc) {
static_cast<TLB*>(dtb)->setOldPriv(tc);
static_cast<TLB*>(itb)->setOldPriv(tc);
}

virtual void
useNewPriv(ThreadContext *tc) {
static_cast<TLB*>(dtb)->useNewPriv(tc);
static_cast<TLB*>(itb)->useNewPriv(tc);
}
};

} // namespace RiscvISA
Expand Down
7 changes: 7 additions & 0 deletions src/arch/riscv/tlb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1961,6 +1961,13 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
PrivilegeMode
TLB::getMemPriv(ThreadContext *tc, BaseMMU::Mode mode)
{
if (use_old_priv && mode != BaseMMU::Execute) {
if (mode == BaseMMU::Execute) {
return old_priv_ex;
} else {
return old_priv_ldst;
}
}
STATUS status = (STATUS)tc->readMiscReg(MISCREG_STATUS);
PrivilegeMode pmode = (PrivilegeMode)tc->readMiscReg(MISCREG_PRV);
if (mode != BaseMMU::Execute && status.mprv == 1)
Expand Down
11 changes: 11 additions & 0 deletions src/arch/riscv/tlb.hh
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ class TLB : public BaseTLB
uint64_t lastPc;
uint64_t traceFlag;

bool use_old_priv;
PrivilegeMode old_priv_ldst;
PrivilegeMode old_priv_ex;

Walker *walker;

Expand Down Expand Up @@ -251,6 +254,14 @@ class TLB : public BaseTLB
TlbEntry *lookupL2TLB(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden, int f_level, bool sign_used,
uint8_t translateMode);

void setOldPriv(ThreadContext *tc) {
use_old_priv = true;
old_priv_ex = getMemPriv(tc, BaseMMU::Execute);
old_priv_ldst = getMemPriv(tc, BaseMMU::Read);
}
void useNewPriv(ThreadContext *tc) {
use_old_priv = false;
}


std::vector<TlbEntry> tlbL2L1; // our TLB
Expand Down
6 changes: 6 additions & 0 deletions src/cpu/o3/commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,8 @@ Commit::squashAll(ThreadID tid)
toIEW->commitInfo[tid].squashedTargetId = committedTargetId;
toIEW->commitInfo[tid].squashedLoopIter = committedLoopIter;

cpu->mmu->useNewPriv(cpu->getContext(tid));

squashInflightAndUpdateVersion(tid);
}

Expand Down Expand Up @@ -841,6 +843,8 @@ Commit::handleInterrupt()
DPRINTF(CommitTrace, "Handle interrupt No.%lx\n", cpu->getInterruptsNO() | (1ULL << 63));
cpu->processInterrupts(cpu->getInterrupts());

cpu->mmu->setOldPriv(cpu->getContext(0));

thread[0]->noSquashFromTC = false;

commitStatus[0] = TrapPending;
Expand Down Expand Up @@ -1528,6 +1532,8 @@ Commit::commitHead(const DynInstPtr &head_inst, unsigned inst_num)
head_inst->notAnInst() ? nullStaticInstPtr :
head_inst->staticInst);

cpu->mmu->setOldPriv(cpu->getContext(tid));

// Exit state update mode to avoid accidental updating.
thread[tid]->noSquashFromTC = false;

Expand Down
2 changes: 1 addition & 1 deletion src/cpu/o3/fetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ Fetch::finishTranslation(const Fault &fault, const RequestPtr &mem_req)
// If we have, just wait around for commit to squash something and put
// us on the right track
if (!cpu->system->isMemAddr(mem_req->getPaddr())) {
warn("Address %#x is outside of physical memory, stopping fetch, %lu\n",
DPRINTF(Fetch, "Address %#x is outside of physical memory, stopping fetch, %lu\n",
mem_req->getPaddr(), curTick());
fetchStatus[tid] = NoGoodAddr;
setAllFetchStalls(StallReason::OtherFetchStall);
Expand Down

0 comments on commit f71099d

Please sign in to comment.