Skip to content

Commit 3cf7423

Browse files
skuma24joergroedel
authored andcommitted
iommu/vt-d: Fix potential lockup if qi_submit_sync called with 0 count
If qi_submit_sync() is invoked with 0 invalidation descriptors (for instance, for DMA draining purposes), we can run into a bug where a submitting thread fails to detect the completion of invalidation_wait. Subsequently, this led to a soft lockup. Currently, there is no impact by this bug on the existing users because no callers are submitting invalidations with 0 descriptors. This fix will enable future users (such as DMA drain) calling qi_submit_sync() with 0 count. Suppose thread T1 invokes qi_submit_sync() with non-zero descriptors, while concurrently, thread T2 calls qi_submit_sync() with zero descriptors. Both threads then enter a while loop, waiting for their respective descriptors to complete. T1 detects its completion (i.e., T1's invalidation_wait status changes to QI_DONE by HW) and proceeds to call reclaim_free_desc() to reclaim all descriptors, potentially including adjacent ones of other threads that are also marked as QI_DONE. During this time, while T2 is waiting to acquire the qi->q_lock, the IOMMU hardware may complete the invalidation for T2, setting its status to QI_DONE. However, if T1's execution of reclaim_free_desc() frees T2's invalidation_wait descriptor and changes its status to QI_FREE, T2 will not observe the QI_DONE status for its invalidation_wait and will indefinitely remain stuck. This soft lockup does not occur when only non-zero descriptors are submitted.In such cases, invalidation descriptors are interspersed among wait descriptors with the status QI_IN_USE, acting as barriers. These barriers prevent the reclaim code from mistakenly freeing descriptors belonging to other submitters. Considered the following example timeline: T1 T2 ======================================== ID1 WD1 while(WD1!=QI_DONE) unlock lock WD1=QI_DONE* WD2 while(WD2!=QI_DONE) unlock lock WD1==QI_DONE? ID1=QI_DONE WD2=DONE* reclaim() ID1=FREE WD1=FREE WD2=FREE unlock soft lockup! T2 never sees QI_DONE in WD2 Where: ID = invalidation descriptor WD = wait descriptor * Written by hardware The root of the problem is that the descriptor status QI_DONE flag is used for two conflicting purposes: 1. signal a descriptor is ready for reclaim (to be freed) 2. signal by the hardware that a wait descriptor is complete The solution (in this patch) is state separation by using QI_FREE flag for #1. Once a thread's invalidation descriptors are complete, their status would be set to QI_FREE. The reclaim_free_desc() function would then only free descriptors marked as QI_FREE instead of those marked as QI_DONE. This change ensures that T2 (from the previous example) will correctly observe the completion of its invalidation_wait (marked as QI_DONE). Signed-off-by: Sanjay K Kumar <[email protected]> Signed-off-by: Jacob Pan <[email protected]> Reviewed-by: Kevin Tian <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Lu Baolu <[email protected]> Signed-off-by: Joerg Roedel <[email protected]>
1 parent 50a7e2c commit 3cf7423

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

drivers/iommu/intel/dmar.c

+11-5
Original file line numberDiff line numberDiff line change
@@ -1204,9 +1204,7 @@ static void free_iommu(struct intel_iommu *iommu)
12041204
*/
12051205
static inline void reclaim_free_desc(struct q_inval *qi)
12061206
{
1207-
while (qi->desc_status[qi->free_tail] == QI_DONE ||
1208-
qi->desc_status[qi->free_tail] == QI_ABORT) {
1209-
qi->desc_status[qi->free_tail] = QI_FREE;
1207+
while (qi->desc_status[qi->free_tail] == QI_FREE && qi->free_tail != qi->free_head) {
12101208
qi->free_tail = (qi->free_tail + 1) % QI_LENGTH;
12111209
qi->free_cnt++;
12121210
}
@@ -1463,8 +1461,16 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
14631461
raw_spin_lock(&qi->q_lock);
14641462
}
14651463

1466-
for (i = 0; i < count; i++)
1467-
qi->desc_status[(index + i) % QI_LENGTH] = QI_DONE;
1464+
/*
1465+
* The reclaim code can free descriptors from multiple submissions
1466+
* starting from the tail of the queue. When count == 0, the
1467+
* status of the standalone wait descriptor at the tail of the queue
1468+
* must be set to QI_FREE to allow the reclaim code to proceed.
1469+
* It is also possible that descriptors from one of the previous
1470+
* submissions has to be reclaimed by a subsequent submission.
1471+
*/
1472+
for (i = 0; i <= count; i++)
1473+
qi->desc_status[(index + i) % QI_LENGTH] = QI_FREE;
14681474

14691475
reclaim_free_desc(qi);
14701476
raw_spin_unlock_irqrestore(&qi->q_lock, flags);

0 commit comments

Comments
 (0)