[PATCH] x86/tboot: add an option to disable iommu force on

[PATCH] x86/tboot: add an option to disable iommu force on

[Shaohua Li]

IOMMU harms performance signficantly when we run very fast networking
workloads. It's 40GB networking doing XDP test. Software overhead is
almost unaware, but it's the IOTLB miss (based on our analysis) which
kills the performance. We observed the same performance issue even with
software passthrough (identity mapping), only the hardware passthrough
survives. The pps with iommu (with software passthrough) is only about
~30% of that without it. This is a limitation in hardware based on our
observation, so we'd like to disable the IOMMU force on, but we do want
to use TBOOT and we can sacrifice the DMA security bought by IOMMU. I
must admit I know nothing about TBOOT, but TBOOT guys (cc-ed) think not
eabling IOMMU is totally ok.

So introduce a new boot option to disable the force on. It's kind of
silly we need to run into intel_iommu_init even without force on, but we
need to disable TBOOT PMR registers. For system without the boot option,
nothing is changed.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 arch/x86/kernel/tboot.c                         |  3 +++
 drivers/iommu/intel-iommu.c                     | 21 ++++++++++++++++++++-
 include/linux/dma_remapping.h                   |  1 +
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index facc20a..10c393b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1578,6 +1578,11 @@
 			extended tables themselves, and also PASID support. With
 			this option set, extended tables will not be used even
 			on hardware which claims to support them.
+		tboot_noforce [Default Off]
+			By default, tboot will force Intel IOMMU on, which
+			could harm performance for some workloads even IOMMU
+			identity mapping is enabled. This option will avoid
+			the 'force on' for Intel IOMMU.
 
 	intel_idle.max_cstate=	[KNL,HW,ACPI,X86]
 			0	disables intel_idle and fall back on acpi_idle.
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index b868fa1..edbdfe6 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -510,6 +510,9 @@ int tboot_force_iommu(void)
 	if (!tboot_enabled())
 		return 0;
 
+	if (!intel_iommu_tboot_noforce)
+		return 1;
+
 	if (no_iommu || swiotlb || dmar_disabled)
 		pr_warning("Forcing Intel-IOMMU to enabled\n");
 
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d412a31..fa5d56d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -183,6 +183,7 @@ static int rwbf_quirk;
  * (used when kernel is launched w/ TXT)
  */
 static int force_on = 0;
+int intel_iommu_tboot_noforce;
 
 /*
  * 0: Present
@@ -607,6 +608,10 @@ static int __init intel_iommu_setup(char *str)
 				"Intel-IOMMU: enable pre-production PASID support\n");
 			intel_iommu_pasid28 = 1;
 			iommu_identity_mapping |= IDENTMAP_GFX;
+		} else if (!strncmp(str, "tboot_noforce", 13)) {
+			 printk(KERN_INFO
+				"Intel-IOMMU: not forcing on after tboot. This could expose security risk for tboot\n");
+			intel_iommu_tboot_noforce = 1;
 		}
 
 		str += strcspn(str, ",");
@@ -4840,8 +4845,22 @@ int __init intel_iommu_init(void)
 		goto out_free_dmar;
 	}
 
-	if (no_iommu || dmar_disabled)
+	if (no_iommu || dmar_disabled) {
+		/*
+		 * We exit the function here to ensure IOMMU's remapping and
+		 * mempool aren't setup, which means that the IOMMU's PMRs
+		 * won't be disabled via the call to init_dmars(). So disable
+		 * it explicitly here. The PMRs were setup by tboot prior to
+		 * calling SENTER, but the kernel is expected to reset/tear
+		 * down the PMRs.
+		 */
+		if (intel_iommu_tboot_noforce) {
+			for_each_iommu(iommu, drhd)
+				iommu_disable_protect_mem_regions(iommu);
+		}
+
 		goto out_free_dmar;
+	}
 
 	if (list_empty(&dmar_rmrr_units))
 		pr_info("No RMRR found\n");
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 187c102..9088407 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -39,6 +39,7 @@ extern int iommu_calculate_agaw(struct intel_iommu *iommu);
 extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
 extern int dmar_disabled;
 extern int intel_iommu_enabled;
+extern int intel_iommu_tboot_noforce;
 #else
 static inline int iommu_calculate_agaw(struct intel_iommu *iommu)
 {
-- 
2.9.3

Re: [PATCH] x86/tboot: add an option to disable iommu force on

[Joerg Roedel]

On Tue, Apr 25, 2017 at 09:28:53AM -0700, Shaohua Li wrote:
> IOMMU harms performance signficantly when we run very fast networking
> workloads. It's 40GB networking doing XDP test. Software overhead is
> almost unaware, but it's the IOTLB miss (based on our analysis) which
> kills the performance. We observed the same performance issue even with
> software passthrough (identity mapping), only the hardware passthrough
> survives. The pps with iommu (with software passthrough) is only about
> ~30% of that without it. This is a limitation in hardware based on our
> observation, so we'd like to disable the IOMMU force on, but we do want
> to use TBOOT and we can sacrifice the DMA security bought by IOMMU. I
> must admit I know nothing about TBOOT, but TBOOT guys (cc-ed) think not
> eabling IOMMU is totally ok.
> 
> So introduce a new boot option to disable the force on. It's kind of
> silly we need to run into intel_iommu_init even without force on, but we
> need to disable TBOOT PMR registers. For system without the boot option,
> nothing is changed.
> 
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  5 +++++
>  arch/x86/kernel/tboot.c                         |  3 +++
>  drivers/iommu/intel-iommu.c                     | 21 ++++++++++++++++++++-
>  include/linux/dma_remapping.h                   |  1 +
>  4 files changed, 29 insertions(+), 1 deletion(-)

Patch does not apply to my x86/vt-d branch.

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index facc20a..10c393b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1578,6 +1578,11 @@
>  			extended tables themselves, and also PASID support. With
>  			this option set, extended tables will not be used even
>  			on hardware which claims to support them.
> +		tboot_noforce [Default Off]
> +			By default, tboot will force Intel IOMMU on, which
> +			could harm performance for some workloads even IOMMU
> +			identity mapping is enabled. This option will avoid
> +			the 'force on' for Intel IOMMU.

Also the wording here should be more clear. How about:

> +			Do not force the Intel IOMMU enabled under
> +			tboot.	
> +			By default, tboot will force Intel IOMMU on, which
> +			could harm performance of some high-throughput
> +			devices like 40GBit network cards, even if
> +			identity mapping is enabled.
> +			Note that using this option lowers the security
> +			provided by tboot because it makes the system
> +			vulnerable to DMA attacks.

Regards,

	Joerg