linux-kernel - [PATCH] iommu/vt-d: Handle volatile descriptor status read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240607173817.3914600-1-jacob.jun.pan@linux.intel.com>
Date: Fri,  7 Jun 2024 10:38:17 -0700
From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
To: "Lu Baolu" <baolu.lu@...ux.intel.com>,
	LKML <linux-kernel@...r.kernel.org>,
	iommu@...ts.linux.dev
Cc: "Tian, Kevin" <kevin.tian@...el.com>,
	Yi Liu <yi.l.liu@...el.com>,
	sanjay.k.kumar@...el.com,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: [PATCH] iommu/vt-d: Handle volatile descriptor status read

Queued invalidation wait descriptor status is volatile in that IOMMU hardware
writes the data upon completion.

Use READ_ONCE() to prevent compiler optimizations which ensures memory
reads every time. As a side effect, READ_ONCE() also enforces strict types and
may add an extra instruction. But it should not have negative
performance impact since we use cpu_relax anyway and the extra time(by
adding an instruction) may allow IOMMU HW request cacheline ownership easier.

e.g. gcc 12.3
BEFORE:
	81 38 ad de 00 00       cmpl   $0x2,(%rax)

AFTER (with READ_ONCE())
    772f:       8b 00                   mov    (%rax),%eax
    7731:       3d ad de 00 00          cmp    $0x2,%eax //status data is 32 bit

Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>
---
 drivers/iommu/intel/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 304e84949ca7..1c8d3141cb55 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1446,7 +1446,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
 	 */
 	writel(qi->free_head << shift, iommu->reg + DMAR_IQT_REG);
 
-	while (qi->desc_status[wait_index] != QI_DONE) {
+	while (READ_ONCE(qi->desc_status[wait_index]) != QI_DONE) {
 		/*
 		 * We will leave the interrupts disabled, to prevent interrupt
 		 * context to queue another cmd while a cmd is already submitted
-- 
2.25.1