dev-arm: Improper translation slot release in SMMUv3
authorGiacomo Travaglini <giacomo.travaglini@arm.com>
Thu, 15 Aug 2019 08:54:52 +0000 (09:54 +0100)
committerGiacomo Travaglini <giacomo.travaglini@arm.com>
Thu, 5 Sep 2019 16:37:18 +0000 (16:37 +0000)
The SMMUv3SlaveInterface is using the xlateSlotsRemaining to model a
limit on the number of translation requests it can receive from the
master device.

Patch

https://gem5-review.googlesource.com/c/public/gem5/+/19308/2

moved the resource acquire/release inside the SMMUTranslationProcess
constructor/destructor, for the sake of having a unique place for
calling the signalDrainDone.
While this is convenient, it breaks the original implementation,
which was freeing resources AFTER a translation has completed, but
BEFORE the final memory access (with the translated PA) is performed.
In other words the xlateSlotsRemaining is only modelling translation
slots and should be release once the PA gets produced.

The patch fixes this mismatch by restoring the resource release in
the right place (while keeping the acquire in the constructor)
and by adding a pendingMemAccess counter, which is keeping track
of a complete device memory request (translation + final access)
and will be used by the draining logic

Change-Id: I708fe2d0b6c96ed46f3f4f9a0512f8c1cc43a56c
Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-by: Adrian Herrera <adrian.herrera@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/20260
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>
src/dev/arm/smmu_v3_slaveifc.cc
src/dev/arm/smmu_v3_slaveifc.hh
src/dev/arm/smmu_v3_transl.cc

index fec480da2e4b741f9c15cf24cae94fd81d9ced4d..0ed6c4d4854971f8680ab7f73521ca75464ba526 100644 (file)
@@ -67,6 +67,7 @@ SMMUv3SlaveInterface::SMMUv3SlaveInterface(
     portWidth(p->port_width),
     wrBufSlotsRemaining(p->wrbuf_slots),
     xlateSlotsRemaining(p->xlate_slots),
+    pendingMemAccesses(0),
     prefetchEnable(p->prefetch_enable),
     prefetchReserveLastWay(
         p->prefetch_reserve_last_way),
index 5759a8ffba2fc98df401ac9fb96f0c9822f6fa74..3e03ae49aaa8ae38c4fa243576e8e0848b9210a2 100644 (file)
@@ -83,6 +83,7 @@ class SMMUv3SlaveInterface : public MemObject
 
     unsigned wrBufSlotsRemaining;
     unsigned xlateSlotsRemaining;
+    unsigned pendingMemAccesses;
 
     const bool prefetchEnable;
     const bool prefetchReserveLastWay;
index d7d5768830e64f0a271a6ae7992399c9d633879c..429cc2b44739c58a3ac8352c7ae866b69d209739 100644 (file)
@@ -87,16 +87,20 @@ SMMUTranslationProcess::SMMUTranslationProcess(const std::string &name,
     // Decrease number of pending translation slots on the slave interface
     assert(ifc.xlateSlotsRemaining > 0);
     ifc.xlateSlotsRemaining--;
+
+    ifc.pendingMemAccesses++;
     reinit();
 }
 
 SMMUTranslationProcess::~SMMUTranslationProcess()
 {
     // Increase number of pending translation slots on the slave interface
-    ifc.xlateSlotsRemaining++;
-    // If no more SMMU translations are pending (all slots available),
+    assert(ifc.pendingMemAccesses > 0);
+    ifc.pendingMemAccesses--;
+
+    // If no more SMMU memory accesses are pending,
     // signal SMMU Slave Interface as drained
-    if (ifc.xlateSlotsRemaining == ifc.params()->xlate_slots) {
+    if (ifc.pendingMemAccesses == 0) {
         ifc.signalDrainDone();
     }
 }
@@ -1232,6 +1236,7 @@ SMMUTranslationProcess::completeTransaction(Yield &yield,
 
 
     smmu.translationTimeDist.sample(curTick() - recvTick);
+    ifc.xlateSlotsRemaining++;
     if (!request.isAtsRequest && request.isWrite)
         ifc.wrBufSlotsRemaining +=
             (request.size + (ifc.portWidth-1)) / ifc.portWidth;
@@ -1279,6 +1284,8 @@ SMMUTranslationProcess::completeTransaction(Yield &yield,
 void
 SMMUTranslationProcess::completePrefetch(Yield &yield)
 {
+    ifc.xlateSlotsRemaining++;
+
     SMMUAction a;
     a.type = ACTION_TERMINATE;
     a.pkt = NULL;