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

On 8/24/26 03:58, Vasant Hegde wrote:
> Certain AMD GPUs must always operate in IOMMU identity mode. This was
> previously enforced using a PASID check, which happened to work because
> these specific GPUs are PASID-capable. However, this approach incorrectly
> applies identity mode enforcement to all PASID-capable devices, not just
> the GPUs that require it.
> 
> This made sense in the past because the domain allocation API
> (iommu_ops->domain_alloc()) only received the domain type, so the
> driver had no way to inspect device capabilities and pick the most
> suitable page table format (v1 or v2). With the recent driver
> enhancement to use domain_alloc_paging_flags() for all paging
> domain allocations, the driver can now inspect the device and flags
> directly and choose the appropriate page table type per device.
> 
> Update amd_iommu_def_domain_type() to force identity mapping only for
> the specific GPUs that require it, via a new quirks_force_identity_mapping().
> 
> With this change, a system booting in DMA translation mode will now
> select:
>   * Guest (v2) page table for PASID-capable devices
>   * Host (v1) page table for non-PASID-capable devices
> 
> Also drop the amd_iommu_snp_en check, as SNP enforces paging domain,
> which doesn't work with the identity requirement of these GPUs.
> 
> Link: https://lore.kernel.org/all/[email protected]/
> Link: https://lore.kernel.org/linux-iommu/[email protected]/
> Cc: Alex Deucher <[email protected]>
> Cc: Mario Limonciello <[email protected]>
> Signed-off-by: Vasant Hegde <[email protected]>
Reviewed-by: Mario Limonciello (AMD) <[email protected]>> ---
> Changes in v2:
>    - Dropped disabling ATS for "Radeon Pro WX 4100"
>    - Addressed review comments
> 
> @Jason,
>    Once this patch settles, I will send separate patch to drop
>    'untrusted' check inside amd_iommu_def_domain_type().
> 
>    Regarding SME check, I have retained SME chek inside
>    quirks_force_identity_mapping(). If everyone is fine to drop then I
>    will do follow up patch later.
> 
> -Vasant
> 
>   drivers/iommu/amd/iommu.c | 45 +++++++++++++++++++++++++++++----------
>   1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 29dc18d3d22e..fc7819f57521 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3122,6 +3122,26 @@ 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)
> +{
> +	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;
> +
> +	if (pci_upstream_bridge(pdev)->vendor == PCI_VENDOR_ID_ATI)
> +		return false;
> +
> +	/* It is the GPU in an APU, force identity domain */
> +	return true;
> +}
> +
>   static int amd_iommu_def_domain_type(struct device *dev)
>   {
>   	struct iommu_dev_data *dev_data;
> @@ -3130,20 +3150,23 @@ 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;
>   
> -	/*
> -	 * 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;
> +
>   		return IOMMU_DOMAIN_IDENTITY;
>   	}
>