Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases

From: Robin Murphy <robin.murphy@arm.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
	Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>
Cc: open list <linux-kernel@vger.kernel.org>,
	Hegde Vasant <Vasant.Hegde@amd.com>,
	Christoph Hellwig <hch@infradead.org>,
	"open list:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>
Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
Date: Thu, 7 Apr 2022 14:31:44 +0100	[thread overview]
Message-ID: <f7039f1f-81ef-2fab-4ade-728316b54942@arm.com> (raw)
In-Reply-To: <20220404204723.9767-2-mario.limonciello@amd.com>

On 2022-04-04 21:47, Mario Limonciello wrote:
> Previously the AMD IOMMU would only enable SWIOTLB in certain
> circumstances:
>   * IOMMU in passthrough mode
>   * SME enabled
> 
> This logic however doesn't work when an untrusted device is plugged in
> that doesn't do page aligned DMA transactions.  The expectation is
> that a bounce buffer is used for those transactions.
> 
> This fails like this:
> 
> swiotlb buffer is full (sz: 4096 bytes), total 0 (slots), used 0 (slots)
> 
> That happens because the bounce buffers have been allocated, followed by
> freed during startup but the bounce buffering code expects that all IOMMUs
> have left it enabled.
> 
> Remove the criteria to set up bounce buffers on AMD systems to ensure
> they're always available for supporting untrusted devices.

FWIW it's also broken for another niche case where 
iommu_default_passthrough() == false at init, but the user later changes 
a 32-bit device's default domain type to passthrough via sysfs, such 
that it starts needing regular dma-direct bouncing.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Enable swiotlb for AMD instead of ignoring it for inactive
> 
>   drivers/iommu/amd/iommu.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a1ada7bff44e..079694f894b8 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1838,17 +1838,10 @@ void amd_iommu_domain_update(struct protection_domain *domain)
>   	amd_iommu_domain_flush_complete(domain);
>   }
>   
> -static void __init amd_iommu_init_dma_ops(void)
> -{
> -	swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
> -}
> -
>   int __init amd_iommu_init_api(void)
>   {
>   	int err;
>   
> -	amd_iommu_init_dma_ops();
> -
>   	err = bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
>   	if (err)
>   		return err;
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu