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

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Friday, December 8, 2023 8:46 PM
> 
> On 2023/12/8 17:09, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >> 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;