Older VT-d hardware implementations did not support pass-through
translation mode. The iommu driver relied on a DMA domain with all
physical memory addresses identically mapped to the same IOVA to
simulate pass-through translation.

This workaround is no longer necessary due to the evolution of iommu
core. The core has introduced def_domain_type op, allowing the iommu
driver to specify its capabilities. Additionally, the identity domain
has become a static system domain with "never fail" attach semantics.

Eliminate support for the 1:1 mapping domain on older hardware and
removes the unused code that created the 1:1 page table. This paves a
way for the implementation of a global static identity domain.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 118 +++---------------------------------
 1 file changed, 10 insertions(+), 108 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 605cd1c52e95..7022cc183120 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2146,29 +2146,10 @@ static bool dev_is_real_dma_subdevice(struct device *dev)
 	       pci_real_dma_dev(to_pci_dev(dev)) != to_pci_dev(dev);
 }
 
-static int iommu_domain_identity_map(struct dmar_domain *domain,
-				     unsigned long first_vpfn,
-				     unsigned long last_vpfn)
-{
-	/*
-	 * RMRR range might have overlap with physical memory range,
-	 * clear it first
-	 */
-	dma_pte_clear_range(domain, first_vpfn, last_vpfn);
-
-	return __domain_mapping(domain, first_vpfn,
-				first_vpfn, last_vpfn - first_vpfn + 1,
-				DMA_PTE_READ|DMA_PTE_WRITE, GFP_KERNEL);
-}
-
 static int md_domain_init(struct dmar_domain *domain, int guest_width);
 
 static int __init si_domain_init(int hw)
 {
-	struct dmar_rmrr_unit *rmrr;
-	struct device *dev;
-	int i, nid, ret;
-
 	si_domain = alloc_domain(IOMMU_DOMAIN_IDENTITY);
 	if (!si_domain)
 		return -EFAULT;
@@ -2179,44 +2160,6 @@ static int __init si_domain_init(int hw)
 		return -EFAULT;
 	}
 
-	if (hw)
-		return 0;
-
-	for_each_online_node(nid) {
-		unsigned long start_pfn, end_pfn;
-		int i;
-
-		for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
-			ret = iommu_domain_identity_map(si_domain,
-					mm_to_dma_pfn_start(start_pfn),
-					mm_to_dma_pfn_end(end_pfn));
-			if (ret)
-				return ret;
-		}
-	}
-
-	/*
-	 * Identity map the RMRRs so that devices with RMRRs could also use
-	 * the si_domain.
-	 */
-	for_each_rmrr_units(rmrr) {
-		for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
-					  i, dev) {
-			unsigned long long start = rmrr->base_address;
-			unsigned long long end = rmrr->end_address;
-
-			if (WARN_ON(end < start ||
-				    end >> agaw_to_width(si_domain->agaw)))
-				continue;
-
-			ret = iommu_domain_identity_map(si_domain,
-					mm_to_dma_pfn_start(start >> PAGE_SHIFT),
-					mm_to_dma_pfn_end(end >> PAGE_SHIFT));
-			if (ret)
-				return ret;
-		}
-	}
-
 	return 0;
 }
 
@@ -2301,6 +2244,9 @@ static bool device_rmrr_is_relaxable(struct device *dev)
  */
 static int device_def_domain_type(struct device *dev)
 {
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu = info->iommu;
+
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
 
@@ -2311,6 +2257,13 @@ static int device_def_domain_type(struct device *dev)
 			return IOMMU_DOMAIN_IDENTITY;
 	}
 
+	/*
+	 * Hardware does not support the passthrough translation mode.
+	 * Always use a dynamaic mapping domain.
+	 */
+	if (!ecap_pass_through(iommu->ecap))
+		return IOMMU_DOMAIN_DMA;
+
 	return 0;
 }
 
@@ -3301,52 +3254,6 @@ int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
 	return 0;
 }
 
-static int intel_iommu_memory_notifier(struct notifier_block *nb,
-				       unsigned long val, void *v)
-{
-	struct memory_notify *mhp = v;
-	unsigned long start_vpfn = mm_to_dma_pfn_start(mhp->start_pfn);
-	unsigned long last_vpfn = mm_to_dma_pfn_end(mhp->start_pfn +
-			mhp->nr_pages - 1);
-
-	switch (val) {
-	case MEM_GOING_ONLINE:
-		if (iommu_domain_identity_map(si_domain,
-					      start_vpfn, last_vpfn)) {
-			pr_warn("Failed to build identity map for [%lx-%lx]\n",
-				start_vpfn, last_vpfn);
-			return NOTIFY_BAD;
-		}
-		break;
-
-	case MEM_OFFLINE:
-	case MEM_CANCEL_ONLINE:
-		{
-			struct dmar_drhd_unit *drhd;
-			struct intel_iommu *iommu;
-			LIST_HEAD(freelist);
-
-			domain_unmap(si_domain, start_vpfn, last_vpfn, &freelist);
-
-			rcu_read_lock();
-			for_each_active_iommu(iommu, drhd)
-				iommu_flush_iotlb_psi(iommu, si_domain,
-					start_vpfn, mhp->nr_pages,
-					list_empty(&freelist), 0);
-			rcu_read_unlock();
-			put_pages_list(&freelist);
-		}
-		break;
-	}
-
-	return NOTIFY_OK;
-}
-
-static struct notifier_block intel_iommu_memory_nb = {
-	.notifier_call = intel_iommu_memory_notifier,
-	.priority = 0
-};
-
 static void intel_disable_iommus(void)
 {
 	struct intel_iommu *iommu = NULL;
@@ -3643,12 +3550,7 @@ int __init intel_iommu_init(void)
 
 		iommu_pmu_register(iommu);
 	}
-	up_read(&dmar_global_lock);
 
-	if (si_domain && !hw_pass_through)
-		register_memory_notifier(&intel_iommu_memory_nb);
-
-	down_read(&dmar_global_lock);
 	if (probe_acpi_namespace_devices())
 		pr_warn("ACPI name space devices didn't probe correctly\n");
 
-- 
2.34.1

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, December 5, 2023 9:22 AM
> 
> Older VT-d hardware implementations did not support pass-through
> translation mode. The iommu driver relied on a DMA domain with all
> physical memory addresses identically mapped to the same IOVA to
> simulate pass-through translation.
> 
> This workaround is no longer necessary due to the evolution of iommu
> core. The core has introduced def_domain_type op, allowing the iommu
> driver to specify its capabilities. Additionally, the identity domain
> has become a static system domain with "never fail" attach semantics.

I'm not sure above explains the reason for removing the identity support
on older hardware. Looks the reason is simply that continuing to maintain
that debt prevents intel-iommu driver from catching up  with iommu core
evolution so we decide to remove it.

> 
> Eliminate support for the 1:1 mapping domain on older hardware and
> removes the unused code that created the 1:1 page table. This paves a
> way for the implementation of a global static identity domain.

Do you know how old such hardware is?

> @@ -2311,6 +2257,13 @@ static int device_def_domain_type(struct device
> *dev)
>  			return IOMMU_DOMAIN_IDENTITY;
>  	}
> 
> +	/*
> +	 * Hardware does not support the passthrough translation mode.
> +	 * Always use a dynamaic mapping domain.
> +	 */
> +	if (!ecap_pass_through(iommu->ecap))
> +		return IOMMU_DOMAIN_DMA;
> +
>  	return 0;

there are two cases above which mandates IDENTITY. Have you confirmed
that those platforms support hardware passthrough? otherwise this change
is broken.

On 2023/12/8 17:09, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, December 5, 2023 9:22 AM
>>
>> Older VT-d hardware implementations did not support pass-through
>> translation mode. The iommu driver relied on a DMA domain with all
>> physical memory addresses identically mapped to the same IOVA to
>> simulate pass-through translation.
>>
>> This workaround is no longer necessary due to the evolution of iommu
>> core. The core has introduced def_domain_type op, allowing the iommu
>> driver to specify its capabilities. Additionally, the identity domain
>> has become a static system domain with "never fail" attach semantics.
> 
> I'm not sure above explains the reason for removing the identity support
> on older hardware. Looks the reason is simply that continuing to maintain
> that debt prevents intel-iommu driver from catching up  with iommu core
> evolution so we decide to remove it.

It is not that simple. I should put more words here.

Generally speaking, hardware lacking passthrough translation support,
but the iommu driver fakes it by using a DMA domain with 1:1 mappings,
makes no sense because it doesn't mitigate any hardware overheads.

If the device driver uses the kernel DMA API to do DMA, it does not need
to care about the DMA translation type. This is a user-decided policy.
The iommu subsystem has already provided this support to users through
kernel build options, kernel boot commands, and sysfs nodes.

If the device driver doesn't use kernel API for DMA. While we discourage
this behavior, the iommu subsystem provides the DMA ownership mechanism.
This allows the driver to take over the DMA ownership, allocate and
manage its own domain, and replace it with the default domain, as the
iommu default domain is only designed for kernel DMA with the DMA API.

In summary, whether or not to use a DMA domain with 1:1 mappings should
be a design decision made in the device driver, not a common behavior
for a modern iommu driver.

> 
>>
>> Eliminate support for the 1:1 mapping domain on older hardware and
>> removes the unused code that created the 1:1 page table. This paves a
>> way for the implementation of a global static identity domain.
> 
> Do you know how old such hardware is?

I am not sure, but I have a desktop that is over 10 years old and
supports passthrough translation. :-)

> 
>> @@ -2311,6 +2257,13 @@ static int device_def_domain_type(struct device
>> *dev)
>>   			return IOMMU_DOMAIN_IDENTITY;
>>   	}
>>
>> +	/*
>> +	 * Hardware does not support the passthrough translation mode.
>> +	 * Always use a dynamaic mapping domain.
>> +	 */
>> +	if (!ecap_pass_through(iommu->ecap))
>> +		return IOMMU_DOMAIN_DMA;
>> +
>>   	return 0;
> 
> there are two cases above which mandates IDENTITY. Have you confirmed
> that those platforms support hardware passthrough? otherwise this change
> is broken.

Those two cases should be hardware quirks for SoC-integrated devices. It
makes no reason that a quirk requires IOMMU passthrough translation, but
the hardware doesn't support it.

If, unfortunately, those quirks turn out to be workarounds for a poorly
designed device driver, we should remove those quirks and request the
device driver to utilize the DMA ownership framework to achieve the same
functionality within the driver itself.

Best regards,
baolu

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, December 8, 2023 8:46 PM
> 
> On 2023/12/8 17:09, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Tuesday, December 5, 2023 9:22 AM
> >>
> >> Older VT-d hardware implementations did not support pass-through
> >> translation mode. The iommu driver relied on a DMA domain with all
> >> physical memory addresses identically mapped to the same IOVA to
> >> simulate pass-through translation.
> >>
> >> This workaround is no longer necessary due to the evolution of iommu
> >> core. The core has introduced def_domain_type op, allowing the iommu
> >> driver to specify its capabilities. Additionally, the identity domain
> >> has become a static system domain with "never fail" attach semantics.
> >
> > I'm not sure above explains the reason for removing the identity support
> > on older hardware. Looks the reason is simply that continuing to maintain
> > that debt prevents intel-iommu driver from catching up  with iommu core
> > evolution so we decide to remove it.
> 
> It is not that simple. I should put more words here.
> 
> Generally speaking, hardware lacking passthrough translation support,
> but the iommu driver fakes it by using a DMA domain with 1:1 mappings,
> makes no sense because it doesn't mitigate any hardware overheads.

Not about overhead. Might just be that device or driver is not capable of
handling remapped dma address.

> 
> If the device driver uses the kernel DMA API to do DMA, it does not need
> to care about the DMA translation type. This is a user-decided policy.
> The iommu subsystem has already provided this support to users through
> kernel build options, kernel boot commands, and sysfs nodes.
> 
> If the device driver doesn't use kernel API for DMA. While we discourage
> this behavior, the iommu subsystem provides the DMA ownership
> mechanism.
> This allows the driver to take over the DMA ownership, allocate and
> manage its own domain, and replace it with the default domain, as the
> iommu default domain is only designed for kernel DMA with the DMA API.
> 
> In summary, whether or not to use a DMA domain with 1:1 mappings
> should
> be a design decision made in the device driver, not a common behavior
> for a modern iommu driver.
> 
> >
> >>
> >> Eliminate support for the 1:1 mapping domain on older hardware and
> >> removes the unused code that created the 1:1 page table. This paves a
> >> way for the implementation of a global static identity domain.
> >
> > Do you know how old such hardware is?
> 
> I am not sure, but I have a desktop that is over 10 years old and
> supports passthrough translation. :-)
> 
> >
> >> @@ -2311,6 +2257,13 @@ static int device_def_domain_type(struct
> device
> >> *dev)
> >>   			return IOMMU_DOMAIN_IDENTITY;
> >>   	}
> >>
> >> +	/*
> >> +	 * Hardware does not support the passthrough translation mode.
> >> +	 * Always use a dynamaic mapping domain.
> >> +	 */
> >> +	if (!ecap_pass_through(iommu->ecap))
> >> +		return IOMMU_DOMAIN_DMA;
> >> +
> >>   	return 0;
> >
> > there are two cases above which mandates IDENTITY. Have you confirmed
> > that those platforms support hardware passthrough? otherwise this
> change
> > is broken.
> 
> Those two cases should be hardware quirks for SoC-integrated devices. It
> makes no reason that a quirk requires IOMMU passthrough translation, but
> the hardware doesn't support it.
> 
> If, unfortunately, those quirks turn out to be workarounds for a poorly
> designed device driver, we should remove those quirks and request the
> device driver to utilize the DMA ownership framework to achieve the same
> functionality within the driver itself.
> 

if that is the case you should fix the drivers first before breaking them.

But at a glance looks those two quirks are just fine.

For Azalia sound device the problem is that BIOS enables a dedicated
DMAR for it but allocates zero TLB entries to cause deadlock. This
implies a hw passthrough mode otherwise it's still broken.

For GFX it's a workaround added since day one. there is even still
an option CONFIG_INTEL_IOMMU_BROKEN_GFX_WA available. But
now its meaning is really disabling IOMMU instead of using identity.

sounds like IDENTMAP_GFX can be fully removed now:

#ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
	dmar_map_gfx = 0;
#endif

	if (!dmar_map_gfx)
		iommu_identity_mapping |= IDENTMAP_GFX;

On 12/11/23 11:58 AM, Tian, Kevin wrote:
>>> there are two cases above which mandates IDENTITY. Have you confirmed
>>> that those platforms support hardware passthrough? otherwise this
>> change
>>> is broken.
>> Those two cases should be hardware quirks for SoC-integrated devices. It
>> makes no reason that a quirk requires IOMMU passthrough translation, but
>> the hardware doesn't support it.
>>
>> If, unfortunately, those quirks turn out to be workarounds for a poorly
>> designed device driver, we should remove those quirks and request the
>> device driver to utilize the DMA ownership framework to achieve the same
>> functionality within the driver itself.
>>
> if that is the case you should fix the drivers first before breaking them.
> 
> But at a glance looks those two quirks are just fine.
> 
> For Azalia sound device the problem is that BIOS enables a dedicated
> DMAR for it but allocates zero TLB entries to cause deadlock. This
> implies a hw passthrough mode otherwise it's still broken.

Yes. It's safe for Azalia sound device.

> 
> For GFX it's a workaround added since day one. there is even still
> an option CONFIG_INTEL_IOMMU_BROKEN_GFX_WA available. But
> now its meaning is really disabling IOMMU instead of using identity.
> 
> sounds like IDENTMAP_GFX can be fully removed now:
> 
> #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
> 	dmar_map_gfx = 0;
> #endif

We should already remove the workaround for the 2.6 kernel. :-)

It's default "n". Therefore, if any gfx driver still needs this
workaround, there should already be a bug report.

> 
> 	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?

Best regards,
baolu

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, December 12, 2023 2:21 PM
> >
> > For GFX it's a workaround added since day one. there is even still
> > an option CONFIG_INTEL_IOMMU_BROKEN_GFX_WA available. But
> > now its meaning is really disabling IOMMU instead of using identity.
> >
> > sounds like IDENTMAP_GFX can be fully removed now:
> >
> > #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
> > 	dmar_map_gfx = 0;
> > #endif
> 
> We should already remove the workaround for the 2.6 kernel. :-)

I don't know what happened to ignore that temporary option.

> 
> It's default "n". Therefore, if any gfx driver still needs this
> workaround, there should already be a bug report.

It depends on how it is set in distros.

> 
> >
> > 	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.

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@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
  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

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Wednesday, December 13, 2023 10:44 AM
> 
> 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.

and igfx_off option is still available just in case.

> 
> Suggested-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   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;
> -

let's remove IDENTMAP_GFX (and all its references) in a separate patch.

this patch is for removing the workaround option.

another patch removes IDENTMAP_GFX as it's meaningless when
dmar_map_gfx simply implies disabling the gfx iommu.

>   	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

On 12/13/23 11:04 AM, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Wednesday, December 13, 2023 10:44 AM
>>
>> 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.
> and igfx_off option is still available just in case.

Yeah!

> 
>> Suggested-by: Kevin Tian<kevin.tian@intel.com>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>>    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;
>> -
> let's remove IDENTMAP_GFX (and all its references) in a separate patch.
> 
> this patch is for removing the workaround option.
> 
> another patch removes IDENTMAP_GFX as it's meaningless when
> dmar_map_gfx simply implies disabling the gfx iommu.

Okay, it's fine to me.

Best regards,
baolu