From 4f728b7d6cde5ba07956c8699c6b6e9f68769ec5 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Thu, 15 Aug 2019 09:54:52 +0100 Subject: [PATCH] dev-arm: Improper translation slot release in SMMUv3 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 Reviewed-by: Adrian Herrera Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/20260 Reviewed-by: Andreas Sandberg Maintainer: Andreas Sandberg Tested-by: kokoro --- src/dev/arm/smmu_v3_slaveifc.cc | 1 + src/dev/arm/smmu_v3_slaveifc.hh | 1 + src/dev/arm/smmu_v3_transl.cc | 13 ++++++++++--- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/dev/arm/smmu_v3_slaveifc.cc b/src/dev/arm/smmu_v3_slaveifc.cc index fec480da2..0ed6c4d48 100644 --- a/src/dev/arm/smmu_v3_slaveifc.cc +++ b/src/dev/arm/smmu_v3_slaveifc.cc @@ -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), diff --git a/src/dev/arm/smmu_v3_slaveifc.hh b/src/dev/arm/smmu_v3_slaveifc.hh index 5759a8ffb..3e03ae49a 100644 --- a/src/dev/arm/smmu_v3_slaveifc.hh +++ b/src/dev/arm/smmu_v3_slaveifc.hh @@ -83,6 +83,7 @@ class SMMUv3SlaveInterface : public MemObject unsigned wrBufSlotsRemaining; unsigned xlateSlotsRemaining; + unsigned pendingMemAccesses; const bool prefetchEnable; const bool prefetchReserveLastWay; diff --git a/src/dev/arm/smmu_v3_transl.cc b/src/dev/arm/smmu_v3_transl.cc index d7d576883..429cc2b44 100644 --- a/src/dev/arm/smmu_v3_transl.cc +++ b/src/dev/arm/smmu_v3_transl.cc @@ -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; -- 2.30.2