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

Tian, Kevin
2023-12-11T03:58:48Z

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