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

2022-04-21 Thread Limonciello, Mario via iommu
[AMD Official Use Only]

> On Wed, Apr 06, 2022 at 05:04:52PM +, 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: sta...@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


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

2022-04-07 Thread Christoph Hellwig
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


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

2022-04-07 Thread Robin Murphy

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 


Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Suggested-by: Christoph Hellwig 
Signed-off-by: Mario Limonciello 
---
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


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

2022-04-07 Thread Christoph Hellwig
On Wed, Apr 06, 2022 at 05:04:52PM +, 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: sta...@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


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

2022-04-06 Thread Limonciello, Mario via iommu
[Public]



> -Original Message-
> From: Christoph Hellwig 
> Sent: Monday, April 4, 2022 23:34
> To: Limonciello, Mario 
> Cc: Joerg Roedel ; Will Deacon ;
> Christoph Hellwig ; Marek Szyprowski
> ; Robin Murphy ;
> open list:IOMMU DRIVERS ;
> Suthikulpanit, Suravee ; Hegde, Vasant
> ; open list 
> Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig 

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

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


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

2022-04-04 Thread Christoph Hellwig
Looks good:

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2022-04-04 Thread Mario Limonciello via iommu
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 
Signed-off-by: Mario Limonciello 
---
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