derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

arch-riscv: Fix TLB lookup with vaddrs (#1264)

Browse Source 
Previously, all of the TLB lookup/insert functions were using the full
virtual addresses even though the variables in the functions said "vpn."
This change explicitly converts the virtual address to the VPN without
any least significant zeros for the offset. I.e., vpn >> page_size.

The main bug solved in this changeset is the asid was |'d with the upper
bits of the virtual address, but sometimes there were all 1's.
Therefore, you could get a TLB hit even if the ASID was different.
Interestingly, the page that seemed to cause these issues was a 1 GiB
page.

This change also starts refactoring some of the page table details to
support sv46 and sv57 page table formats.

In my testing, the Linux kernel boot uses large pages (even OpenSBI uses
large pages), so it seems that large pages also work. However, this
seems like magic to me, so I'm not sure if it's correct.

This change also updates some asserts, and debug statements with more
useful debugging information.

Partially fixes #1235. More testing needs to be done to be confident.
This commit is contained in:
Jason Lowe-Power
2024-06-20 13:24:50 -07:00
committed by GitHub
parent 7137b73ca0
commit 013f773d31
5 changed files with 112 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/arch/riscv/pagetable.cc
View File

@@ -60,5 +60,20 @@ TlbEntry::unserialize(CheckpointIn &cp)
UNSERIALIZE_SCALAR(lruSeq);
}
Addr
getVPNFromVAddr(Addr vaddr, Addr mode)
{
switch (mode) {
case SV39:
return bits(vaddr, 38, 12);
case SV48:
return bits(vaddr, 47, 12);
case SV57:
return bits(vaddr, 56, 12);
default:
panic("Unknown address translation mode %d\n", mode);
}
}
} // namespace RiscvISA
} // namespace gem5

8
src/arch/riscv/pagetable.hh
View File

@@ -53,6 +53,7 @@ enum AddrXlateMode
BARE = 0,
SV39 = 8,
SV48 = 9,
SV57 = 10,
};
// Sv39 paging
@@ -76,6 +77,13 @@ BitUnion64(PTESv39)
Bitfield<0> v;
EndBitUnion(PTESv39)
/**
* Remove the page offset and the upper bits that are
* not part of the VPN from the address.
* Note that this must assume the smallest page size
*/
Addr getVPNFromVAddr(Addr vaddr, Addr mode);
struct TlbEntry;
typedef Trie<Addr, TlbEntry> TlbEntryTrie;

16
src/arch/riscv/pagetable_walker.cc
View File

@@ -203,7 +203,8 @@ Walker::startWalkWrapper()
// check if we get a tlb hit to skip the walk
Addr vaddr = Addr(sext<VADDR_BITS>(currState->req->getVaddr()));
TlbEntry *e = tlb->lookup(vaddr, currState->satp.asid, currState->mode,
Addr vpn = getVPNFromVAddr(vaddr, currState->satp.mode);
TlbEntry *e = tlb->lookup(vpn, currState->satp.asid, currState->mode,
true);
Fault fault = NoFault;
if (e) {
@@ -242,7 +243,8 @@ Walker::startWalkWrapper()
if (currStates.size()) {
currState = currStates.front();
vaddr = Addr(sext<VADDR_BITS>(currState->req->getVaddr()));
e = tlb->lookup(vaddr, currState->satp.asid, currState->mode,
Addr vpn = getVPNFromVAddr(vaddr, currState->satp.mode);
e = tlb->lookup(vpn, currState->satp.asid, currState->mode,
true);
if (e) {
fault = tlb->checkPermissions(currState->status,
@@ -461,9 +463,10 @@ Walker::WalkerState::stepWalk(PacketPtr &write)
}
if (doTLBInsert) {
if (!functional)
walker->tlb->insert(entry.vaddr, entry);
else {
if (!functional) {
Addr vpn = getVPNFromVAddr(entry.vaddr, satp.mode);
walker->tlb->insert(vpn, entry);
} else {
DPRINTF(PageTableWalker, "Translated %#x -> %#x\n",
entry.vaddr, entry.paddr << PageShift |
(entry.vaddr & mask(entry.logBytes)));
@@ -571,7 +574,8 @@ Walker::WalkerState::recvPacket(PacketPtr pkt)
*/
Addr vaddr = req->getVaddr();
vaddr = Addr(sext<VADDR_BITS>(vaddr));
Addr paddr = walker->tlb->translateWithTLB(vaddr, satp.asid, mode);
Addr paddr = walker->tlb->translateWithTLB(vaddr, satp.asid,
satp.mode, mode);
req->setPaddr(paddr);
// do pmp check if any checking condition is met.

84
src/arch/riscv/tlb.cc
View File

@@ -67,6 +67,12 @@ using namespace RiscvISA;
static Addr
buildKey(Addr vpn, uint16_t asid)
{
// Note ASID is 16 bits
// The VPN in sv39 is up to 39-12=27 bits
// The VPN in sv48 is up to 48-12=36 bits
// The VPN in sv57 is up to 57-12=45 bits
// So, shifting the ASID into the top 16 bits is safe.
assert(bits(vpn, 63, 48) == 0);
return (static_cast<Addr>(asid) << 48) | vpn;
}
@@ -110,6 +116,12 @@ TLB::lookup(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden)
{
TlbEntry *entry = trie.lookup(buildKey(vpn, asid));
DPRINTF(TLBVerbose, "lookup(vpn=%#x, asid=%#x, key=%#x): "
"%s ppn=%#x (%#x) %s\n",
vpn, asid, buildKey(vpn, asid), entry ? "hit" : "miss",
entry ? entry->paddr : 0, entry ? entry->size() : 0,
hidden ? "hidden" : "");
if (!hidden) {
if (entry)
entry->lruSeq = nextSeq();
@@ -131,9 +143,6 @@ TLB::lookup(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden)
else
stats.readHits++;
}
DPRINTF(TLBVerbose, "lookup(vpn=%#x, asid=%#x): %s ppn %#x\n",
vpn, asid, entry ? "hit" : "miss", entry ? entry->paddr : 0);
}
return entry;
@@ -142,15 +151,19 @@ TLB::lookup(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden)
TlbEntry *
TLB::insert(Addr vpn, const TlbEntry &entry)
{
DPRINTF(TLB, "insert(vpn=%#x, asid=%#x): ppn=%#x pte=%#x size=%#x\n",
vpn, entry.asid, entry.paddr, entry.pte, entry.size());
DPRINTF(TLB, "insert(vpn=%#x, asid=%#x, key=%#x): "
"vaddr=%#x paddr=%#x pte=%#x size=%#x\n",
vpn, entry.asid, buildKey(vpn, entry.asid), entry.vaddr, entry.paddr,
entry.pte, entry.size());
// If somebody beat us to it, just use that existing entry.
TlbEntry *newEntry = lookup(vpn, entry.asid, BaseMMU::Read, true);
if (newEntry) {
// update PTE flags (maybe we set the dirty/writable flag)
newEntry->pte = entry.pte;
assert(newEntry->vaddr == vpn);
assert(newEntry->vaddr == entry.vaddr);
assert(newEntry->asid == entry.asid);
assert(newEntry->logBytes == entry.logBytes);
return newEntry;
}
@@ -163,31 +176,46 @@ TLB::insert(Addr vpn, const TlbEntry &entry)
Addr key = buildKey(vpn, entry.asid);
*newEntry = entry;
newEntry->lruSeq = nextSeq();
newEntry->vaddr = vpn;
newEntry->trieHandle =
trie.insert(key, TlbEntryTrie::MaxBits - entry.logBytes, newEntry);
newEntry->trieHandle = trie.insert(
key, TlbEntryTrie::MaxBits - entry.logBytes + PageShift, newEntry
);
return newEntry;
}
void
TLB::demapPage(Addr vpn, uint64_t asid)
TLB::demapPage(Addr vaddr, uint64_t asid)
{
// Note: vaddr is Reg[rs1] and asid is Reg[rs2]
// The definition of this instruction is
// if vaddr=x0 and asid=x0, then flush all
// if vaddr=x0 and asid!=x0 then flush all with matching asid
// if vaddr!=x0 and asid=x0 then flush all leaf PTEs that match vaddr
// if vaddr!=x0 and asid!=x0 then flush the leaf PTE that matches vaddr
// in the given asid.
// No effect if vaddr is not valid
// Currently, we assume if the values of the registers are 0 then it was
// referencing x0.
asid &= 0xFFFF;
if (vpn == 0 && asid == 0)
DPRINTF(TLB, "flush(vaddr=%#x, asid=%#x)\n", vaddr, asid);
if (vaddr == 0 && asid == 0) {
DPRINTF(TLB, "Flushing all TLB entries\n");
flushAll();
else {
DPRINTF(TLB, "flush(vpn=%#x, asid=%#x)\n", vpn, asid);
if (vpn != 0 && asid != 0) {
TlbEntry *newEntry = lookup(vpn, asid, BaseMMU::Read, true);
if (newEntry)
remove(newEntry - tlb.data());
} else {
if (vaddr != 0 && asid != 0) {
// TODO: When supporting other address translation modes, fix this
Addr vpn = getVPNFromVAddr(vaddr, AddrXlateMode::SV39);
TlbEntry *entry = lookup(vpn, asid, BaseMMU::Read, true);
if (entry) {
remove(entry - tlb.data());
}
}
else {
for (size_t i = 0; i < size; i++) {
if (tlb[i].trieHandle) {
Addr mask = ~(tlb[i].size() - 1);
if ((vpn == 0 || (vpn & mask) == tlb[i].vaddr) &&
if ((vaddr == 0 || (vaddr & mask) == tlb[i].vaddr) &&
(asid == 0 || tlb[i].asid == asid))
remove(i);
}
@@ -267,9 +295,10 @@ TLB::createPagefault(Addr vaddr, BaseMMU::Mode mode)
}
Addr
TLB::translateWithTLB(Addr vaddr, uint16_t asid, BaseMMU::Mode mode)
TLB::translateWithTLB(Addr vaddr, uint16_t asid, Addr xmode,
BaseMMU::Mode mode)
{
TlbEntry *e = lookup(vaddr, asid, mode, false);
TlbEntry *e = lookup(getVPNFromVAddr(vaddr, xmode), asid, mode, false);
assert(e != nullptr);
return e->paddr << PageShift | (vaddr & mask(e->logBytes));
}
@@ -284,7 +313,8 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
Addr vaddr = Addr(sext<VADDR_BITS>(req->getVaddr()));
SATP satp = tc->readMiscReg(MISCREG_SATP);
TlbEntry *e = lookup(vaddr, satp.asid, mode, false);
Addr vpn = getVPNFromVAddr(vaddr, satp.mode);
TlbEntry *e = lookup(vpn, satp.asid, mode, false);
if (!e) {
Fault fault = walker->start(tc, translation, req, mode);
if (translation != nullptr || fault != NoFault) {
@@ -292,7 +322,7 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
delayed = true;
return fault;
}
e = lookup(vaddr, satp.asid, mode, true);
e = lookup(vpn, satp.asid, mode, true);
assert(e != nullptr);
}
@@ -315,8 +345,8 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
}
Addr paddr = e->paddr << PageShift | (vaddr & mask(e->logBytes));
DPRINTF(TLBVerbose, "translate(vpn=%#x, asid=%#x): %#x\n",
vaddr, satp.asid, paddr);
DPRINTF(TLBVerbose, "translate(vaddr=%#x, vpn=%#x, asid=%#x): %#x\n",
vaddr, vpn, satp.asid, paddr);
req->setPaddr(paddr);
return NoFault;
@@ -505,9 +535,11 @@ TLB::unserialize(CheckpointIn &cp)
freeList.pop_front();
newEntry->unserializeSection(cp, csprintf("Entry%d", x));
Addr key = buildKey(newEntry->vaddr, newEntry->asid);
// TODO: When supporting other addressing modes fix this
Addr vpn = getVPNFromVAddr(newEntry->vaddr, AddrXlateMode::SV39);
Addr key = buildKey(vpn, newEntry->asid);
newEntry->trieHandle = trie.insert(key,
TlbEntryTrie::MaxBits - newEntry->logBytes, newEntry);
TlbEntryTrie::MaxBits - newEntry->logBytes + PageShift, newEntry);
}
}

22
src/arch/riscv/tlb.hh
View File

@@ -97,6 +97,14 @@ class TLB : public BaseTLB
void takeOverFrom(BaseTLB *old) override {}
/**
* Insert an entry into the TLB.
* @param vpn The virtual page number extracted from the address.
* It is shifted based on the page size. We assume the
* smallest defined page size and remove the upper bits of the
* virtual address that are not part of the page number.
* @param entry The entry to insert.
*/
TlbEntry *insert(Addr vpn, const TlbEntry &entry);
void flushAll() override;
void demapPage(Addr vaddr, uint64_t asn) override;
@@ -123,7 +131,8 @@ class TLB : public BaseTLB
*/
Port *getTableWalkerPort() override;
Addr translateWithTLB(Addr vaddr, uint16_t asid, BaseMMU::Mode mode);
Addr translateWithTLB(Addr vaddr, uint16_t asid, Addr xmode,
BaseMMU::Mode mode);
Fault translateAtomic(const RequestPtr &req,
ThreadContext *tc, BaseMMU::Mode mode) override;
@@ -134,6 +143,17 @@ class TLB : public BaseTLB
BaseMMU::Mode mode) override;
Fault finalizePhysical(const RequestPtr &req, ThreadContext *tc,
BaseMMU::Mode mode) const override;
/**
* Perform the tlb lookup
* @param vpn The virtual page number extracted from the address.
* It is shifted based on the page size. We assume the
* smallest defined page size and remove the upper bits of the
* virtual address that are not part of the page number.
* @param asid The address space identifier as specified by satp.
* @param mode The mode of the memory operation.
* @param hidden If the lookup should be hidden from the statistics.
*/
TlbEntry *lookup(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden);
private: