linux-kernel - [PATCH v1 3/3] iommu/vt-d: Fix ATS support check for integrated endpoints

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250509140021.4029303-4-wei.w.wang@intel.com>
Date: Fri,  9 May 2025 22:00:21 +0800
From: Wei Wang <wei.w.wang@...el.com>
To: dwmw2@...radead.org,
	baolu.lu@...ux.intel.com,
	kevin.tian@...el.com,
	yi.l.liu@...el.com,
	jroedel@...e.de,
	linux-kernel@...r.kernel.org,
	iommu@...ts.linux.dev
Cc: Wei Wang <wei.w.wang@...el.com>
Subject: [PATCH v1 3/3] iommu/vt-d: Fix ATS support check for integrated endpoints

The VT-d spec states that "SATC reporting structure identifies devices
that have address translation cache and that is validated per requirements
described in the 'Device TLB in System-on-Chip (SoC) Integrated Devices'
section. It is recommended that system software enable ATC for this
device". It is possible for an integrated device to have PCI ATC
capability implemented but not validated per the requirements, and thus
not appear in the SATC structure as recommended for ATS enablement.

The current implementation checks ATS support for integrated endpoints in
two places. First, it verifies if the integrated endpoint device is listed
in SATC. If not, it proceeds to the second check that always returns true
for integrated devices. This could result in endpoint devices not
recommended in SATC presenting "supported = true" to the caller.

Add integrated_device_ats_supported() for the integrated device ATS check
in a single location, which improves readability. The above issue is
also fixed in the function via returning false in that case.

Signed-off-by: Wei Wang <wei.w.wang@...el.com>
---
 drivers/iommu/intel/iommu.c | 42 +++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 2778bfe14f36..39abcf4e0f8f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2760,6 +2760,34 @@ static struct dmar_satc_unit *dmar_find_matched_satc_unit(struct pci_dev *dev)
 	return satcu;
 }
 
+static bool integrated_device_ats_supported(struct pci_dev *dev, struct intel_iommu *iommu)
+{
+	struct dmar_satc_unit *satcu = dmar_find_matched_satc_unit(dev);
+
+	/*
+	 * This device supports ATS as it is in SATC table. When IOMMU is in
+	 * legacy mode, enabling ATS is done automatically by HW for the device
+	 * that requires ATS, hence OS should not enable this device ATS to
+	 * avoid duplicated TLB invalidation.
+	 */
+	if (satcu)
+		return !(satcu->atc_required && !sm_supported(iommu));
+
+	/*
+	 * The integrated device isn't enumerated in the SATC structure. For
+	 * example, it has ATS PCI capability implemented but not validated per
+	 * the requirements described in the VT-d specification, specifically
+	 * in the "Device TLB in System-on-Chip (SoC) Integrated Devices"
+	 * section. Therefore, it does not appear in the SATC structure. Return
+	 * false in this case.
+	 *
+	 * On older machines that do not support SATC (i.e., no SATC structure
+	 * present), ATS is considered to be "always" supported for integrated
+	 * endpoints.
+	 */
+	return !list_empty(&dmar_satc_units);
+}
+
 static bool dmar_ats_supported(struct pci_dev *dev, struct intel_iommu *iommu)
 {
 	int i;
@@ -2769,25 +2797,13 @@ static bool dmar_ats_supported(struct pci_dev *dev, struct intel_iommu *iommu)
 	struct device *tmp;
 	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
-	struct dmar_satc_unit *satcu;
 
 	dev = pci_physfn(dev);
-	satcu = dmar_find_matched_satc_unit(dev);
-	if (satcu)
-		/*
-		 * This device supports ATS as it is in SATC table.
-		 * When IOMMU is in legacy mode, enabling ATS is done
-		 * automatically by HW for the device that requires
-		 * ATS, hence OS should not enable this device ATS
-		 * to avoid duplicated TLB invalidation.
-		 */
-		return !(satcu->atc_required && !sm_supported(iommu));
 
 	for (bus = dev->bus; bus; bus = bus->parent) {
 		bridge = bus->self;
-		/* If it's an integrated device, allow ATS */
 		if (!bridge)
-			return true;
+			return integrated_device_ats_supported(dev, iommu);
 		/* Connected via non-PCIe: no ATS */
 		if (!pci_is_pcie(bridge) ||
 		    pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)
-- 
2.43.0