[PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Vasant Hegde]

Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain for
untrusted device. This always took precedence over driver's
def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
iommu_get_default_domain_type() to respect def_domain_type()") changed
the behaviour. Current code calls def_domain_type() but if it doesn't
return IOMMU_DOMAIN_DMA for untrusted device it throws error. This
results in IOMMU group (and potentially IOMMU itself) in undetermined
sstate.

This patch adds untrusted check in AMD IOMMU driver code. So that it
lets eGPUs behind Thunderbolt work again.

Fine tuning amd_iommu_def_domain_type() will be done later.

Reported-by: Eric Wagner <ewagner12@gmail.com>
Link: https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
Fixes: 59ddce4418da ("iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()")
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: stable@kernel.org # v6.7+
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
@Eric,
  Can you please test this patch?

-Vasant

 drivers/iommu/amd/iommu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index e692217fcb28..e405714640b0 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2754,6 +2754,10 @@ static int amd_iommu_def_domain_type(struct device *dev)
 	if (!dev_data)
 		return 0;
 
+	/* Always use DMA domain for untrusted device */
+	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
-- 
2.31.1

Re: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Jason Gunthorpe]

On Fri, Apr 19, 2024 at 11:47:15AM +0000, Vasant Hegde wrote:
> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain for
> untrusted device. This always took precedence over driver's
> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
> iommu_get_default_domain_type() to respect def_domain_type()") changed
> the behaviour. Current code calls def_domain_type() but if it doesn't
> return IOMMU_DOMAIN_DMA for untrusted device it throws error. This
> results in IOMMU group (and potentially IOMMU itself) in undetermined
> sstate.
> 
> This patch adds untrusted check in AMD IOMMU driver code. So that it
> lets eGPUs behind Thunderbolt work again.
> 
> Fine tuning amd_iommu_def_domain_type() will be done later.

As long as this is done, and with the commitment that identity rid
with pasid will be eventually supported as well

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

RE: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> Sent: Friday, April 19, 2024 7:47 AM
> To: iommu@lists.linux.dev; joro@8bytes.org
> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Hegde, Vasant
> <Vasant.Hegde@amd.com>; Eric Wagner <ewagner12@gmail.com>; Robin
> Murphy <robin.murphy@arm.com>; Jason Gunthorpe <jgg@ziepe.ca>;
> stable@kernel.org
> Subject: [PATCH] iommu/amd: Enhance def_domain_type to handle
> untrusted device
>
> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain
> for untrusted device. This always took precedence over driver's
> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
> iommu_get_default_domain_type() to respect def_domain_type()") changed
> the behaviour. Current code calls def_domain_type() but if it doesn't return
> IOMMU_DOMAIN_DMA for untrusted device it throws error. This results in
> IOMMU group (and potentially IOMMU itself) in undetermined sstate.
>
> This patch adds untrusted check in AMD IOMMU driver code. So that it lets
> eGPUs behind Thunderbolt work again.
>
> Fine tuning amd_iommu_def_domain_type() will be done later.
>
> Reported-by: Eric Wagner <ewagner12@gmail.com>
> Link: https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-
> v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
> Fixes: 59ddce4418da ("iommu: Reorganize
> iommu_get_default_domain_type() to respect def_domain_type()")
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: stable@kernel.org # v6.7+
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Assuming it fixes the issue, please add:
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3182

Alex

> ---
> @Eric,
>   Can you please test this patch?
>
> -Vasant
>
>  drivers/iommu/amd/iommu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index e692217fcb28..e405714640b0 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2754,6 +2754,10 @@ static int amd_iommu_def_domain_type(struct
> device *dev)
>       if (!dev_data)
>               return 0;
>
> +     /* Always use DMA domain for untrusted device */
> +     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
> --
> 2.31.1

Re: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Vasant Hegde]

Alex,


On 4/19/2024 8:54 PM, Deucher, Alexander wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Hegde, Vasant <Vasant.Hegde@amd.com>
>> Sent: Friday, April 19, 2024 7:47 AM
>> To: iommu@lists.linux.dev; joro@8bytes.org
>> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Deucher,
>> Alexander <Alexander.Deucher@amd.com>; Hegde, Vasant
>> <Vasant.Hegde@amd.com>; Eric Wagner <ewagner12@gmail.com>; Robin
>> Murphy <robin.murphy@arm.com>; Jason Gunthorpe <jgg@ziepe.ca>;
>> stable@kernel.org
>> Subject: [PATCH] iommu/amd: Enhance def_domain_type to handle
>> untrusted device
>>
>> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain
>> for untrusted device. This always took precedence over driver's
>> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
>> iommu_get_default_domain_type() to respect def_domain_type()") changed
>> the behaviour. Current code calls def_domain_type() but if it doesn't return
>> IOMMU_DOMAIN_DMA for untrusted device it throws error. This results in
>> IOMMU group (and potentially IOMMU itself) in undetermined sstate.
>>
>> This patch adds untrusted check in AMD IOMMU driver code. So that it lets
>> eGPUs behind Thunderbolt work again.
>>
>> Fine tuning amd_iommu_def_domain_type() will be done later.
>>
>> Reported-by: Eric Wagner <ewagner12@gmail.com>
>> Link: https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-
>> v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
>> Fixes: 59ddce4418da ("iommu: Reorganize
>> iommu_get_default_domain_type() to respect def_domain_type()")
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>> Cc: stable@kernel.org # v6.7+
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> 
> Assuming it fixes the issue, please add:
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3182

Sure. Will add it in v2.

-Vasant

Re: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Eric Wagner]

On Fri, Apr 19, 2024 at 7:47 AM Vasant Hegde <vasant.hegde@amd.com> wrote:
>
> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain for
> untrusted device. This always took precedence over driver's
> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
> iommu_get_default_domain_type() to respect def_domain_type()") changed
> the behaviour. Current code calls def_domain_type() but if it doesn't
> return IOMMU_DOMAIN_DMA for untrusted device it throws error. This
> results in IOMMU group (and potentially IOMMU itself) in undetermined
> sstate.
>
> This patch adds untrusted check in AMD IOMMU driver code. So that it
> lets eGPUs behind Thunderbolt work again.
>
> Fine tuning amd_iommu_def_domain_type() will be done later.
>
> Reported-by: Eric Wagner <ewagner12@gmail.com>
> Link: https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
> Fixes: 59ddce4418da ("iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()")
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: stable@kernel.org # v6.7+
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
> @Eric,
>   Can you please test this patch?
>
> -Vasant
>
>  drivers/iommu/amd/iommu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index e692217fcb28..e405714640b0 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2754,6 +2754,10 @@ static int amd_iommu_def_domain_type(struct device *dev)
>         if (!dev_data)
>                 return 0;
>
> +       /* Always use DMA domain for untrusted device */
> +       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
> --
> 2.31.1
>

Tested the patch and confirmed it fixes the reported issue for me.

Tested-by: Eric Wagner <ewagner12@gmail.com>

Re: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Baolu Lu]

On 2024/4/19 19:47, Vasant Hegde wrote:
> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain for
> untrusted device. This always took precedence over driver's
> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
> iommu_get_default_domain_type() to respect def_domain_type()") changed
> the behaviour. Current code calls def_domain_type() but if it doesn't
> return IOMMU_DOMAIN_DMA for untrusted device it throws error. This
> results in IOMMU group (and potentially IOMMU itself) in undetermined
> sstate.
> 
> This patch adds untrusted check in AMD IOMMU driver code. So that it
> lets eGPUs behind Thunderbolt work again.
> 
> Fine tuning amd_iommu_def_domain_type() will be done later.
> 
> Reported-by: Eric Wagner<ewagner12@gmail.com>
> Link:https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
> Fixes: 59ddce4418da ("iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()")
> Cc: Robin Murphy<robin.murphy@arm.com>
> Cc: Jason Gunthorpe<jgg@ziepe.ca>
> Cc:stable@kernel.org  # v6.7+
> Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
> ---
> @Eric,
>    Can you please test this patch?
> 
> -Vasant
> 
>   drivers/iommu/amd/iommu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index e692217fcb28..e405714640b0 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2754,6 +2754,10 @@ static int amd_iommu_def_domain_type(struct device *dev)
>   	if (!dev_data)
>   		return 0;
>   
> +	/* Always use DMA domain for untrusted device */
> +	if (to_pci_dev(dev)->untrusted)
> +		return IOMMU_DOMAIN_DMA;

Are you sure that dev always represents a pci device? If not, perhaps
here it needs a dev_is_pci() check?

Best regards,
baolu

Re: [PATCH] iommu/amd: Enhance def_domain_type to handle untrusted device

[Vasant Hegde]

Hi,


On 4/21/2024 7:15 AM, Baolu Lu wrote:
> On 2024/4/19 19:47, Vasant Hegde wrote:
>> Previously, IOMMU core layer was forcing IOMMU_DOMAIN_DMA domain for
>> untrusted device. This always took precedence over driver's
>> def_domain_type(). Commit 59ddce4418da ("iommu: Reorganize
>> iommu_get_default_domain_type() to respect def_domain_type()") changed
>> the behaviour. Current code calls def_domain_type() but if it doesn't
>> return IOMMU_DOMAIN_DMA for untrusted device it throws error. This
>> results in IOMMU group (and potentially IOMMU itself) in undetermined
>> sstate.
>>
>> This patch adds untrusted check in AMD IOMMU driver code. So that it
>> lets eGPUs behind Thunderbolt work again.
>>
>> Fine tuning amd_iommu_def_domain_type() will be done later.
>>
>> Reported-by: Eric Wagner<ewagner12@gmail.com>
>> Link:https://lore.kernel.org/linux-iommu/CAHudX3zLH6CsRmLE-yb+gRjhh-v4bU5_1jW_xCcxOo_oUUZKYg@mail.gmail.com
>> Fixes: 59ddce4418da ("iommu: Reorganize iommu_get_default_domain_type() to
>> respect def_domain_type()")
>> Cc: Robin Murphy<robin.murphy@arm.com>
>> Cc: Jason Gunthorpe<jgg@ziepe.ca>
>> Cc:stable@kernel.org  # v6.7+
>> Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
>> ---
>> @Eric,
>>    Can you please test this patch?
>>
>> -Vasant
>>
>>   drivers/iommu/amd/iommu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index e692217fcb28..e405714640b0 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2754,6 +2754,10 @@ static int amd_iommu_def_domain_type(struct device *dev)
>>       if (!dev_data)
>>           return 0;
>>   +    /* Always use DMA domain for untrusted device */
>> +    if (to_pci_dev(dev)->untrusted)
>> +        return IOMMU_DOMAIN_DMA;
> 
> Are you sure that dev always represents a pci device? If not, perhaps
> here it needs a dev_is_pci() check?

You are right. I need to add check to verify whether its PCI device or not.


-Vasant