[PATCH 0/3] Disable IOMMU Passthrough when SME is active

[PATCH 0/3] Disable IOMMU Passthrough when SME is active

[Joerg Roedel]

Hi,

here is a small patch-set to disable IOMMU Passthrough mode when SME is
active even when CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y is set.

The reason for that change is that SME with passthrough mode turned out
to be fragile with devices requiring SWIOTLB, mainly because SWIOTLB has
a maximum allocation size of 256kb and a limit overall size of the
bounce buffer.

Therefore having IOMMU in translation mode by default is better when SME
is active on a system.

Please review.

Thanks,

	Joerg

Joerg Roedel (3):
  iommu: Print default domain type on boot
  iommu: Set default domain type at runtime
  iommu: Disable passthrough mode when SME is active

 drivers/iommu/iommu.c | 48 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

-- 
2.17.1

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

[PATCH 1/3] iommu: Print default domain type on boot

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Introduce a subsys_initcall for IOMMU code and use it to
print the default domain type at boot.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0c674d80c37f..b57ce00c1310 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -80,12 +80,40 @@ struct iommu_group_attribute iommu_group_attr_##_name =		\
 static LIST_HEAD(iommu_device_list);
 static DEFINE_SPINLOCK(iommu_device_lock);
 
+/*
+ * Use a function instead of an array here because the domain-type is a
+ * bit-field, so an array would waste memory.
+ */
+static const char *iommu_domain_type_str(unsigned int t)
+{
+	switch (t) {
+		case IOMMU_DOMAIN_BLOCKED:
+			return "Blocked";
+		case IOMMU_DOMAIN_IDENTITY:
+			return "Passthrough";
+		case IOMMU_DOMAIN_UNMANAGED:
+			return "Unmanaged";
+		case IOMMU_DOMAIN_DMA:
+			return "Translated";
+		default:
+			return "Unknown";
+	}
+}
+
+static int __init iommu_subsys_init(void)
+{
+	pr_info("Default domain type: %s\n",
+		iommu_domain_type_str(iommu_def_domain_type));
+
+	return 0;
+}
+subsys_initcall(iommu_subsys_init);
+
 int iommu_device_register(struct iommu_device *iommu)
 {
 	spin_lock(&iommu_device_lock);
 	list_add_tail(&iommu->list, &iommu_device_list);
 	spin_unlock(&iommu_device_lock);
-
 	return 0;
 }
 
-- 
2.17.1

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

[PATCH 2/3] iommu: Set default domain type at runtime

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Set the default domain-type at runtime, not at compile-time.
This keeps default domain type setting in one place when we
have to change it at runtime.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b57ce00c1310..62cae6db0970 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -26,11 +26,8 @@
 
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
-#else
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
-#endif
+
+static unsigned int iommu_def_domain_type __read_mostly;
 static bool iommu_dma_strict __read_mostly = true;
 
 struct iommu_group {
@@ -102,6 +99,11 @@ static const char *iommu_domain_type_str(unsigned int t)
 
 static int __init iommu_subsys_init(void)
 {
+	if (IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH))
+		iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+	else
+		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

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

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

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

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 <jroedel@suse.de>
---
 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

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

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

[Lendacky, Thomas]

On 8/9/19 10:22 AM, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> 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 <jroedel@suse.de>
> ---
>   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));
>   
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Joerg Roedel]

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

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