Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only

On Thu, Jul 23, 2026 at 06:15:48AM +0000, Vasant Hegde wrote:
> Certain AMD GPU's must always be in identity mode. Currently its enforced
> using PASID check. It worked fine as most GPU's has PASID feature. But
> this means, identity mode enforcement is done for all PASID capable devices.

I think it would be useful to know something about *why* these devices
require identity mode.  And what happens without identity mode, i.e., is
there a user-visible symptom that happens when the wrong mode is used?

Since the code doesn't test any feature bits, I assume it's because these
devices have some hardware defect?

s/GPU's/GPUs/ (twice)
s/Currently its/Currently it's/
s/has PASID/have PASID/

> Previously it made sense as domain allocation API
> (iommu_ops->domain_alloc()) was just passing domain type. So it couldn't
> check device capability and decide best suited page table type (v1 or
> v2). With recent enhancement to driver code, it uses
> domain_alloc_paging_flags() ops for all paging mode domain allocation.
> This can check device/flags and allocate best suited page table (v1 or v2).
> Hence fix amd_iommu_def_domain_type() to force identity mapping for selected
> GPUs only.
> 
> With this change system booted with DMA translation mode will select:
>  * Guest (v2) page table for PASID capable device
>  * Host (v1) page table for non-PASID capable device
> 
> Cc: Alex Deucher <[email protected]>
> Cc: Mario Limonciello <[email protected]>
> Signed-off-by: Vasant Hegde <[email protected]>
> Tested-by: Amandeep Kaur Longia <[email protected]>
> ---
>  drivers/iommu/amd/iommu.c | 56 +++++++++++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 563f9c2672d5..fe642e33c87d 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3112,6 +3112,32 @@ static bool amd_iommu_is_attach_deferred(struct device *dev)
>  	return dev_data->defer_attach;
>  }
>  
> +static bool quirks_force_identity_mapping(struct pci_dev *pdev)
> +{
> +	struct pci_dev *root_port;
> +	int class = pdev->class >> 8;
> +
> +	/* AMD GPU vendor ID */
> +	if (pdev->vendor != PCI_VENDOR_ID_ATI)
> +		return false;
> +
> +	/* GPU class */
> +	if (class != PCI_CLASS_DISPLAY_VGA &&
> +	    class != PCI_CLASS_DISPLAY_OTHER)
> +		return false;
> +
> +	root_port = pcie_find_root_port(pdev);
> +	if (!root_port)
> +		return false;
> +
> +	/* If bridge vendor is not ATI then its APU and force IDENTITY mode */

s/its/it's/  "it's" == "it is"; "its" shows ownership

> +	if (root_port->vendor != PCI_VENDOR_ID_ATI)
> +		return true;
> +
> +	/* Rest all are dGPUs and works fine with DMA mode */
> +	return false;
> +}
> +
>  static int amd_iommu_def_domain_type(struct device *dev)
>  {
>  	struct iommu_dev_data *dev_data;
> @@ -3120,20 +3146,28 @@ static int amd_iommu_def_domain_type(struct device *dev)
>  	if (!dev_data)
>  		return 0;
>  
> +	if (!dev_is_pci(dev))
> +		return 0;
> +
>  	/* Always use DMA domain for untrusted device */
> -	if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
> +	if (to_pci_dev(dev)->untrusted)
>  		return IOMMU_DOMAIN_DMA;

This seems to apply a different rule to untrusted (e.g., removable) devices
based on the topology, not the device itself.  What if one of these devices
is used as an eGPU and thus uses IOMMU_DOMAIN_DMA when it really needs
IOMMU_DOMAIN_IDENTITY?

> -	/*
> -	 * Do not identity map IOMMUv2 capable devices when:
> -	 *  - memory encryption is active, because some of those devices
> -	 *    (AMD GPUs) don't have the encryption bit in their DMA-mask
> -	 *    and require remapping.
> -	 *  - SNP is enabled, because it prohibits DTE[Mode]=0.
> -	 */
> -	if (pdev_pasid_supported(dev_data) &&
> -	    !cc_platform_has(CC_ATTR_MEM_ENCRYPT) &&
> -	    !amd_iommu_snp_en) {
> +	/* Apply device specific quirks */
> +	if (quirks_force_identity_mapping(to_pci_dev(dev))) {
> +
> +		/*
> +		 * When memory encryption is active, some of these devices
> +		 * don't have the encryption bit in their DMA-mask and
> +		 * require remapping.
> +		 */
> +		if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> +			return 0;
> +
> +		/* DTE[Mode]=0 is prohibited when SNP is enabled */
> +		if (amd_iommu_snp_en)
> +			return 0;
> +
>  		return IOMMU_DOMAIN_IDENTITY;
>  	}
>  
> -- 
> 2.31.1
>