On Mon, Jun 21, 2021 at 10:51 AM Ross Philipson
[off-list ref] wrote:

On 6/18/21 2:32 PM, Robin Murphy wrote:

quoted

On 2021-06-18 17:12, Ross Philipson wrote:

quoted

The IOMMU should always be set to default translated type after
the PMRs are disabled to protect the MLE from DMA.

Signed-off-by: Ross Philipson <redacted>
---
  drivers/iommu/intel/iommu.c | 5 +++++
  drivers/iommu/iommu.c       | 6 +++++-
  2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index be35284..4f0256d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c

@@ -41,6 +41,7 @@
  #include <linux/dma-direct.h>
  #include <linux/crash_dump.h>
  #include <linux/numa.h>
+#include <linux/slaunch.h>
  #include <asm/irq_remapping.h>
  #include <asm/cacheflush.h>
  #include <asm/iommu.h>

@@ -2877,6 +2878,10 @@ static bool device_is_rmrr_locked(struct device

*dev)
   */
  static int device_def_domain_type(struct device *dev)
  {
+    /* Do not allow identity domain when Secure Launch is configured */
+    if (slaunch_get_flags() & SL_FLAG_ACTIVE)
+        return IOMMU_DOMAIN_DMA;

Is this specific to Intel? It seems like it could easily be done
commonly like the check for untrusted external devices.

It is currently Intel only but that will change. I will look into what
you suggest.

quoted

quoted

+
      if (dev_is_pci(dev)) {
          struct pci_dev *pdev = to_pci_dev(dev);
  diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 808ab70d..d49b7dd 100644

--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c

@@ -23,6 +23,7 @@
  #include <linux/property.h>
  #include <linux/fsl/mc.h>
  #include <linux/module.h>
+#include <linux/slaunch.h>
  #include <trace/events/iommu.h>
    static struct kset *iommu_group_kset;

@@ -2761,7 +2762,10 @@ void iommu_set_default_passthrough(bool cmd_line)
  {
      if (cmd_line)
          iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
-    iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+
+    /* Do not allow identity domain when Secure Launch is configured */
+    if (!(slaunch_get_flags() & SL_FLAG_ACTIVE))
+        iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;

Quietly ignoring the setting and possibly leaving iommu_def_domain_type
uninitialised (note that 0 is not actually a usable type) doesn't seem
great. AFAICS this probably warrants similar treatment to the

Ok so I guess it would be better to set it to IOMMU_DOMAIN_DMA event
though passthrough was requested. Or perhaps something more is needed here?

quoted

mem_encrypt_active() case - there doesn't seem a great deal of value in
trying to save users from themselves if they care about measured boot
yet explicitly pass options which may compromise measured boot. If you
really want to go down that route there's at least the sysfs interface
you'd need to nobble as well, not to mention the various ways of
completely disabling IOMMUs...

Doing a secure launch with the kernel is not a general purpose user use
case. A lot of work is done to secure the environment. Allowing
passthrough mode would leave the secure launch kernel exposed to DMA. I
think what we are trying to do here is what we intend though there may
be a better way or perhaps it is incomplete as you suggest.

I don't really like all these special cases.  Generically, what you're
trying to do is (AFAICT) to get the kernel to run in a mode in which
it does its best not to trust attached devices.  Nothing about this is
specific to Secure Launch.  There are plenty of scenarios in which
this the case:

 - Virtual devices in a VM host outside the TCB, e.g. VDUSE, Xen
device domains (did I get the name right), whatever tricks QEMU has,
etc.
 - SRTM / DRTM technologies (including but not limited to Secure
Launch -- plain old Secure Boot can work like this too).
 - Secure guest technologies, including but not limited to TDX and SEV.
 - Any computer with a USB-C port or other external DMA-capable port.
 - Regular computers in which the admin wants to enable this mode for
whatever reason.

Can you folks all please agree on a coordinated way for a Linux kernel
to configure itself appropriately?  Or to be configured via initramfs,
boot option, or some other trusted source of configuration supplied at
boot time?  We don't need a whole bunch of if (TDX), if (SEV), if
(secure launch), if (I have a USB-C port with PCIe exposed), if
(running on Xen), and similar checks all over the place.