iommu.c: rework initialization function

Test for INVALIDATE_ALL completion is now done inside the initialization
function, instead of main.c. Function names were changed to better
describe what the function does. Comment describing window in anti-DMA
security was updated with new finds.

Some bugs were fixed:
- EventLogInt wasn't properly cleared (it is write-1-to-clear bit)
- CmdBufEn was set on transition to the kernel, which didn't clear it
  before setting up head/tail pointers, which in turn lead to hang
  (it is listed as undefined behavior in IOMMU specification)
- Command buffer tail/head pointer registers were incorrectly masked

Signed-off-by: Krystian Hebel <[email protected]>

Author: krystian-hebel

include/iommu.h

@@ -28,6 +28,7 @@
 #define IOMMU_DTE_Q0_IW			(1ULL << 62)
 
 #define IOMMU_DTE_Q1_I			(1ULL << (96 - 64))
+#define IOMMU_DTE_Q1_SE			(1ULL << (97 - 64))
 
 #define IOMMU_DTE_Q2_IV			(1ULL << (128 - 128))
 #define IOMMU_DTE_Q2_IG			(1ULL << (133 - 128))
@@ -78,8 +79,7 @@
 #define IOMMU_CR_BlkStopMrkEn		(1ULL << 41)
 #define IOMMU_CR_PprAutoRspAon		(1ULL << 42)
 
-#define IOMMU_CR_ENABLE_ALL_MASK	(IOMMU_CR_IommuEn | \
-					 IOMMU_CR_HtTunEn | \
+#define IOMMU_CR_ENABLE_ALL_MASK	(IOMMU_CR_HtTunEn | \
 					 IOMMU_CR_EventLogEn | \
 					 IOMMU_CR_EventIntEn | \
 					 IOMMU_CR_ComWaitIntEn | \
@@ -104,6 +104,8 @@
 
 #define IOMMU_EF_IASup			(1ULL << 6)
 
+#define IOMMU_SR_EventLogInt		(1ULL << 1)
+
 #define COMPLETION_WAIT			1
 #define INVALIDATE_DEVTAB_ENTRY		2
 #define INVALIDATE_IOMMU_PAGES		3
@@ -135,6 +137,6 @@ extern iommu_command_t command_buf[2];
 void disable_memory_protection(void);
 
 u32 iommu_locate(void);
-u32 iommu_load_device_table(u32 cap, volatile u64 *completed);
+int iommu_init(u32 cap);
 
 #endif /* __IOMMU_H__ */

iommu.c

@@ -39,14 +39,25 @@ static void send_command(u64 *mmio_base, iommu_command_t cmd, u32 *idx)
         _u(&command_buf[*idx]) - (_u(&command_buf) & ~0xfff);
 }
 
-u32 iommu_load_device_table(u32 cap, volatile u64 *completed)
+int iommu_init(u32 cap)
 {
     u64 *mmio_base;
+    volatile u64 completed __attribute__ ((aligned (8))) = 0;
     u32 low, hi, i, idx = 0;
     iommu_command_t cmd = {0};
 
-    for (i = 0; i < ARRAY_SIZE(device_table); i++)
+    for (i = 0; i < ARRAY_SIZE(device_table); i++) {
         device_table[i].a = IOMMU_DTE_Q0_V + IOMMU_DTE_Q0_TV;
+        /*
+         * Devices (e.g. USB with legacy keyboard emulation) on some platforms
+         * can be quite noisy, this limits number of events logged to one page
+         * fault per device.
+         *
+         * NB: there are erratas due to which all entries may still be logged
+         * despite enabled suppression.
+         */
+        device_table[i].b = IOMMU_DTE_Q1_SE;
+    }
 
     pci_read(0, IOMMU_PCI_BUS,
              PCI_DEVFN(IOMMU_PCI_DEVICE, IOMMU_PCI_FUNCTION),
@@ -70,7 +81,7 @@ u32 iommu_load_device_table(u32 cap, volatile u64 *completed)
     print_u64(mmio_base[IOMMU_MMIO_STATUS_REGISTER]);
     print("IOMMU_MMIO_STATUS_REGISTER\n");
 
-    /* Disable IOMMU and all its features */
+    /* Disable all features, but don't touch IommuEn */
     mmio_base[IOMMU_MMIO_CONTROL_REGISTER] &= ~IOMMU_CR_ENABLE_ALL_MASK;
     smp_wmb();
 
@@ -100,7 +111,8 @@ u32 iommu_load_device_table(u32 cap, volatile u64 *completed)
      * command_buf[] to begin with, but we do save almost 4k of space,
      * 1/16th of that available to us.
      */
-    mmio_base[IOMMU_MMIO_COMMAND_BUF_BA] = (u64)(_u(command_buf) & ~0xfff)| (0x9ULL << 56);
+    mmio_base[IOMMU_MMIO_COMMAND_BUF_BA] = (u64)(_u(command_buf) & ~0xfff) |
+                                           (0x9ULL << 56);
     mmio_base[IOMMU_MMIO_COMMAND_BUF_HEAD] =
         mmio_base[IOMMU_MMIO_COMMAND_BUF_TAIL] = _u(command_buf) & 0xff0;
 
@@ -121,8 +133,8 @@ u32 iommu_load_device_table(u32 cap, volatile u64 *completed)
     print_u64(mmio_base[IOMMU_MMIO_EVENT_LOG_BA]);
     print("IOMMU_MMIO_EVENT_LOG_BA\n");
 
-    /* Clear EventLogInt set by IOMMU not being able to read command buffer */
-    mmio_base[IOMMU_MMIO_STATUS_REGISTER] &= ~2;
+    /* Write 1 to clear EventLogInt set by IOMMU being unable to read command */
+    mmio_base[IOMMU_MMIO_STATUS_REGISTER] &= IOMMU_SR_EventLogInt;
     smp_wmb();
     mmio_base[IOMMU_MMIO_CONTROL_REGISTER] |= IOMMU_CR_CmdBufEn | IOMMU_CR_EventLogEn;
     smp_wmb();
@@ -145,16 +157,58 @@ u32 iommu_load_device_table(u32 cap, volatile u64 *completed)
     print("IOMMU_MMIO_STATUS_REGISTER\n");
 
     /* Write to a variable inside SLB (does not work in the first call) */
-    cmd.u0 = _u(completed) | 1;
-    /* This should be '_u(completed)>>32', but SLB can't be above 4GB anyway */
+    cmd.u0 = _u(&completed) | 1;
+    /* This should be '_u(&completed)>>32', but SLB can't be above 4GB anyway */
     cmd.u1 = 0;
 
     cmd.opcode = COMPLETION_WAIT;
     cmd.u2 = 0x656e6f64;    /* "done" */
     send_command(mmio_base, cmd, &idx);
 
+    /* Make sure tail pointer is updated before checking for completion */
+    smp_wmb();
+
+    print("Flushing IOMMU cache");
+    while ( !completed ) {
+        print(".");
+
+        /*
+         * On the first run, EventLogInt will be set, but IOMMU won't be able to
+         * write to the log due to active DEV. After that IOMMU stops fetching
+         * new commands, so 'completed' won't be ever set. Check if EventCode
+         * has the initial value of 0 and return instead of waiting indefinitely
+         * for 'completed'.
+         */
+        if ( mmio_base[IOMMU_MMIO_STATUS_REGISTER] & IOMMU_SR_EventLogInt ) {
+            if ( event_log[7] == 0 )
+                return 0;
+        }
+    }
+    print("\n");
+
+    if ( mmio_base[IOMMU_MMIO_STATUS_REGISTER] & IOMMU_SR_EventLogInt ) {
+        print("IOMMU event log not empty:\n");
+        hexdump(event_log, 512);
+    }
+
+    /*
+     * If Command Buffer Head Pointer Register is written to by software while
+     * CmdBufRun = 1b, the IOMMU behavior is undefined, similarly for Event Log
+     * Tail Pointer and EventLogRun = 1b. DLME may not be so kind to check for
+     * this, so stop all parsing and logging here, but keep IOMMU enabled.
+     *
+     * NB: IOMMU specification doesn't mention undefined behavior when changing
+     * Command Buffer Base Address, although it does so for Event Log Base
+     * Address. This may be an overlook, but it doesn't change anything since
+     * all parsing and logging is disabled.
+     */
+    mmio_base[IOMMU_MMIO_CONTROL_REGISTER] &= ~IOMMU_CR_ENABLE_ALL_MASK;
+    smp_wmb();
+
     print_u64(mmio_base[IOMMU_MMIO_STATUS_REGISTER]);
     print("IOMMU_MMIO_STATUS_REGISTER\n");
 
+    print("IOMMU set\n");
+
     return 0;
 }

main.c

@@ -117,10 +117,9 @@ static void do_dma(void)
 }
 #endif
 
-static void iommu_setup(void)
+static void dma_protection_setup(void)
 {
     u32 iommu_cap;
-    volatile u64 iommu_done __attribute__ ((aligned (8))) = 0;
 
 #ifdef TEST_DMA
     memset(_p(1), 0xcc, 0x20); //_p(0) gives a null-pointer error
@@ -150,19 +149,24 @@ static void iommu_setup(void)
      * When IOMMU is trying to read a command from buffer located in SLB it
      * receives COMMAND_HARDWARE_ERROR (master abort).
      *
-     * Luckily, after that error it enters a fail-safe state in which all
-     * operations originating from devices are blocked. The IOMMU itself can
-     * still access the memory, so after the SLB protection is lifted, it can
-     * try to read the data located inside SLB and set up a proper protection.
+     * After that error IOMMU enters a state in which protections use whatever
+     * settings are cached. In case of cold boot, all operations originating
+     * from devices are blocked. The IOMMU itself can still access the memory,
+     * so after the SLB protection is lifted, it can try to read the commands
+     * located inside SLB and set up a proper protection.
+     *
+     * The same isn't true for IOMMU that was already used earlier. It may
+     * allow devices to access the SLB memory between time when DEV is disabled
+     * and when IOMMU cache is flushed. As IOMMU command table is located in
+     * that memory, the invalidation command may never be executed...
      *
      * TODO: split iommu_load_device_table() into two parts, before and after
-     *       DEV disabling
+     *       DEV disabling, to minimize vulnerability time window
      *
-     * TODO2: check if IOMMU always blocks the devices, even when it was
-     *        configured before SKINIT
+     * TODO2: check what IOMMU_PGFSM_CONFIG does and if it exists in newer CPUs
      */
 
-    if ( iommu_cap == 0 || iommu_load_device_table(iommu_cap, &iommu_done) )
+    if ( iommu_cap == 0 || iommu_init(iommu_cap) )
     {
         if ( iommu_cap )
             print("IOMMU disabled by a firmware, please check your settings\n");
@@ -199,12 +203,7 @@ static void iommu_setup(void)
         hexdump(_p(0), 0x30);
 #endif
 
-        iommu_load_device_table(iommu_cap, &iommu_done);
-        print("Flushing IOMMU cache");
-        while ( !iommu_done )
-            print(".");
-
-        print("\nIOMMU set\n");
+        iommu_init(iommu_cap);
     }
 
 #ifdef TEST_DMA
@@ -253,15 +252,12 @@ asm_return_t skl_main(void)
 
     /*
      * Now in 64b mode, paging is setup. This is the launching point. We can
-     * now do what we want. First order of business is to setup
-     * DEV to cover memory from the start of bzImage to the end of the SKL
-     * "kernel". At the end, trampoline to the PM entry point which will
-     * include the Secure Launch stub.
+     * now do what we want. At the end, trampoline to the PM entry point which
+     * will include the Secure Launch stub.
      */
-    pci_init();
 
     /* Disable memory protection and setup IOMMU */
-    iommu_setup();
+    dma_protection_setup();
 
     /*
      * TODO Note these functions can fail but there is no clear way to