lkml.org 
[lkml]   [2024]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] iommu: Fix def_domain_type interaction with untrusted devices
On Thu, Apr 18, 2024 at 05:14:59PM +0530, Vasant Hegde wrote:
> We return IDENTITY only if SNP and memory encryption is not enabled and device
> is SVA capable. Upstream has below code (v6.9-rc4)
>
> if (pdev_pasid_supported(dev_data) &&
> !cc_platform_has(CC_ATTR_MEM_ENCRYPT) &&
> !amd_iommu_snp_en) {
> return IOMMU_DOMAIN_IDENTITY;
> }
>
>
> and during boot, we will enforce Paging domain when Encryption/SNP is enabled.

It is very confusing considering the comment says identity isn't supported:

* 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.
*/

Then it goes on to return IDENTITY anyhow.

If the HW cannot identity map it needs to return DMA for those cases:

/* SNP is enabled, because it prohibits DTE[Mode]=0, IDENTITY is not
* supported */
if (amd_snp_en)
return IOMMU_DOMAIN_DMA;

/* memory encryption is active, because some of those devices
* (AMD GPUs) don't have the encryption bit in their DMA-mask
* and require remapping. IDENTITY is not supported.
*/
if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
return IOMMU_DOMAIN_DMA;


> So far the policy was core layer to handle untrusted device which got changed
> and hence we hit this bug.

Well I would say the AMD driver changed to mis-use def_domain for
policy and that causes problems.

> I don't see any issue with enforcing these checks inside AMD driver.. If we go
> with this approach then IMO core should just adhere to what driver returned
> rather than failing. Having policy decision at two level is inviting trouble
> like this one.

Again, no policy in drivers.

def_domain should return only the HW capability. If it returns
IDENTITY then the driver must fail attaches of PAGING.

You should send a patch fixing this function and remove the PASID test
entirely for now.

Jason

\
 
 \ /
  Last update: 2024-05-27 16:44    [W:0.155 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site