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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Missing NULL pointer check for `pci_upstream_bridge()` causes a kernel panic during boot on systems where the GPU is on the root bus.
- [High] The heuristic used to distinguish discrete GPUs (dGPUs) from APU GPUs is flawed and will incorrectly force identity mapping on many dGPUs.
- [High] Discrete GPUs break on SNP-enabled systems due to the removal of the SNP check combined with flawed GPU identification.
--

--- Patch [1]: [PATCH v2] iommu/amd: Force identity mode for selected GPUs only ---
commit 8994b69ea108b9e83363d3bf65d86e8798e45ba5
Author: Vasant Hegde <vasant.hegde@amd.com>

iommu/amd: Force identity mode for selected GPUs only

This commit updates AMD IOMMU domain allocation to force identity mapping
only for specific APU GPUs, rather than all PASID-capable devices. It
introduces quirks_force_identity_mapping() to apply this targeted enforcement
and drops the previous SNP check.

Link: https://lore.kernel.org/all/20200824105415.21000-1-joro@8bytes.org/
Link: https://lore.kernel.org/linux-iommu/20260723061548.10187-1-vasant.hegde@amd.com/

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4dc306a4b5c62..eef585918e607 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3165,6 +3165,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)

[Severity: Critical]
Is it possible for pci_upstream_bridge() to return NULL here?

If the GPU is located on the PCIe root bus (such as in an integrated SoC 
design, or when passed through to a VM via VFIO), pci_upstream_bridge(pdev)
would return NULL. This would lead to a kernel panic when dereferencing 
->vendor during boot.

> +		return false;
> +
> +	/* It is the GPU in an APU, force identity domain */
> +	return true;
> +}

[Severity: High]
Does this heuristic accurately distinguish APUs from standard discrete GPUs?

Standard discrete GPUs are typically plugged directly into motherboard PCIe 
slots, which means their upstream bridge is the motherboard's root port 
(e.g., an Intel or AMD root port, not an ATI switch). Since the root port 
vendor is not ATI, this check would fall through and erroneously return true, 
forcing standard discrete GPUs into the identity domain.

[ ... ]

> -	/*
> -	 * 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;
>  	}

[Severity: High]
Since the amd_iommu_snp_en check was dropped, how does this affect discrete 
GPUs on SNP-enabled systems?

If standard discrete GPUs are incorrectly identified as APUs by 
quirks_force_identity_mapping(), they will be unconditionally forced into 
IOMMU_DOMAIN_IDENTITY here when SME is disabled. 

On systems with SNP enabled, amd_iommu_identity_attach() will reject the 
identity domain if Mode0 isn't supported, returning -EINVAL. Could this leave 
the discrete GPU without a valid domain and break DMA entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824085821.5422-1-vasant.hegde@amd.com?part=1