From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 38F2B18892C; Wed, 25 Sep 2024 12:15:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727266517; cv=none; b=mIhBv9GTB5x5Zf6qTi4B2IB81hLDB6aSMA/MyhhSSiWh+mTHY51QsXQM7yzO3yvHFBe9M1VWHi0YYq6dRt5COpVBB+ep2e+QGy7w/xxbBgqMAGMRwwpOAUHxodHkDzQbMMc8f9lJSxRPig1Fkc3dtiiNLgTA+pA3JVGV9RndRm0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727266517; c=relaxed/simple; bh=MVrHshqS6okqmx33IUUgWFbOHXM5m6ZOziCB0z65Fv4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=pe0QkOgIWBg7QSnxE38hmV5IY6fuqLjQj8sbhMJPnMslW3puwchpyUpCV9N8ZRzaRB9zJffKyWrnkJTqd5rmtE/InJ4sQsZ234VJhiiK1h2TbT2BpV1Lnd0QJFsDUpslK23WPX+iv8hIrkBALKMT4f68VJyQLYj5leavUZnJ0CU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=uquDa7JQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="uquDa7JQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 96965C4CEC7; Wed, 25 Sep 2024 12:15:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727266517; bh=MVrHshqS6okqmx33IUUgWFbOHXM5m6ZOziCB0z65Fv4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uquDa7JQL0z+5dTRRJAg1PNq1bUZc2jn35p+CrlM0BGMHMwMwjEhuewJlDXG7J1Ek z9nkvqltAs1k0MjZnR/PixQbYyXci+nL2ACauKy+rEYZXtzr/R4sVpuXnACAUhHpyF sYiPoVFUv6FrF5I4xz4fMn72Ifdc4SPugRXM3vnxoAjb7DK/QMUJgFAMwRahbfwOjx sE+SbEKuEiaccov1ZCLuoLaLVC+aFC/u+LD1SyZvkDVev+4eHXyjxjShxzdo/L52cR m3IIlJmK9vuzqOB2WM1dQqAq1Yrqygco90EyZo3wvN0JdKJvV8Me4cswsoODqj5rOa riObZQRcVYNPA== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Sanjay K Kumar , Jacob Pan , Kevin Tian , Lu Baolu , Joerg Roedel , Sasha Levin , dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, iommu@lists.linux.dev Subject: [PATCH AUTOSEL 6.6 082/139] iommu/vt-d: Fix potential lockup if qi_submit_sync called with 0 count Date: Wed, 25 Sep 2024 08:08:22 -0400 Message-ID: <20240925121137.1307574-82-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240925121137.1307574-1-sashal@kernel.org> References: <20240925121137.1307574-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.6.52 Content-Transfer-Encoding: 8bit From: Sanjay K Kumar [ Upstream commit 3cf74230c139f208b7fb313ae0054386eee31a81 ] 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 Signed-off-by: Jacob Pan Reviewed-by: Kevin Tian Link: https://lore.kernel.org/r/20240728210059.1964602-1-jacob.jun.pan@linux.intel.com Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel Signed-off-by: Sasha Levin --- drivers/iommu/intel/dmar.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c index 84f0459e503cf..7a38e18b18196 100644 --- a/drivers/iommu/intel/dmar.c +++ b/drivers/iommu/intel/dmar.c @@ -1202,9 +1202,7 @@ static void free_iommu(struct intel_iommu *iommu) */ static inline void reclaim_free_desc(struct q_inval *qi) { - while (qi->desc_status[qi->free_tail] == QI_DONE || - qi->desc_status[qi->free_tail] == QI_ABORT) { - qi->desc_status[qi->free_tail] = QI_FREE; + while (qi->desc_status[qi->free_tail] == QI_FREE && qi->free_tail != qi->free_head) { qi->free_tail = (qi->free_tail + 1) % QI_LENGTH; qi->free_cnt++; } @@ -1439,8 +1437,16 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc, raw_spin_lock(&qi->q_lock); } - for (i = 0; i < count; i++) - qi->desc_status[(index + i) % QI_LENGTH] = QI_DONE; + /* + * The reclaim code can free descriptors from multiple submissions + * starting from the tail of the queue. When count == 0, the + * status of the standalone wait descriptor at the tail of the queue + * must be set to QI_FREE to allow the reclaim code to proceed. + * It is also possible that descriptors from one of the previous + * submissions has to be reclaimed by a subsequent submission. + */ + for (i = 0; i <= count; i++) + qi->desc_status[(index + i) % QI_LENGTH] = QI_FREE; reclaim_free_desc(qi); raw_spin_unlock_irqrestore(&qi->q_lock, flags); -- 2.43.0