All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU
@ 2022-04-04 20:47 ` Mario Limonciello
  0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello via iommu @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Mario Limonciello, Robin Murphy

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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU
@ 2022-04-04 20:47 ` Mario Limonciello
  0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit Suravee, Hegde Vasant,
	open list, Mario Limonciello

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-04 20:47 ` Mario Limonciello
@ 2022-04-04 20:47   ` Mario Limonciello
  -1 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello via iommu @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Mario Limonciello, Robin Murphy

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 <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;
-- 
2.34.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-04 20:47   ` Mario Limonciello
  0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit Suravee, Hegde Vasant,
	open list, Mario Limonciello

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 <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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
  2022-04-04 20:47 ` Mario Limonciello
@ 2022-04-04 20:47   ` Mario Limonciello
  -1 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello via iommu @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Mario Limonciello, Robin Murphy

If the IOMMU is in use and an untrusted device is connected to an external
facing port but the address requested isn't page aligned will cause the
kernel to attempt to use bounce buffers.

If for some reason the bounce buffers have not been allocated this is a
problem that should be made apparent to the user.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Move error message into the caller

 drivers/iommu/dma-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 09f6e1c0f9c0..1ca85d37eeab 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -971,6 +971,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
 		void *padding_start;
 		size_t padding_size, aligned_size;
 
+		if (!is_swiotlb_active(dev)) {
+			dev_warn_once(dev, "DMA bounce buffers are inactive, unable to map unaligned transaction.\n");
+			return DMA_MAPPING_ERROR;
+		}
+
 		aligned_size = iova_align(iovad, size);
 		phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
 					      iova_mask(iovad), dir, attrs);
-- 
2.34.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
@ 2022-04-04 20:47   ` Mario Limonciello
  0 siblings, 0 replies; 24+ messages in thread
From: Mario Limonciello @ 2022-04-04 20:47 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit Suravee, Hegde Vasant,
	open list, Mario Limonciello

If the IOMMU is in use and an untrusted device is connected to an external
facing port but the address requested isn't page aligned will cause the
kernel to attempt to use bounce buffers.

If for some reason the bounce buffers have not been allocated this is a
problem that should be made apparent to the user.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Move error message into the caller

 drivers/iommu/dma-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 09f6e1c0f9c0..1ca85d37eeab 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -971,6 +971,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
 		void *padding_start;
 		size_t padding_size, aligned_size;
 
+		if (!is_swiotlb_active(dev)) {
+			dev_warn_once(dev, "DMA bounce buffers are inactive, unable to map unaligned transaction.\n");
+			return DMA_MAPPING_ERROR;
+		}
+
 		aligned_size = iova_align(iovad, size);
 		phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
 					      iova_mask(iovad), dir, attrs);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-04 20:47   ` Mario Limonciello
@ 2022-04-05  4:34     ` Christoph Hellwig
  -1 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-05  4:34 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Joerg Roedel, Will Deacon, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, open list:IOMMU DRIVERS, Suthikulpanit Suravee,
	Hegde Vasant, open list

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-05  4:34     ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-05  4:34 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Will Deacon, Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Robin Murphy

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
  2022-04-04 20:47   ` Mario Limonciello
@ 2022-04-05  4:34     ` Christoph Hellwig
  -1 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-05  4:34 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Will Deacon, Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Robin Murphy

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
@ 2022-04-05  4:34     ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-05  4:34 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Joerg Roedel, Will Deacon, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, open list:IOMMU DRIVERS, Suthikulpanit Suravee,
	Hegde Vasant, open list

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-05  4:34     ` Christoph Hellwig
@ 2022-04-06 17:04       ` Limonciello, Mario
  -1 siblings, 0 replies; 24+ messages in thread
From: Limonciello, Mario via iommu @ 2022-04-06 17:04 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Will Deacon, Hegde, Vasant, open list, open list:IOMMU DRIVERS,
	Robin Murphy

[Public]



> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Monday, April 4, 2022 23:34
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Joerg Roedel <joro@8bytes.org>; Will Deacon <will@kernel.org>;
> Christoph Hellwig <hch@infradead.org>; Marek Szyprowski
> <m.szyprowski@samsung.com>; Robin Murphy <robin.murphy@arm.com>;
> open list:IOMMU DRIVERS <iommu@lists.linux-foundation.org>;
> Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Hegde, Vasant
> <Vasant.Hegde@amd.com>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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: stable@vger.kernel.org # 5.11+

As well.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-06 17:04       ` Limonciello, Mario
  0 siblings, 0 replies; 24+ messages in thread
From: Limonciello, Mario @ 2022-04-06 17:04 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Joerg Roedel, Will Deacon, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit, Suravee, Hegde, Vasant,
	open list

[Public]



> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Monday, April 4, 2022 23:34
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Joerg Roedel <joro@8bytes.org>; Will Deacon <will@kernel.org>;
> Christoph Hellwig <hch@infradead.org>; Marek Szyprowski
> <m.szyprowski@samsung.com>; Robin Murphy <robin.murphy@arm.com>;
> open list:IOMMU DRIVERS <iommu@lists.linux-foundation.org>;
> Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Hegde, Vasant
> <Vasant.Hegde@amd.com>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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: stable@vger.kernel.org # 5.11+

As well.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-06 17:04       ` Limonciello, Mario
@ 2022-04-07  7:58         ` Christoph Hellwig
  -1 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-07  7:58 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Christoph Hellwig, Joerg Roedel, Will Deacon, Marek Szyprowski,
	Robin Murphy, open list:IOMMU DRIVERS, Suthikulpanit, Suravee,
	Hegde, Vasant, open list

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: stable@vger.kernel.org # 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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-07  7:58         ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-07  7:58 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Will Deacon, Hegde, Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Robin Murphy

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: stable@vger.kernel.org # 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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
  2022-04-04 20:47   ` Mario Limonciello
@ 2022-04-07 13:07     ` Robin Murphy
  -1 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2022-04-07 13:07 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Will Deacon
  Cc: Hegde Vasant, open list, Christoph Hellwig, open list:IOMMU DRIVERS

On 2022-04-04 21:47, Mario Limonciello via iommu wrote:
> If the IOMMU is in use and an untrusted device is connected to an external
> facing port but the address requested isn't page aligned will cause the
> kernel to attempt to use bounce buffers.
> 
> If for some reason the bounce buffers have not been allocated this is a
> problem that should be made apparent to the user.

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

> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Move error message into the caller
> 
>   drivers/iommu/dma-iommu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 09f6e1c0f9c0..1ca85d37eeab 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -971,6 +971,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
>   		void *padding_start;
>   		size_t padding_size, aligned_size;
>   
> +		if (!is_swiotlb_active(dev)) {
> +			dev_warn_once(dev, "DMA bounce buffers are inactive, unable to map unaligned transaction.\n");
> +			return DMA_MAPPING_ERROR;
> +		}
> +
>   		aligned_size = iova_align(iovad, size);
>   		phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
>   					      iova_mask(iovad), dir, attrs);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it
@ 2022-04-07 13:07     ` Robin Murphy
  0 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2022-04-07 13:07 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Will Deacon
  Cc: Christoph Hellwig, open list:IOMMU DRIVERS, Hegde Vasant, open list

On 2022-04-04 21:47, Mario Limonciello via iommu wrote:
> If the IOMMU is in use and an untrusted device is connected to an external
> facing port but the address requested isn't page aligned will cause the
> kernel to attempt to use bounce buffers.
> 
> If for some reason the bounce buffers have not been allocated this is a
> problem that should be made apparent to the user.

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

> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Move error message into the caller
> 
>   drivers/iommu/dma-iommu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 09f6e1c0f9c0..1ca85d37eeab 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -971,6 +971,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
>   		void *padding_start;
>   		size_t padding_size, aligned_size;
>   
> +		if (!is_swiotlb_active(dev)) {
> +			dev_warn_once(dev, "DMA bounce buffers are inactive, unable to map unaligned transaction.\n");
> +			return DMA_MAPPING_ERROR;
> +		}
> +
>   		aligned_size = iova_align(iovad, size);
>   		phys = swiotlb_tbl_map_single(dev, phys, size, aligned_size,
>   					      iova_mask(iovad), dir, attrs);
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-04 20:47   ` Mario Limonciello
@ 2022-04-07 13:31     ` Robin Murphy
  -1 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2022-04-07 13:31 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Will Deacon
  Cc: Christoph Hellwig, Marek Szyprowski, open list:IOMMU DRIVERS,
	Suthikulpanit Suravee, Hegde Vasant, open list

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;

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-07 13:31     ` Robin Murphy
  0 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2022-04-07 13:31 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Will Deacon
  Cc: open list, Hegde Vasant, Christoph Hellwig, open list:IOMMU DRIVERS

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-07 13:31     ` Robin Murphy
@ 2022-04-07 13:41       ` Christoph Hellwig
  -1 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-07 13:41 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mario Limonciello, Joerg Roedel, Will Deacon, Christoph Hellwig,
	Marek Szyprowski, open list:IOMMU DRIVERS, Suthikulpanit Suravee,
	Hegde Vasant, open list

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-07 13:41       ` Christoph Hellwig
  0 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2022-04-07 13:41 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Mario Limonciello, Will Deacon

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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
  2022-04-07  7:58         ` Christoph Hellwig
@ 2022-04-21 14:44           ` Limonciello, Mario via iommu
  -1 siblings, 0 replies; 24+ messages in thread
From: Limonciello, Mario @ 2022-04-21 14:44 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Will Deacon, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit, Suravee, Hegde, Vasant,
	open list, Christoph Hellwig

[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: stable@vger.kernel.org # 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,

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
@ 2022-04-21 14:44           ` Limonciello, Mario via iommu
  0 siblings, 0 replies; 24+ messages in thread
From: Limonciello, Mario via iommu @ 2022-04-21 14:44 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Will Deacon, Hegde, Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Robin Murphy

[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: stable@vger.kernel.org # 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,
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU
  2022-04-04 20:47 ` Mario Limonciello
@ 2022-04-28  8:27   ` Joerg Roedel
  -1 siblings, 0 replies; 24+ messages in thread
From: Joerg Roedel @ 2022-04-28  8:27 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Will Deacon, Christoph Hellwig, Marek Szyprowski, Robin Murphy,
	open list:IOMMU DRIVERS, Suthikulpanit Suravee, Hegde Vasant,
	open list

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU
@ 2022-04-28  8:27   ` Joerg Roedel
  0 siblings, 0 replies; 24+ messages in thread
From: Joerg Roedel @ 2022-04-28  8:27 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Will Deacon, Hegde Vasant, open list, Christoph Hellwig,
	open list:IOMMU DRIVERS, Robin Murphy

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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-04-28  8:27 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 20:47 [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU Mario Limonciello via iommu
2022-04-04 20:47 ` Mario Limonciello
2022-04-04 20:47 ` [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases Mario Limonciello via iommu
2022-04-04 20:47   ` Mario Limonciello
2022-04-05  4:34   ` Christoph Hellwig
2022-04-05  4:34     ` Christoph Hellwig
2022-04-06 17:04     ` Limonciello, Mario via iommu
2022-04-06 17:04       ` Limonciello, Mario
2022-04-07  7:58       ` Christoph Hellwig
2022-04-07  7:58         ` Christoph Hellwig
2022-04-21 14:44         ` Limonciello, Mario
2022-04-21 14:44           ` Limonciello, Mario via iommu
2022-04-07 13:31   ` Robin Murphy
2022-04-07 13:31     ` Robin Murphy
2022-04-07 13:41     ` Christoph Hellwig
2022-04-07 13:41       ` Christoph Hellwig
2022-04-04 20:47 ` [PATCH v2 2/2] dma-iommu: Check that swiotlb is active before trying to use it Mario Limonciello via iommu
2022-04-04 20:47   ` Mario Limonciello
2022-04-05  4:34   ` Christoph Hellwig
2022-04-05  4:34     ` Christoph Hellwig
2022-04-07 13:07   ` Robin Murphy
2022-04-07 13:07     ` Robin Murphy
2022-04-28  8:27 ` [PATCH v2 0/2] Fix issues with untrusted devices and AMD IOMMU Joerg Roedel
2022-04-28  8:27   ` Joerg Roedel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.