Re: [PATCH v2 4/6] iommu/vt-d: Remove 1:1 mappings from identity domain

[Baolu Lu]

On 12/13/23 10:20 AM, Tian, Kevin wrote:
> > > 	if (!dmar_map_gfx)
> > > 		iommu_identity_mapping |= IDENTMAP_GFX;
> > So with above cleaned up, we have no need to worry about drivers that
> > are not capable of handling remapped dma address any more.
> >
> > Did I miss anything?
> I prefer to removing IDENTMAP_GFX in this series and put a comment
> explaining why Azalia device has no problem.
>
> Then send a separate patch to remove the GFX workaround option.
> If there is any valid usage still relying on that, it's easy to revert.

Agreed. We should be more cautious. Perhaps I will postpone this series
to a time when we are sure that graphic drivers are okay with this
change. As a first step, perhaps we can make a change to remove the
workaround for graphic drivers, so that any hidden bugs in the graphic
driver could be reported.

The patch looks like,

iommu/vt-d: Remove INTEL_IOMMU_BROKEN_GFX_WA

Commit 62edf5dc4a524 ("intel-iommu: Restore DMAR_BROKEN_GFX_WA option for
broken graphics drivers") was introduced 24 years ago as a temporary
workaround for graphics drivers that used physical addresses for DMA and
avoided DMA APIs. This workaround was disabled by default.

As 24 years have passed, it is expected that graphics driver developers
have migrated their drivers to use kernel DMA APIs. Therefore, this
workaround is no longer required and could been removed.

Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
 drivers/iommu/intel/iommu.c | 10 ----------
 drivers/iommu/intel/Kconfig | 11 -----------
 2 files changed, 21 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 84b78e42a470..27b8638291f2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2357,9 +2357,6 @@ static int device_def_domain_type(struct device *dev)

 		if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev))
 			return IOMMU_DOMAIN_IDENTITY;
-
-		if ((iommu_identity_mapping & IDENTMAP_GFX) && IS_GFX_DEVICE(pdev))
-			return IOMMU_DOMAIN_IDENTITY;
 	}

 	return 0;
@@ -2660,13 +2657,6 @@ static int __init init_dmars(void)
 		iommu_set_root_entry(iommu);
 	}

-#ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
-	dmar_map_gfx = 0;
-#endif
-
-	if (!dmar_map_gfx)
-		iommu_identity_mapping |= IDENTMAP_GFX;
-
 	check_tylersburg_isoch();

 	ret = si_domain_init(hw_pass_through);
diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 012cd2541a68..d2d34eb28d94 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -64,17 +64,6 @@ config INTEL_IOMMU_DEFAULT_ON
 	  one is found. If this option is not selected, DMAR support can
 	  be enabled by passing intel_iommu=on to the kernel.

-config INTEL_IOMMU_BROKEN_GFX_WA
-	bool "Workaround broken graphics drivers (going away soon)"
-	depends on BROKEN && X86
-	help
-	  Current Graphics drivers tend to use physical address
-	  for DMA and avoid using DMA APIs. Setting this config
-	  option permits the IOMMU driver to set a unity map for
-	  all the OS-visible memory. Hence the driver can continue
-	  to use physical addresses for DMA, at least until this
-	  option is removed in the 2.6.32 kernel.
-
 config INTEL_IOMMU_FLOPPY_WA
 	def_bool y
 	depends on X86

Best regards,
baolu