arm: fix a page table walker issue where a page could be translated multiple times
authorMrinmoy Ghosh <mrinmoy.ghosh@arm.com>
Fri, 15 Feb 2013 22:40:10 +0000 (17:40 -0500)
committerMrinmoy Ghosh <mrinmoy.ghosh@arm.com>
Fri, 15 Feb 2013 22:40:10 +0000 (17:40 -0500)
If multiple memory operations to the same page are miss the TLB they are
all inserted into the page table queue and before this change could result
in multiple uncessesary walks as well as duplicate enteries being inserted
into the TLB.

src/arch/arm/table_walker.cc
src/arch/arm/tlb.cc
src/arch/arm/tlb.hh

index 44f12833bd26e3787d6106cc56d7fb3f8da99217..9755299ffa0ba7c8a856453a740c37f1292acf9f 100644 (file)
@@ -186,8 +186,15 @@ TableWalker::processWalkWrapper()
     assert(pendingQueue.size());
     currState = pendingQueue.front();
 
-
-    if (!currState->transState->squashed()) {
+    // Check if a previous walk filled this request already
+    TlbEntry* te = tlb->lookup(currState->vaddr, currState->contextId, true);
+
+    // Check if we still need to have a walk for this request. If the requesting
+    // instruction has been squashed, or a previous walk has filled the TLB with
+    // a match, we just want to get rid of the walk. The latter could happen
+    // when there are multiple outstanding misses to a single page and a
+    // previous request has been successfully translated.
+    if (!currState->transState->squashed() && !te) {
         // We've got a valid request, lets process it
         pending = true;
         pendingQueue.pop_front();
@@ -200,26 +207,34 @@ TableWalker::processWalkWrapper()
     // squashed we shouldn't bother.
     unsigned num_squashed = 0;
     ThreadContext *tc = currState->tc;
-    assert(currState->transState->squashed());
     while ((num_squashed < numSquashable) && currState &&
-            currState->transState->squashed()) {
+           (currState->transState->squashed() || te)) {
         pendingQueue.pop_front();
         num_squashed++;
 
         DPRINTF(TLB, "Squashing table walk for address %#x\n", currState->vaddr);
 
-        // finish the translation which will delete the translation object
-        currState->transState->finish(new UnimpFault("Squashed Inst"),
-                currState->req, currState->tc, currState->mode);
+        if (currState->transState->squashed()) {
+            // finish the translation which will delete the translation object
+            currState->transState->finish(new UnimpFault("Squashed Inst"),
+                    currState->req, currState->tc, currState->mode);
+        } else {
+            // translate the request now that we know it will work
+            currState->fault = tlb->translateTiming(currState->req, currState->tc,
+                                      currState->transState, currState->mode);
+        }
 
         // delete the current request
         delete currState;
 
         // peak at the next one
-        if (pendingQueue.size())
+        if (pendingQueue.size()) {
             currState = pendingQueue.front();
-        else
+            te = tlb->lookup(currState->vaddr, currState->contextId, true);
+        } else {
+            // Terminate the loop, nothing more to do
             currState = NULL;
+        }
     }
 
     // if we've still got pending translations schedule more work
index 170d819d8a4e154f2f083c280930e10be2071fe3..6b864b980a00f8559e3853361f719d9330c793f6 100644 (file)
@@ -107,7 +107,7 @@ TLB::lookup(Addr va, uint8_t cid, bool functional)
         if (table[x].match(va, cid)) {
 
             // We only move the hit entry ahead when the position is higher than rangeMRU
-            if (x > rangeMRU) {
+            if (x > rangeMRU && !functional) {
                 TlbEntry tmp_entry = table[x];
                 for(int i = x; i > 0; i--)
                     table[i] = table[i-1];
index f5c7320ed322a02d8979025907008a7749f42c9d..c1eba1ba780ba7acddb624f0dd49218b2e621383 100644 (file)
@@ -91,14 +91,6 @@ class TLB : public BaseTLB
 
     TableWalker *tableWalker;
 
-    /** Lookup an entry in the TLB
-     * @param vpn virtual address
-     * @param asn context id/address space id to use
-     * @param functional if the lookup should modify state
-     * @return pointer to TLB entrry if it exists
-     */
-    TlbEntry *lookup(Addr vpn, uint8_t asn, bool functional = false);
-
     // Access Stats
     mutable Stats::Scalar instHits;
     mutable Stats::Scalar instMisses;
@@ -132,6 +124,14 @@ class TLB : public BaseTLB
     typedef ArmTLBParams Params;
     TLB(const Params *p);
 
+    /** Lookup an entry in the TLB
+     * @param vpn virtual address
+     * @param asn context id/address space id to use
+     * @param functional if the lookup should modify state
+     * @return pointer to TLB entrry if it exists
+     */
+    TlbEntry *lookup(Addr vpn, uint8_t asn, bool functional = false);
+
     virtual ~TLB();
     int getsize() const { return size; }