* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 6:15 ` [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only Vasant Hegde
@ 2026-07-23 6:25 ` sashiko-bot
2026-07-23 15:53 ` Ankit Soni
2026-07-23 15:59 ` Bjorn Helgaas
2 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-23 6:25 UTC (permalink / raw)
To: Vasant Hegde; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] quirks_force_identity_mapping() incorrectly identifies AMD discrete GPUs (dGPUs) as APUs, inadvertently forcing them into IOMMU IDENTITY mapping mode instead of the intended DMA mode.
--
--- Patch [2]: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only ---
commit 43e3d4caff8faca1297d432e0146e34c6ec99227
Author: Vasant Hegde <vasant.hegde@amd.com>
Subject: iommu/amd: Force identity mode for selected GPUs only
This commit attempts to enforce identity mapping mode specifically for APUs
while keeping discrete GPUs in DMA mode by checking the upstream bridge
vendor ID.
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 1e76386e5eb11..a9c7209f1b587 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
[ ... ]
> @@ -3122,6 +3122,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;
[Severity: High]
If an APU is implemented as a Root Complex Integrated Endpoint (RCiEP),
pcie_find_root_port() will return NULL. Does this cause the function
to return false early and leave the APU in DMA mode, which seems to be the
opposite of the intended behavior?
> +
> + /* If bridge vendor is not ATI then its APU and force IDENTITY mode */
> + if (root_port->vendor != PCI_VENDOR_ID_ATI)
> + return true;
[Severity: High]
Will this correctly distinguish between discrete GPUs and APUs?
Since pcie_find_root_port() traces up to the host's root port, the
vendor ID will typically match the system chipset (like AMD 0x1022 or
Intel 0x8086) rather than ATI 0x1002. Doesn't this mean the condition
root_port->vendor != PCI_VENDOR_ID_ATI evaluates to true for discrete
GPUs, forcing them into IDENTITY mode instead of the intended DMA mode?
> +
> + /* Rest all are dGPUs and works fine with DMA mode */
> + return false;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723061548.10187-1-vasant.hegde@amd.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 6:15 ` [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only Vasant Hegde
2026-07-23 6:25 ` sashiko-bot
@ 2026-07-23 15:53 ` Ankit Soni
2026-07-23 16:29 ` Vasant Hegde
2026-07-23 15:59 ` Bjorn Helgaas
2 siblings, 1 reply; 16+ messages in thread
From: Ankit Soni @ 2026-07-23 15:53 UTC (permalink / raw)
To: Vasant Hegde
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, mario.limonciello, jgg,
Amandeep Kaur Longia
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.
>
> 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 <alexander.deucher@amd.com>
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
> ---
> 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 */
> + if (root_port->vendor != PCI_VENDOR_ID_ATI)
> + return true;
Hi Vasant,
For a discrete GPU, pcie_find_root_port() may walk up to the system root port.
On an AMD platform that root port is vendored PCI_VENDOR_ID_AMD, which is not ATI.
so this device will be forced to identity.
Is that a valid scenario?
-Ankit
> +
> + /* 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;
>
> - /*
> - * 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 15:53 ` Ankit Soni
@ 2026-07-23 16:29 ` Vasant Hegde
0 siblings, 0 replies; 16+ messages in thread
From: Vasant Hegde @ 2026-07-23 16:29 UTC (permalink / raw)
To: Ankit Soni
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, mario.limonciello, jgg,
Amandeep Kaur Longia
Ankit,
On 7/23/2026 9:23 PM, Ankit Soni wrote:
> 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.
>>
>> 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 <alexander.deucher@amd.com>
>> Cc: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
>> ---
>> 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 */
>> + if (root_port->vendor != PCI_VENDOR_ID_ATI)
>> + return true;
>
> Hi Vasant,
>
> For a discrete GPU, pcie_find_root_port() may walk up to the system root port.
> On an AMD platform that root port is vendored PCI_VENDOR_ID_AMD, which is not ATI.
We want to filter based on vendor as ATI only. As Mario pointed out, logic got
inverted. Will fix it in v2.
-Vasant
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 6:15 ` [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only Vasant Hegde
2026-07-23 6:25 ` sashiko-bot
2026-07-23 15:53 ` Ankit Soni
@ 2026-07-23 15:59 ` Bjorn Helgaas
2026-07-23 16:11 ` Mario Limonciello
2026-07-23 16:33 ` Vasant Hegde
2 siblings, 2 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2026-07-23 15:59 UTC (permalink / raw)
To: Vasant Hegde
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, mario.limonciello, jgg,
Amandeep Kaur Longia
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 <alexander.deucher@amd.com>
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 15:59 ` Bjorn Helgaas
@ 2026-07-23 16:11 ` Mario Limonciello
2026-07-23 16:23 ` Vasant Hegde
2026-07-23 16:33 ` Vasant Hegde
1 sibling, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2026-07-23 16:11 UTC (permalink / raw)
To: Bjorn Helgaas, Vasant Hegde
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, jgg, Amandeep Kaur Longia
On 7/23/26 10:59, Bjorn Helgaas wrote:
> 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 <alexander.deucher@amd.com>
>> Cc: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
>> ---
>> 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;
This logic I believe is wrong. You're trying to look at the parent of
the display device (which is an internal PCIe switch for a dGPU).
You basically want a similar implementation to
amdgpu_device_find_parent() which figures out first device outside of
the dGPU.
>> +
>> + /* Rest all are dGPUs and works fine with DMA mode */
>> + return false;
Mostly for code flow, I think it would make sense the
force_identity_mapping fallback is purely for APU.
IE something like this:
if (vendor != PCI_VENDOR_ATI)
return false;
if (class != display)
return false;
if (pci_upstream_bridge()->vendor == PCI_VENDOR_ATI)
return false;
/* rest are APUs, force identity */
return true;
>> +}
>> +
>> 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?
I believe the identity domain mapping is only needed for APUs, which
can't be put into an eGPU enclosure.
>
>> - /*
>> - * 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
>>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 16:11 ` Mario Limonciello
@ 2026-07-23 16:23 ` Vasant Hegde
2026-07-23 16:32 ` Mario Limonciello
0 siblings, 1 reply; 16+ messages in thread
From: Vasant Hegde @ 2026-07-23 16:23 UTC (permalink / raw)
To: Mario Limonciello, Bjorn Helgaas
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, jgg, Amandeep Kaur Longia
Mario,
On 7/23/2026 9:41 PM, Mario Limonciello wrote:
>
>
> On 7/23/26 10:59, Bjorn Helgaas wrote:
>> 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 <alexander.deucher@amd.com>
>>> Cc: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>>> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
>>> ---
>>> 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;
>
> This logic I believe is wrong. You're trying to look at the parent of the
> display device (which is an internal PCIe switch for a dGPU).
You are right. It got inverted while I was fine tuning the code.
>
> You basically want a similar implementation to amdgpu_device_find_parent() which
> figures out first device outside of the dGPU.
Ack.
>
>>> +
>>> + /* Rest all are dGPUs and works fine with DMA mode */
>>> + return false;
>
> Mostly for code flow, I think it would make sense the force_identity_mapping
> fallback is purely for APU.
>
> IE something like this:
>
> if (vendor != PCI_VENDOR_ATI)
> return false;
> if (class != display)
> return false;
> if (pci_upstream_bridge()->vendor == PCI_VENDOR_ATI)
> return false;
>
> /* rest are APUs, force identity */
> return true;
Makes sense. And pci_upstream_bridge() return NULL, then return true? (something
like below) ?
+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;
+
+ if (pci_upstream_bridge(pdev) &&
+ pci_upstream_bridge(pdev)->vendor == PCI_VENDOR_ID_ATI)
+ return false;
+
+ return true;
+}
+
-Vasant
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 16:23 ` Vasant Hegde
@ 2026-07-23 16:32 ` Mario Limonciello
2026-07-23 16:55 ` Vasant Hegde
0 siblings, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2026-07-23 16:32 UTC (permalink / raw)
To: Vasant Hegde, Bjorn Helgaas
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, jgg, Amandeep Kaur Longia
On 7/23/26 11:23, Vasant Hegde wrote:
> Mario,
>
>
> On 7/23/2026 9:41 PM, Mario Limonciello wrote:
>>
>>
>> On 7/23/26 10:59, Bjorn Helgaas wrote:
>>> 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 <alexander.deucher@amd.com>
>>>> Cc: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>>>> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
>>>> ---
>>>> 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;
>>
>> This logic I believe is wrong. You're trying to look at the parent of the
>> display device (which is an internal PCIe switch for a dGPU).
>
> You are right. It got inverted while I was fine tuning the code.
>
>>
>> You basically want a similar implementation to amdgpu_device_find_parent() which
>> figures out first device outside of the dGPU.
>
> Ack.
>
>>
>>>> +
>>>> + /* Rest all are dGPUs and works fine with DMA mode */
>>>> + return false;
>>
>> Mostly for code flow, I think it would make sense the force_identity_mapping
>> fallback is purely for APU.
>>
>> IE something like this:
>>
>> if (vendor != PCI_VENDOR_ATI)
>> return false;
>> if (class != display)
>> return false;
>> if (pci_upstream_bridge()->vendor == PCI_VENDOR_ATI)
>> return false;
>>
>> /* rest are APUs, force identity */
>> return true;
>
> Makes sense. And pci_upstream_bridge() return NULL, then return true? (something
> like below) ?
Bascially; yeah. But one comment below.
>
> +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;
> +
> + if (pci_upstream_bridge(pdev) &&
I don't think you need to check for pci_upstream_bridge() to be
non-NULL. You already checked that it's an endpoint by looking at the
class. So an endpoint will be connected to a bridge of some sort
(either a switch internal to the dGPU or to a root port).
> + pci_upstream_bridge(pdev)->vendor == PCI_VENDOR_ID_ATI)
> + return false;
> +
Make sure you leave a comment here about what true means (/* it is the
GPU in an APU */) so that if we need to add more cases later it's
obvious without having to dig up this thread again.
> + return true;
> +}
> +
>
> -Vasant
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 16:32 ` Mario Limonciello
@ 2026-07-23 16:55 ` Vasant Hegde
0 siblings, 0 replies; 16+ messages in thread
From: Vasant Hegde @ 2026-07-23 16:55 UTC (permalink / raw)
To: Mario Limonciello, Bjorn Helgaas
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, jgg, Amandeep Kaur Longia
Mario,
On 7/23/2026 10:02 PM, Mario Limonciello wrote:
>
>
> On 7/23/26 11:23, Vasant Hegde wrote:
>> Mario,
>>
>>
>> On 7/23/2026 9:41 PM, Mario Limonciello wrote:
>>>
>>>
>>> On 7/23/26 10:59, Bjorn Helgaas wrote:
>>>> 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)
.../...
>>
>> +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;
>> +
>> + if (pci_upstream_bridge(pdev) &&
>
> I don't think you need to check for pci_upstream_bridge() to be non-NULL. You
> already checked that it's an endpoint by looking at the class. So an endpoint
> will be connected to a bridge of some sort (either a switch internal to the dGPU
> or to a root port).
Ok will fix it .
>
>> + pci_upstream_bridge(pdev)->vendor == PCI_VENDOR_ID_ATI)
>> + return false;
>> +
>
> Make sure you leave a comment here about what true means (/* it is the GPU in an
> APU */) so that if we need to add more cases later it's obvious without having
> to dig up this thread again.
Sure.
-Vasant
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] iommu/amd: Force identity mode for selected GPUs only
2026-07-23 15:59 ` Bjorn Helgaas
2026-07-23 16:11 ` Mario Limonciello
@ 2026-07-23 16:33 ` Vasant Hegde
1 sibling, 0 replies; 16+ messages in thread
From: Vasant Hegde @ 2026-07-23 16:33 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: iommu, joro, linux-pci, will, robin.murphy, suravee.suthikulpanit,
bhelgaas, alexander.deucher, mario.limonciello, jgg,
Amandeep Kaur Longia
Bjorn,
On 7/23/2026 9:29 PM, Bjorn Helgaas wrote:
> 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?
I will have to check with Alex for the details. Will check with him.
>
> 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/
Ack. Will fix it in v2.
>
>> 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 <alexander.deucher@amd.com>
>> Cc: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> Tested-by: Amandeep Kaur Longia <AmandeepKaur.Longia@amd.com>
>> ---
>> 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
Ack. Will fix it in v2.
>
>> + 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?
I tried to keep the existing order (untested followed by GPU check). As Mario
commented, we need identity for APU only. So this is fine.
-Vasant
^ permalink raw reply [flat|nested] 16+ messages in thread