Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/7343

Change subject: arch, mem: Make the page table lookup function return a pointer.
......................................................................

arch, mem: Make the page table lookup function return a pointer.

This avoids having a copy in the lookup function itself, and the
declaration of a lot of temporary TLB entry pointers in callers. The
gpu TLB seems to have had the most dependence on the original signature
of the lookup function, partially because it was relying on a somewhat
unsafe copy to a TLB entry using a base class pointer type.

Change-Id: I8b1cf494468163deee000002d243541657faf57f
---
M src/arch/alpha/faults.cc
M src/arch/arm/remote_gdb.cc
M src/arch/mips/remote_gdb.cc
M src/arch/power/remote_gdb.cc
M src/arch/riscv/remote_gdb.cc
M src/arch/sparc/faults.cc
M src/arch/sparc/remote_gdb.cc
M src/arch/x86/remote_gdb.cc
M src/arch/x86/tlb.cc
M src/arch/x86/tlb.hh
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
13 files changed, 183 insertions(+), 226 deletions(-)



diff --git a/src/arch/alpha/faults.cc b/src/arch/alpha/faults.cc
index 4a829cd..89b0ece 100644
--- a/src/arch/alpha/faults.cc
+++ b/src/arch/alpha/faults.cc
@@ -196,14 +196,11 @@
     }

     Process *p = tc->getProcessPtr();
-    TlbEntry entry;
-    bool success = p->pTable->lookup(pc, entry);
-    if (!success) {
-        panic("Tried to execute unmapped address %#x.\n", pc);
-    } else {
-        VAddr vaddr(pc);
-        dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), entry);
-    }
+    TlbEntry *entry = p->pTable->lookup(pc);
+    panic_if(!entry, "Tried to execute unmapped address %#x.\n", pc);
+
+    VAddr vaddr(pc);
+    dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), *entry);
 }

 void
@@ -215,17 +212,11 @@
     }

     Process *p = tc->getProcessPtr();
-    TlbEntry entry;
-    bool success = p->pTable->lookup(vaddr, entry);
-    if (!success) {
-        if (p->fixupStackFault(vaddr))
-            success = p->pTable->lookup(vaddr, entry);
-    }
-    if (!success) {
-        panic("Tried to access unmapped address %#x.\n", (Addr)vaddr);
-    } else {
-        dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), entry);
-    }
+    TlbEntry *entry = p->pTable->lookup(vaddr);
+    if (!entry && p->fixupStackFault(vaddr))
+        entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to access unmapped address %#x.\n", (Addr)vaddr);
+    dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), *entry);
 }

 } // namespace AlphaISA
diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 6dc68b1..333ba70 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -186,12 +186,9 @@
         DPRINTF(GDBAcc, "acc:   %#x mapping is valid\n", va);
         return true;
     } else {
-        TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes address
-        //space.
-        if (context->getProcessPtr()->pTable->lookup(va, entry))
-            return true;
-        return false;
+        // Check to make sure the first byte is mapped into the processes
+        // address space.
+        return context->getProcessPtr()->pTable->lookup(va) != nullptr;
     }
 }

diff --git a/src/arch/mips/remote_gdb.cc b/src/arch/mips/remote_gdb.cc
index 2cc2d77..dc0c27e 100644
--- a/src/arch/mips/remote_gdb.cc
+++ b/src/arch/mips/remote_gdb.cc
@@ -162,13 +162,10 @@
 bool
 RemoteGDB::acc(Addr va, size_t len)
 {
-    TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes address
-    //space.
-    if (FullSystem)
-        panic("acc not implemented for MIPS FS!");
-    else
-        return context->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes address
+    // space.
+    panic_if(FullSystem, "acc not implemented for MIPS FS!");
+    return context->getProcessPtr()->pTable->lookup(va) != nullptr;
 }

 void
diff --git a/src/arch/power/remote_gdb.cc b/src/arch/power/remote_gdb.cc
index c85aa38..30deba7 100644
--- a/src/arch/power/remote_gdb.cc
+++ b/src/arch/power/remote_gdb.cc
@@ -162,16 +162,13 @@
 bool
 RemoteGDB::acc(Addr va, size_t len)
 {
-    TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes address
-    //space.  At the time of this writing, the acc() check is used when
- //processing the MemR/MemW packets before actually asking the translating - //port proxy to read/writeBlob. I (bgs) am not convinced the first byte
-    //check is enough.
-    if (FullSystem)
-        panic("acc not implemented for POWER FS!");
-    else
-        return context->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes address
+    // space.  At the time of this writing, the acc() check is used when
+ // processing the MemR/MemW packets before actually asking the translating + // port proxy to read/writeBlob. I (bgs) am not convinced the first byte
+    // check is enough.
+    panic_if(FullSystem, "acc not implemented for POWER FS!");
+    return context->getProcessPtr()->pTable->lookup(va) != nullptr;
 }

 void
diff --git a/src/arch/riscv/remote_gdb.cc b/src/arch/riscv/remote_gdb.cc
index 3488c81..e3dba4a 100644
--- a/src/arch/riscv/remote_gdb.cc
+++ b/src/arch/riscv/remote_gdb.cc
@@ -156,11 +156,8 @@
 bool
 RemoteGDB::acc(Addr va, size_t len)
 {
-    TlbEntry entry;
-    if (FullSystem)
-        panic("acc not implemented for RISCV FS!");
-    else
-        return context->getProcessPtr()->pTable->lookup(va, entry);
+    panic_if(FullSystem, "acc not implemented for RISCV FS!");
+    return context->getProcessPtr()->pTable->lookup(va) != nullptr;
 }

 void
diff --git a/src/arch/sparc/faults.cc b/src/arch/sparc/faults.cc
index 5466115..0b6a289 100644
--- a/src/arch/sparc/faults.cc
+++ b/src/arch/sparc/faults.cc
@@ -629,49 +629,46 @@
     }

     Process *p = tc->getProcessPtr();
-    TlbEntry entry;
-    bool success = p->pTable->lookup(vaddr, entry);
-    if (!success) {
-        panic("Tried to execute unmapped address %#x.\n", vaddr);
-    } else {
-        Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+    TlbEntry *entry = p->pTable->lookup(vaddr);
+    panic_if(!entry, "Tried to execute unmapped address %#x.\n", vaddr);

-        // Grab fields used during instruction translation to figure out
-        // which context to use.
-        uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+    Addr alignedvaddr = p->pTable->pageAlign(vaddr);

-        // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical address,
-        // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the context is
-        // zero (kernel-level) if real addressing is being used.
-        bool is_real_address = !bits(tlbdata, 4);
+    // Grab fields used during instruction translation to figure out
+    // which context to use.
+    uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

-        // The SPARC ITLB code assumes that traps are executed in context
-        // zero so we carry that assumption through here.
-        bool trapped = bits(tlbdata, 18, 16) > 0;
+    // Inside a VM, a real address is the address that guest OS would
+    // interpret to be a physical address. To map to the physical address,
+    // it still needs to undergo a translation. The instruction
+    // translation code in the SPARC ITLB code assumes that the context is
+    // zero (kernel-level) if real addressing is being used.
+    bool is_real_address = !bits(tlbdata, 4);

-        // The primary context acts as a PASID. It allows the MMU to
-        // distinguish between virtual addresses that would alias to the
-        // same physical address (if two or more processes shared the same
-        // virtual address mapping).
-        int primary_context = bits(tlbdata, 47, 32);
+    // The SPARC ITLB code assumes that traps are executed in context
+    // zero so we carry that assumption through here.
+    bool trapped = bits(tlbdata, 18, 16) > 0;

-        // The partition id distinguishes between virtualized environments.
-        int const partition_id = 0;
+    // The primary context acts as a PASID. It allows the MMU to
+    // distinguish between virtual addresses that would alias to the
+    // same physical address (if two or more processes shared the same
+    // virtual address mapping).
+    int primary_context = bits(tlbdata, 47, 32);

- // Given the assumptions in the translateInst code in the SPARC ITLB,
-        // the logic works out to the following for the context.
- int context_id = (is_real_address || trapped) ? 0 : primary_context;
+    // The partition id distinguishes between virtualized environments.
+    int const partition_id = 0;

-        // Insert the TLB entry.
-        // The entry specifying whether the address is "real" is set to
-        // false for syscall emulation mode regardless of whether the
-        // address is real in preceding code. Not sure sure that this is
-        // correct, but also not sure if it matters at all.
-        dynamic_cast<TLB *>(tc->getITBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry.pte);
-    }
+    // Given the assumptions in the translateInst code in the SPARC ITLB,
+    // the logic works out to the following for the context.
+    int context_id = (is_real_address || trapped) ? 0 : primary_context;
+
+    // Insert the TLB entry.
+    // The entry specifying whether the address is "real" is set to
+    // false for syscall emulation mode regardless of whether the
+    // address is real in preceding code. Not sure sure that this is
+    // correct, but also not sure if it matters at all.
+    dynamic_cast<TLB *>(tc->getITBPtr())->
+        insert(alignedvaddr, partition_id, context_id, false, entry->pte);
 }

 void
@@ -683,83 +680,78 @@
     }

     Process *p = tc->getProcessPtr();
-    TlbEntry entry;
-    bool success = p->pTable->lookup(vaddr, entry);
-    if (!success) {
-        if (p->fixupStackFault(vaddr))
-            success = p->pTable->lookup(vaddr, entry);
-    }
-    if (!success) {
-        panic("Tried to access unmapped address %#x.\n", vaddr);
-    } else {
-        Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+    TlbEntry *entry = p->pTable->lookup(vaddr);
+    if (!entry && p->fixupStackFault(vaddr))
+        entry = p->pTable->lookup(vaddr);
+    panic_if(!entry, "Tried to access unmapped address %#x.\n", vaddr);

-        // Grab fields used during data translation to figure out
-        // which context to use.
-        uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+    Addr alignedvaddr = p->pTable->pageAlign(vaddr);

-        // The primary context acts as a PASID. It allows the MMU to
-        // distinguish between virtual addresses that would alias to the
-        // same physical address (if two or more processes shared the same
- // virtual address mapping). There's a secondary context used in the
-        // DTLB translation code, but it should __probably__ be zero for
- // syscall emulation code. (The secondary context is used by Solaris
-        // to allow kernel privilege code to access user space code:
-        // [ISBN 0-13-022496-0]:PG199.)
-        int primary_context = bits(tlbdata, 47, 32);
+    // Grab fields used during data translation to figure out
+    // which context to use.
+    uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

-        // "Hyper-Privileged Mode" is in use. There are three main modes of
-        // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
-        // User Mode.
-        int hpriv = bits(tlbdata, 0);
+    // The primary context acts as a PASID. It allows the MMU to
+    // distinguish between virtual addresses that would alias to the
+    // same physical address (if two or more processes shared the same
+    // virtual address mapping). There's a secondary context used in the
+    // DTLB translation code, but it should __probably__ be zero for
+    // syscall emulation code. (The secondary context is used by Solaris
+    // to allow kernel privilege code to access user space code:
+    // [ISBN 0-13-022496-0]:PG199.)
+    int primary_context = bits(tlbdata, 47, 32);

-        // Reset, Error and Debug state is in use. Something horrible has
-        // happened or the system is operating in Reset Mode.
-        int red = bits(tlbdata, 1);
+    // "Hyper-Privileged Mode" is in use. There are three main modes of
+    // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
+    // User Mode.
+    int hpriv = bits(tlbdata, 0);

-        // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical address,
-        // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the context is
-        // zero (kernel-level) if real addressing is being used.
-        int is_real_address = !bits(tlbdata, 5);
+    // Reset, Error and Debug state is in use. Something horrible has
+    // happened or the system is operating in Reset Mode.
+    int red = bits(tlbdata, 1);

- // Grab the address space identifier register from the thread context.
-        // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
- // MISCREG_ASI causes me to think that the ASI register implementation
-        // might be bugged. The NoEffect variant changes the ASI register
- // value in the architectural state while the normal variant changes - // the context field in the thread context's currently decoded request
-        // but does not directly affect the ASI register value in the
-        // architectural state. The ASI values and the context field in the
-        // request packet seem to have completely different uses.
-        MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
-        ASI asi = static_cast<ASI>(reg_asi);
+    // Inside a VM, a real address is the address that guest OS would
+    // interpret to be a physical address. To map to the physical address,
+    // it still needs to undergo a translation. The instruction
+    // translation code in the SPARC ITLB code assumes that the context is
+    // zero (kernel-level) if real addressing is being used.
+    int is_real_address = !bits(tlbdata, 5);

-        // The SPARC DTLB code assumes that traps are executed in context
- // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's also
-        // an assumption that the nucleus address space is being used, but
- // the context is the relevant issue since we need to pass it to TLB.
-        bool trapped = bits(tlbdata, 18, 16) > 0;
+    // Grab the address space identifier register from the thread context.
+    // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
+    // MISCREG_ASI causes me to think that the ASI register implementation
+    // might be bugged. The NoEffect variant changes the ASI register
+    // value in the architectural state while the normal variant changes
+    // the context field in the thread context's currently decoded request
+    // but does not directly affect the ASI register value in the
+    // architectural state. The ASI values and the context field in the
+    // request packet seem to have completely different uses.
+    MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
+    ASI asi = static_cast<ASI>(reg_asi);

- // Given the assumptions in the translateData code in the SPARC DTLB,
-        // the logic works out to the following for the context.
-        int context_id = ((!hpriv && !red && is_real_address) ||
-                          asiIsReal(asi) ||
-                          (trapped && asi == ASI_IMPLICIT))
-                         ? 0 : primary_context;
+    // The SPARC DTLB code assumes that traps are executed in context
+    // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's also
+    // an assumption that the nucleus address space is being used, but
+    // the context is the relevant issue since we need to pass it to TLB.
+    bool trapped = bits(tlbdata, 18, 16) > 0;

-        // The partition id distinguishes between virtualized environments.
-        int const partition_id = 0;
+    // Given the assumptions in the translateData code in the SPARC DTLB,
+    // the logic works out to the following for the context.
+    int context_id = ((!hpriv && !red && is_real_address) ||
+                      asiIsReal(asi) ||
+                      (trapped && asi == ASI_IMPLICIT))
+                     ? 0 : primary_context;

-        // Insert the TLB entry.
-        // The entry specifying whether the address is "real" is set to
-        // false for syscall emulation mode regardless of whether the
-        // address is real in preceding code. Not sure sure that this is
-        // correct, but also not sure if it matters at all.
-        dynamic_cast<TLB *>(tc->getDTBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry.pte);
-    }
+    // The partition id distinguishes between virtualized environments.
+    int const partition_id = 0;
+
+    // Insert the TLB entry.
+    // The entry specifying whether the address is "real" is set to
+    // false for syscall emulation mode regardless of whether the
+    // address is real in preceding code. Not sure sure that this is
+    // correct, but also not sure if it matters at all.
+    dynamic_cast<TLB *>(tc->getDTBPtr())->
+        insert(alignedvaddr, partition_id, context_id, false, entry->pte);
 }

 void
diff --git a/src/arch/sparc/remote_gdb.cc b/src/arch/sparc/remote_gdb.cc
index 3f4df0d..778bb24 100644
--- a/src/arch/sparc/remote_gdb.cc
+++ b/src/arch/sparc/remote_gdb.cc
@@ -163,16 +163,11 @@
     // from va to va + len have valid page map entries. Not
     // sure how this will work for other OSes or in general.
     if (FullSystem) {
-        if (va)
-            return true;
-        return false;
+        return va != 0;
     } else {
-        TlbEntry entry;
         // Check to make sure the first byte is mapped into the processes
         // address space.
-        if (context->getProcessPtr()->pTable->lookup(va, entry))
-            return true;
-        return false;
+        return context->getProcessPtr()->pTable->lookup(va) != nullptr;
     }
 }

diff --git a/src/arch/x86/remote_gdb.cc b/src/arch/x86/remote_gdb.cc
index a6fdabd..45265d1 100644
--- a/src/arch/x86/remote_gdb.cc
+++ b/src/arch/x86/remote_gdb.cc
@@ -88,8 +88,7 @@
                                         BaseTLB::Read);
         return fault == NoFault;
     } else {
-        TlbEntry entry;
-        return context->getProcessPtr()->pTable->lookup(va, entry);
+        return context->getProcessPtr()->pTable->lookup(va) != nullptr;
     }
 }

diff --git a/src/arch/x86/tlb.cc b/src/arch/x86/tlb.cc
index 0b1df9e..9336949 100644
--- a/src/arch/x86/tlb.cc
+++ b/src/arch/x86/tlb.cc
@@ -94,7 +94,7 @@
 }

 TlbEntry *
-TLB::insert(Addr vpn, TlbEntry &entry)
+TLB::insert(Addr vpn, const TlbEntry &entry)
 {
     // If somebody beat us to it, just use that existing entry.
     TlbEntry *newEntry = trie.lookup(vpn);
@@ -357,23 +357,22 @@
                     assert(entry);
                 } else {
                     Process *p = tc->getProcessPtr();
-                    TlbEntry newEntry;
-                    bool success = p->pTable->lookup(vaddr, newEntry);
-                    if (!success && mode != Execute) {
+                    TlbEntry *newEntry = p->pTable->lookup(vaddr);
+                    if (!newEntry && mode != Execute) {
                         // Check if we just need to grow the stack.
                         if (p->fixupStackFault(vaddr)) {
// If we did, lookup the entry for the new page.
-                            success = p->pTable->lookup(vaddr, newEntry);
+                            newEntry = p->pTable->lookup(vaddr);
                         }
                     }
-                    if (!success) {
+                    if (!newEntry) {
return std::make_shared<PageFault>(vaddr, true, mode,
                                                            true, false);
                     } else {
                         Addr alignedVaddr = p->pTable->pageAlign(vaddr);
                         DPRINTF(TLB, "Mapping %#x to %#x\n", alignedVaddr,
-                                newEntry.pageStart());
-                        entry = insert(alignedVaddr, newEntry);
+                                newEntry->pageStart());
+                        entry = insert(alignedVaddr, *newEntry);
                     }
                     DPRINTF(TLB, "Miss was serviced.\n");
                 }
diff --git a/src/arch/x86/tlb.hh b/src/arch/x86/tlb.hh
index c3dc83b..08804a4 100644
--- a/src/arch/x86/tlb.hh
+++ b/src/arch/x86/tlb.hh
@@ -144,7 +144,7 @@
         Fault finalizePhysical(RequestPtr req, ThreadContext *tc,
                                Mode mode) const override;

-        TlbEntry * insert(Addr vpn, TlbEntry &entry);
+        TlbEntry *insert(Addr vpn, const TlbEntry &entry);

         /*
          * Function to register Stats
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index b5411f8..9cbf3e8 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -808,31 +808,31 @@
                                 "at pc %#x.\n", vaddr, tc->instAddr());

                         Process *p = tc->getProcessPtr();
-                        GpuTlbEntry newEntry;
-                        bool success = p->pTable->lookup(vaddr, newEntry);
+                        TlbEntry *newEntry = p->pTable->lookup(vaddr);

-                        if (!success && mode != BaseTLB::Execute) {
+                        if (!newEntry && mode != BaseTLB::Execute) {
                             // penalize a "page fault" more
-                            if (timing) {
+                            if (timing)
                                 latency += missLatency2;
-                            }

                             if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, newEntry);
+                                newEntry = p->pTable->lookup(vaddr);
                         }

-                        if (!success) {
+                        if (!newEntry) {
                             return std::make_shared<PageFault>(vaddr, true,
                                                                mode, true,
                                                                false);
                         } else {
-                            newEntry.valid = success;
Addr alignedVaddr = p->pTable->pageAlign(vaddr);

                             DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
-                                    alignedVaddr, newEntry.pageStart());
+                                    alignedVaddr, newEntry->pageStart());

-                            entry = insert(alignedVaddr, newEntry);
+                            GpuTlbEntry gpuEntry;
+                            *(TlbEntry *)&gpuEntry = *newEntry;
+                            gpuEntry.valid = true;
+                            entry = insert(alignedVaddr, gpuEntry);
                         }

                         DPRINTF(GPUTLB, "Miss was serviced.\n");
@@ -1330,25 +1330,27 @@
                 safe_cast<TranslationState*>(pkt->senderState);

             Process *p = sender_state->tc->getProcessPtr();
-            TlbEntry newEntry;
             Addr vaddr = pkt->req->getVaddr();
     #ifndef NDEBUG
             Addr alignedVaddr = p->pTable->pageAlign(vaddr);
             assert(alignedVaddr == virtPageAddr);
     #endif
-            bool success;
-            success = p->pTable->lookup(vaddr, newEntry);
-            if (!success && sender_state->tlbMode != BaseTLB::Execute) {
-                if (p->fixupStackFault(vaddr)) {
-                    success = p->pTable->lookup(vaddr, newEntry);
-                }
+            TlbEntry *newEntry = p->pTable->lookup(vaddr);
+            if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+                    p->fixupStackFault(vaddr)) {
+                newEntry = p->pTable->lookup(vaddr);
             }

-            DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
-                    newEntry.pageStart());
+            if (newEntry) {
+                DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
+                        newEntry->pageStart());

-            sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry.vaddr, newEntry.paddr, success);
+                sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr, newEntry->paddr, true);
+            } else {
+                sender_state->tlbEntry =
+                    new GpuTlbEntry(0, 0, 0, false);
+            }

             handleTranslationReturn(virtPageAddr, TLB_MISS, pkt);
         } else if (outcome == MISS_RETURN) {
@@ -1524,7 +1526,6 @@
                         virt_page_addr);

                 Process *p = tc->getProcessPtr();
-                TlbEntry newEntry;

                 Addr vaddr = pkt->req->getVaddr();
     #ifndef NDEBUG
@@ -1532,10 +1533,10 @@
                 assert(alignedVaddr == virt_page_addr);
     #endif

-                bool success = p->pTable->lookup(vaddr, newEntry);
- if (!success && sender_state->tlbMode != BaseTLB::Execute) {
-                    if (p->fixupStackFault(vaddr))
-                        success = p->pTable->lookup(vaddr, newEntry);
+                TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+                        p->fixupStackFault(vaddr)) {
+                    newEntry = p->pTable->lookup(vaddr);
                 }

                 if (!sender_state->prefetch) {
@@ -1544,24 +1545,23 @@
                     assert(success);

                     DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
-                           newEntry.pageStart());
+                           newEntry->pageStart());

- sender_state->tlbEntry = new GpuTlbEntry(0, newEntry.vaddr, - newEntry.paddr,
-                                                             success);
+                    sender_state->tlbEntry =
+                        new GpuTlbEntry(0, newEntry->vaddr,
+                                        newEntry->paddr, success);
                 } else {
// If this was a prefetch, then do the normal thing if it // was a successful translation. Otherwise, send an empty // TLB entry back so that it can be figured out as empty and
                     // handled accordingly.
-                    if (success) {
+                    if (newEntry) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
-                               newEntry.pageStart());
+                               newEntry->pageStart());

-                        sender_state->tlbEntry = new GpuTlbEntry(0,
- newEntry.vaddr, - newEntry.paddr,
-                                                                 success);
+                        sender_state->tlbEntry =
+                            new GpuTlbEntry(0, newEntry->vaddr,
+                                            newEntry->paddr, success);
                     } else {
                         DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
                                 alignedVaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index 4d62b80..3031c2a 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -151,43 +151,36 @@
     return true;
 }

-bool
-EmulationPageTable::lookup(Addr vaddr, TheISA::TlbEntry &entry)
+TheISA::TlbEntry *
+EmulationPageTable::lookup(Addr vaddr)
 {
     Addr page_addr = pageAlign(vaddr);

-    if (pTableCache[0].entry && pTableCache[0].vaddr == page_addr) {
-        entry = *pTableCache[0].entry;
-        return true;
-    }
-    if (pTableCache[1].entry && pTableCache[1].vaddr == page_addr) {
-        entry = *pTableCache[1].entry;
-        return true;
-    }
-    if (pTableCache[2].entry && pTableCache[2].vaddr == page_addr) {
-        entry = *pTableCache[2].entry;
-        return true;
-    }
+    if (pTableCache[0].entry && pTableCache[0].vaddr == page_addr)
+        return pTableCache[0].entry;
+    if (pTableCache[1].entry && pTableCache[1].vaddr == page_addr)
+        return pTableCache[1].entry;
+    if (pTableCache[2].entry && pTableCache[2].vaddr == page_addr)
+        return pTableCache[2].entry;

     PTableItr iter = pTable.find(page_addr);

     if (iter == pTable.end())
-        return false;
+        return nullptr;

     updateCache(page_addr, iter->second);
-    entry = *iter->second;
-    return true;
+    return iter->second;
 }

 bool
 EmulationPageTable::translate(Addr vaddr, Addr &paddr)
 {
-    TheISA::TlbEntry entry;
-    if (!lookup(vaddr, entry)) {
+    TheISA::TlbEntry *entry = lookup(vaddr);
+    if (!entry) {
         DPRINTF(MMU, "Couldn't Translate: %#x\n", vaddr);
         return false;
     }
-    paddr = pageOffset(vaddr) + entry.pageStart();
+    paddr = pageOffset(vaddr) + entry->pageStart();
     DPRINTF(MMU, "Translating: %#x->%#x\n", vaddr, paddr);
     return true;
 }
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index de16740..cf3c378 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -164,9 +164,9 @@
     /**
      * Lookup function
      * @param vaddr The virtual address.
-     * @return entry The page table entry corresponding to vaddr.
+     * @return The page table entry corresponding to vaddr.
      */
-    virtual bool lookup(Addr vaddr, TheISA::TlbEntry &entry);
+    virtual TheISA::TlbEntry *lookup(Addr vaddr);

     /**
      * Translate function

--
To view, visit https://gem5-review.googlesource.com/7343
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b1cf494468163deee000002d243541657faf57f
Gerrit-Change-Number: 7343
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to