gpu-compute: use X86ISA::TlbEntry over GpuTlbEntry

GpuTlbEntry was derived from a vanilla X86ISA::TlbEntry definition. It
wrapped the class and included an extra member "valid". This member was
intended to report on the validity of the entry, however it introduced
bugs when folks forgot to set field properly in the code. So, instead of
keeping the extra field which we might forget to set, we track validity by
using nullptr for invalid tlb entries (as the tlb entries are dynamically
allocated). This saves on the extra class definition and prevents bugs
creeping into the code since the checks are intrinsically tied into
accessing any of the X86ISA::TlbEntry members.

This changeset fixes the issues introduced by a8d030522, a4e722725, and
2a15bfd79.

Change-Id: I30ebe3ec223fb833f3795bf0403d0016ac9a8bc2
Reviewed-on: https://gem5-review.googlesource.com/10481
Reviewed-by: Brandon Potter <Brandon.Potter@amd.com>
Maintainer: Anthony Gutierrez <anthony.gutierrez@amd.com>

1 files changed, 41 insertions, 33 deletions

diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index 5691f3556..8b9bd4320 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -73,7 +73,7 @@ namespace X86ISA
         accessDistance = p->accessDistance;
         clock = p->clk_domain->clockPeriod();
 
-        tlb.assign(size, GpuTlbEntry());
+        tlb.assign(size, TlbEntry());
 
         freeList.resize(numSets);
         entryList.resize(numSets);
@@ -166,10 +166,10 @@ namespace X86ISA
         }
     }
 
-    GpuTlbEntry*
-    GpuTLB::insert(Addr vpn, GpuTlbEntry &entry)
+    TlbEntry*
+    GpuTLB::insert(Addr vpn, TlbEntry &entry)
     {
-        GpuTlbEntry *newEntry = nullptr;
+        TlbEntry *newEntry = nullptr;
 
         /**
          * vpn holds the virtual page address
@@ -222,7 +222,7 @@ namespace X86ISA
         return entry;
     }
 
-    GpuTlbEntry*
+    TlbEntry*
     GpuTLB::lookup(Addr va, bool update_lru)
     {
         int set = (va >> TheISA::PageShift) & setMask;
@@ -242,7 +242,7 @@ namespace X86ISA
 
         for (int i = 0; i < numSets; ++i) {
             while (!entryList[i].empty()) {
-                GpuTlbEntry *entry = entryList[i].front();
+                TlbEntry *entry = entryList[i].front();
                 entryList[i].pop_front();
                 freeList[i].push_back(entry);
             }
@@ -684,7 +684,7 @@ namespace X86ISA
             if (m5Reg.paging) {
                 DPRINTF(GPUTLB, "Paging enabled.\n");
                 //update LRU stack on a hit
-                GpuTlbEntry *entry = lookup(vaddr, true);
+                TlbEntry *entry = lookup(vaddr, true);
 
                 if (entry)
                     tlb_hit = true;
@@ -792,7 +792,7 @@ namespace X86ISA
             if (m5Reg.paging) {
                 DPRINTF(GPUTLB, "Paging enabled.\n");
                 // The vaddr already has the segment base applied.
-                GpuTlbEntry *entry = lookup(vaddr);
+                TlbEntry *entry = lookup(vaddr);
                 localNumTLBAccesses++;
 
                 if (!entry) {
@@ -830,9 +830,8 @@ namespace X86ISA
                             DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
                                     alignedVaddr, pte->paddr);
 
-                            GpuTlbEntry gpuEntry(
-                                p->pTable->pid(), alignedVaddr,
-                                pte->paddr, true);
+                            TlbEntry gpuEntry(p->pid(), alignedVaddr,
+                                              pte->paddr, false, false);
                             entry = insert(alignedVaddr, gpuEntry);
                         }
 
@@ -1078,11 +1077,13 @@ namespace X86ISA
         if (success) {
             lookup_outcome = TLB_HIT;
             // Put the entry in SenderState
-            GpuTlbEntry *entry = lookup(tmp_req->getVaddr(), false);
+            TlbEntry *entry = lookup(tmp_req->getVaddr(), false);
             assert(entry);
 
+            auto p = sender_state->tc->getProcessPtr();
             sender_state->tlbEntry =
-                new GpuTlbEntry(0, entry->vaddr, entry->paddr, entry->valid);
+                new TlbEntry(p->pid(), entry->vaddr, entry->paddr,
+                             false, false);
 
             if (update_stats) {
                 // the reqCnt has an entry per level, so its size tells us
@@ -1134,7 +1135,7 @@ namespace X86ISA
      */
     void
     GpuTLB::pagingProtectionChecks(ThreadContext *tc, PacketPtr pkt,
-            GpuTlbEntry * tlb_entry, Mode mode)
+            TlbEntry * tlb_entry, Mode mode)
     {
         HandyM5Reg m5Reg = tc->readMiscRegNoEffect(MISCREG_M5_REG);
         uint32_t flags = pkt->req->getFlags();
@@ -1180,7 +1181,7 @@ namespace X86ISA
         ThreadContext *tc = sender_state->tc;
         Mode mode = sender_state->tlbMode;
 
-        GpuTlbEntry *local_entry, *new_entry;
+        TlbEntry *local_entry, *new_entry;
 
         if (tlb_outcome == TLB_HIT) {
             DPRINTF(GPUTLB, "Translation Done - TLB Hit for addr %#x\n", vaddr);
@@ -1347,10 +1348,10 @@ namespace X86ISA
                         pte->paddr);
 
                 sender_state->tlbEntry =
-                    new GpuTlbEntry(0, virtPageAddr, pte->paddr, true);
+                    new TlbEntry(p->pid(), virtPageAddr, pte->paddr, false,
+                                 false);
             } else {
-                sender_state->tlbEntry =
-                    new GpuTlbEntry(0, 0, 0, false);
+                sender_state->tlbEntry = nullptr;
             }
 
             handleTranslationReturn(virtPageAddr, TLB_MISS, pkt);
@@ -1427,7 +1428,7 @@ namespace X86ISA
         Mode mode = sender_state->tlbMode;
         Addr vaddr = pkt->req->getVaddr();
 
-        GpuTlbEntry *local_entry, *new_entry;
+        TlbEntry *local_entry, *new_entry;
 
         if (tlb_outcome == TLB_HIT) {
             DPRINTF(GPUTLB, "Functional Translation Done - TLB hit for addr "
@@ -1461,13 +1462,18 @@ namespace X86ISA
                 "while paddr was %#x.\n", local_entry->vaddr,
                 local_entry->paddr);
 
-        // Do paging checks if it's a normal functional access.  If it's for a
-        // prefetch, then sometimes you can try to prefetch something that won't
-        // pass protection. We don't actually want to fault becuase there is no
-        // demand access to deem this a violation.  Just put it in the TLB and
-        // it will fault if indeed a future demand access touches it in
-        // violation.
-        if (!sender_state->prefetch && sender_state->tlbEntry->valid)
+        /**
+         * Do paging checks if it's a normal functional access.  If it's for a
+         * prefetch, then sometimes you can try to prefetch something that
+         * won't pass protection. We don't actually want to fault becuase there
+         * is no demand access to deem this a violation.  Just put it in the
+         * TLB and it will fault if indeed a future demand access touches it in
+         * violation.
+         *
+         * This feature could be used to explore security issues around
+         * speculative memory accesses.
+         */
+        if (!sender_state->prefetch && sender_state->tlbEntry)
             pagingProtectionChecks(tc, pkt, local_entry, mode);
 
         int page_size = local_entry->size();
@@ -1550,8 +1556,8 @@ namespace X86ISA
                             pte->paddr);
 
                     sender_state->tlbEntry =
-                        new GpuTlbEntry(0, virt_page_addr,
-                                        pte->paddr, true);
+                        new TlbEntry(p->pid(), virt_page_addr,
+                                     pte->paddr, false, false);
                 } else {
                     // If this was a prefetch, then do the normal thing if it
                     // was a successful translation.  Otherwise, send an empty
@@ -1562,13 +1568,13 @@ namespace X86ISA
                                 pte->paddr);
 
                         sender_state->tlbEntry =
-                            new GpuTlbEntry(0, virt_page_addr,
-                                            pte->paddr, true);
+                            new TlbEntry(p->pid(), virt_page_addr,
+                                         pte->paddr, false, false);
                     } else {
                         DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
                                 alignedVaddr);
 
-                        sender_state->tlbEntry = new GpuTlbEntry();
+                        sender_state->tlbEntry = nullptr;
 
                         return;
                     }
@@ -1578,13 +1584,15 @@ namespace X86ISA
             DPRINTF(GPUPrefetch, "Functional Hit for vaddr %#x\n",
                     tlb->lookup(pkt->req->getVaddr()));
 
-            GpuTlbEntry *entry = tlb->lookup(pkt->req->getVaddr(),
+            TlbEntry *entry = tlb->lookup(pkt->req->getVaddr(),
                                              update_stats);
 
             assert(entry);
 
+            auto p = sender_state->tc->getProcessPtr();
             sender_state->tlbEntry =
-                new GpuTlbEntry(0, entry->vaddr, entry->paddr, entry->valid);
+                new TlbEntry(p->pid(), entry->vaddr, entry->paddr,
+                             false, false);
         }
         // This is the function that would populate pkt->req with the paddr of
         // the translation. But if no translation happens (i.e Prefetch fails)