It's been observed that plugging in a TBT3 NVME device to a port marked
with ExternalFacingPort that some DMA transactions occur that are not a
full page and so the DMA API attempts to use software bounce buffers
instead of relying upon the IOMMU translation.
This doesn't work and leads to messaging like:
swiotlb buffer is full (sz: 4096 bytes), total 0 (slots), used 0 (slots)
The bounce buffers were originally set up, but torn down during
the boot process.
* This happens because as part of IOMMU initialization
`amd_iommu_init_dma_ops` gets called and resets the global swiotlb to 0.
* When late_init gets called `pci_swiotlb_late_init` `swiotlb_exit` is
called and the buffers are torn down.
This can be observed in the logs:
```
[ 0.407286] AMD-Vi: Extended features (0x246577efa2254afa): PPR NX GT [5] IA GA PC GA_vAPIC
[ 0.407291] AMD-Vi: Interrupt remapping enabled
[ 0.407292] AMD-Vi: Virtual APIC enabled
[ 0.407872] software IO TLB: tearing down default memory pool
```
This series fixes the behavior of AMD IOMMU to enable swiotlb so that
non-page aligned DMA goes through a bounce buffer.
It also adds a message to help with debugging similar problems in the
future.
Mario Limonciello (2):
iommu/amd: Enable swiotlb in all cases
dma-iommu: Check that swiotlb is active before trying to use it
drivers/iommu/amd/iommu.c | 7 -------
drivers/iommu/dma-iommu.c | 5 +++++
2 files changed, 5 insertions(+), 7 deletions(-)
--
2.34.1
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.
Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Mario Limonciello <[email protected]>
---
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;
--
2.34.1
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
[Public]
> -----Original Message-----
> From: Christoph Hellwig <[email protected]>
> Sent: Monday, April 4, 2022 23:34
> To: Limonciello, Mario <[email protected]>
> Cc: Joerg Roedel <[email protected]>; Will Deacon <[email protected]>;
> Christoph Hellwig <[email protected]>; Marek Szyprowski
> <[email protected]>; Robin Murphy <[email protected]>;
> open list:IOMMU DRIVERS <[email protected]>;
> Suthikulpanit, Suravee <[email protected]>; Hegde, Vasant
> <[email protected]>; open list <[email protected]>
> Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
Thanks!
Considering before this fix effectively swiotlb was turned off on most AMD
systems, when this is picked up I think y'all should consider to add a:
Cc: [email protected] # 5.11+
As well.
On Wed, Apr 06, 2022 at 05:04:52PM +0000, Limonciello, Mario wrote:
> Considering before this fix effectively swiotlb was turned off on most AMD
> systems, when this is picked up I think y'all should consider to add a:
>
> Cc: [email protected] # 5.11+
Agreed. I think this is for Joerg to pick up, and I'd love to see it
picked up soon as I'll have to rebase my
"cleanup swiotlb initialization" series on top of it.
On Thu, Apr 07, 2022 at 02:31:44PM +0100, Robin Murphy wrote:
> 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.
Yeah.
We also have yet another issue: swiotlb is not allocate if there is
no memory outside the 4GB physical address space. I think I can fix
that easily after my swiotlb init series goes in, before that it
would be a bit of a mess spread over all the architectures.
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 <[email protected]>
> Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
> Suggested-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> 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;
[AMD Official Use Only]
> On Wed, Apr 06, 2022 at 05:04:52PM +0000, Limonciello, Mario wrote:
> > Considering before this fix effectively swiotlb was turned off on most AMD
> > systems, when this is picked up I think y'all should consider to add a:
> >
> > Cc: [email protected] # 5.11+
>
> Agreed. I think this is for Joerg to pick up, and I'd love to see it
> picked up soon as I'll have to rebase my
>
> "cleanup swiotlb initialization" series on top of it.
Joerg,
Just want to double check this wasn't lost the last few weeks. I was just checking
iommu.git/next and didn't notice it still.
Thanks,
On Mon, Apr 04, 2022 at 03:47:21PM -0500, Mario Limonciello wrote:
> Mario Limonciello (2):
> iommu/amd: Enable swiotlb in all cases
> dma-iommu: Check that swiotlb is active before trying to use it
>
> drivers/iommu/amd/iommu.c | 7 -------
> drivers/iommu/dma-iommu.c | 5 +++++
> 2 files changed, 5 insertions(+), 7 deletions(-)
Applied to core branch, thanks.