2019-08-09 15:23:34

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/3] iommu: Disable passthrough mode when SME is active

From: Joerg Roedel <[email protected]>

Using Passthrough mode when SME is active causes certain
devices to use the SWIOTLB bounce buffer. The bounce buffer
code has an upper limit of 256kb for the size of DMA
allocations, which is too small for certain devices and
causes them to fail.

With this patch we enable IOMMU by default when SME is
active in the system, making the default configuration work
for more systems than it does now.

Users that don't want IOMMUs to be enabled still can disable
them with kernel parameters.

Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/iommu.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 62cae6db0970..fbe1aa51bce9 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -104,6 +104,12 @@ static int __init iommu_subsys_init(void)
else
iommu_def_domain_type = IOMMU_DOMAIN_DMA;

+ if ((iommu_def_domain_type == IOMMU_DOMAIN_IDENTITY) &&
+ sme_active()) {
+ pr_info("SME detected - Disabling default IOMMU passthrough\n");
+ iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+ }
+
pr_info("Default domain type: %s\n",
iommu_domain_type_str(iommu_def_domain_type));

--
2.17.1


2019-08-09 18:17:35

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu: Disable passthrough mode when SME is active

On 8/9/19 10:22 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Using Passthrough mode when SME is active causes certain
> devices to use the SWIOTLB bounce buffer. The bounce buffer
> code has an upper limit of 256kb for the size of DMA
> allocations, which is too small for certain devices and
> causes them to fail.
>
> With this patch we enable IOMMU by default when SME is
> active in the system, making the default configuration work
> for more systems than it does now.
>
> Users that don't want IOMMUs to be enabled still can disable
> them with kernel parameters.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/iommu.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 62cae6db0970..fbe1aa51bce9 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -104,6 +104,12 @@ static int __init iommu_subsys_init(void)
> else
> iommu_def_domain_type = IOMMU_DOMAIN_DMA;
>
> + if ((iommu_def_domain_type == IOMMU_DOMAIN_IDENTITY) &&
> + sme_active()) {
> + pr_info("SME detected - Disabling default IOMMU passthrough\n");
> + iommu_def_domain_type = IOMMU_DOMAIN_DMA;

Should this also clear the iommu_pass_through variable (the one set by the
iommu kernel parameter in arch/x86/kernel/pci-dma.c)?

I guess this is more applicable to the original patchset that created the
CONFIG_IOMMU_DEFAULT_PASSTHROUGH option, but should the default
passthrough support be modified so that you don't have to specify multiple
kernel parameters to change it?

Right now, if CONFIG_IOMMU_DEFAULT_PASSTHROUGH is set to yes, you can't
just specify iommu=nopt to enable the IOMMU. You have to also specify
iommu.passthrough=0. Do we want to fix that so that just specifying
iommu=nopt or iommu.passthrough=0 does what is needed?

Thanks,
Tom

> + }
> +
> pr_info("Default domain type: %s\n",
> iommu_domain_type_str(iommu_def_domain_type));
>
>

2019-08-09 20:33:04

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu: Disable passthrough mode when SME is active

Hey Tom,

On Fri, Aug 09, 2019 at 04:50:48PM +0000, Lendacky, Thomas wrote:
> On 8/9/19 10:22 AM, Joerg Roedel wrote:
> > + if ((iommu_def_domain_type == IOMMU_DOMAIN_IDENTITY) &&
> > + sme_active()) {
> > + pr_info("SME detected - Disabling default IOMMU passthrough\n");
> > + iommu_def_domain_type = IOMMU_DOMAIN_DMA;
>
> Should this also clear the iommu_pass_through variable (the one set by the
> iommu kernel parameter in arch/x86/kernel/pci-dma.c)?

This code is used on different architectures, so I can't cleanly access
architecture specific variables here.

> I guess this is more applicable to the original patchset that created the
> CONFIG_IOMMU_DEFAULT_PASSTHROUGH option, but should the default
> passthrough support be modified so that you don't have to specify multiple
> kernel parameters to change it?
>
> Right now, if CONFIG_IOMMU_DEFAULT_PASSTHROUGH is set to yes, you can't
> just specify iommu=nopt to enable the IOMMU. You have to also specify
> iommu.passthrough=0. Do we want to fix that so that just specifying
> iommu=nopt or iommu.passthrough=0 does what is needed?

Yeah, that is currently a mess and I think cleaning that up is at least
partly in the scope of this patch-set. I'll look into that next week.


Regards,

Joerg