[PATCH 0/5] iommu/intel: Handle DMAR faults in a wq

[PATCH 0/5] iommu/intel: Handle DMAR faults in a wq

[Dmitry Safonov]

A softlockup-panic fix I've meet on kernel test suite.
While at it, fix a couple of minor issues.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Lu Baolu <baolu.lu@linux.intel.com>
Cc: iommu@lists.linux-foundation.org

Dmitry Safonov (5):
  iommu/intel: Add __init for dmar_register_bus_notifier()
  iommu/intel: Clean/document fault status flags
  iommu/intel: Introduce clear_primary_faults() helper
  iommu/intel: Handle DMAR faults on workqueue
  iommu/intel: Rename dmar_fault() => dmar_serve_faults()

 drivers/iommu/dmar.c                | 66 +++++++++++++++++++++++++++++++++----
 drivers/iommu/intel-iommu.c         |  8 ++++-
 drivers/iommu/intel_irq_remapping.c |  2 +-
 include/linux/dmar.h                |  3 +-
 include/linux/intel-iommu.h         | 12 +++----
 5 files changed, 75 insertions(+), 16 deletions(-)

-- 
2.13.6

[PATCH 1/5] iommu/intel: Add __init for dmar_register_bus_notifier()

[Dmitry Safonov]

It's called only from intel_iommu_init(), which is init function.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 9a7ffd13c7f0..accf58388bdb 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -806,7 +806,7 @@ int __init dmar_dev_scope_init(void)
 	return dmar_dev_scope_status;
 }
 
-void dmar_register_bus_notifier(void)
+void __init dmar_register_bus_notifier(void)
 {
 	bus_register_notifier(&pci_bus_type, &dmar_pci_bus_nb);
 }
-- 
2.13.6

[PATCH 2/5] iommu/intel: Clean/document fault status flags

[Dmitry Safonov]

So one could decode them without opening the specification.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/linux/intel-iommu.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index f3274d9f46a2..a4dc9c2875cc 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -207,12 +207,12 @@
 #define DMA_FECTL_IM (((u32)1) << 31)
 
 /* FSTS_REG */
-#define DMA_FSTS_PPF ((u32)2)
-#define DMA_FSTS_PFO ((u32)1)
-#define DMA_FSTS_IQE (1 << 4)
-#define DMA_FSTS_ICE (1 << 5)
-#define DMA_FSTS_ITE (1 << 6)
-#define DMA_FSTS_PRO (1 << 7)
+#define DMA_FSTS_PFO (1 << 0) /* Primary Fault Overflow */
+#define DMA_FSTS_PPF (1 << 1) /* Primary Pending Fault */
+#define DMA_FSTS_IQE (1 << 4) /* Invalidation Queue Error */
+#define DMA_FSTS_ICE (1 << 5) /* Invalidation Completion Error */
+#define DMA_FSTS_ITE (1 << 6) /* Invalidation Time-out Error */
+#define DMA_FSTS_PRO (1 << 7) /* Page Request Overflow */
 #define dma_fsts_fault_record_index(s) (((s) >> 8) & 0xff)
 
 /* FRCD_REG, 32 bits access */
-- 
2.13.6

[PATCH 3/5] iommu/intel: Introduce clear_primary_faults() helper

[Dmitry Safonov]

To my mind it's a bit more readable - and I will re-use it in the next
patch.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 drivers/iommu/dmar.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index accf58388bdb..33fb4244e438 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1611,6 +1611,15 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
 	return 0;
 }
 
+static void clear_primary_faults(struct intel_iommu *iommu)
+{
+	volatile void __iomem *fsts = iommu->reg + DMAR_FSTS_REG;
+
+	assert_raw_spin_locked(&iommu->register_lock);
+
+	writel(DMA_FSTS_PFO | DMA_FSTS_PPF | DMA_FSTS_PRO, fsts);
+}
+
 #define PRIMARY_FAULT_REG_LEN (16)
 irqreturn_t dmar_fault(int irq, void *dev_id)
 {
@@ -1679,8 +1688,7 @@ irqreturn_t dmar_fault(int irq, void *dev_id)
 		raw_spin_lock_irqsave(&iommu->register_lock, flag);
 	}
 
-	writel(DMA_FSTS_PFO | DMA_FSTS_PPF | DMA_FSTS_PRO,
-	       iommu->reg + DMAR_FSTS_REG);
+	clear_primary_faults(iommu);
 
 unlock_exit:
 	raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
-- 
2.13.6

[PATCH 4/5] iommu/intel: Handle DMAR faults on workqueue

[Dmitry Safonov]

dmar_fault() reports/handles/cleans DMAR faults in a cycle one-by-one.
The nuisance is that it's set as a irq handler and runs with disabled
interrupts - which works OK if you have only a couple of DMAR faults,
but becomes a problem if your intel iommu has a plenty of mappings.

We have a test that checks flapping of PCI link. Once or twice it had
stuck on soft lockup (which is panic for the typical switch):

 dmar: DMAR:[DMA Write] Request device [4a:00.0] fault addr 5e1a3000
 DMAR:[fault reason 02] Present bit in context entry is clear
 NMI watchdog: BUG: soft lockup - CPU#8 stuck for 21s!
 CPU: 8 PID: 2774 Comm: SuperServer
 Call Trace:
  generic_exec_single+0x11b/0x12d
  ? flush_tlb_func+0x0/0x18a
  smp_call_function_single+0x89/0xb5
  smp_call_function_many+0xef/0x207
  native_flush_tlb_others+0x2e/0x30
  flush_tlb_mm_range+0x139/0x18a
  tlb_flush_mmu_tlbonly+0x35/0x85
  tlb_flush_mmu+0x13/0x1f
  tlb_finish_mmu+0x14/0x39
  unmap_region+0xda/0xec
  do_munmap+0x273/0x2f5
  vm_munmap+0x45/0x5e
  SyS_munmap+0x26/0x2f
  sysenter_dispatch+0x7/0x25

 Kernel panic - not syncing: softlockup: hung tasks
 sending NMI to all CPUs:
 NMI backtrace for cpu 0
 CPU: 0 PID: 0 Comm: swapper/0
 Call Trace:
  <IRQ>
  wait_for_xmitr+0x26/0x8f
  serial8250_console_putchar+0x1c/0x2c
  uart_console_write+0x40/0x4b
  serial8250_console_write+0xe6/0x13f
  call_console_drivers.constprop.13+0xce/0x103
  console_unlock+0x1f8/0x39b
  ? local_clock+0x21/0x23
  vprintk_emit+0x39e/0x3e6
  printk+0x4d/0x4f
  dmar_fault+0x1ab/0x1fd
  handle_irq_event_percpu+0x8a/0x1f5
  handle_irq_event+0x41/0x61
  handle_edge_irq+0xe1/0xfa
  handle_irq+0x155/0x166
  do_IRQ+0x5a/0xea
  ret_from_intr+0x0/0x15
  <EOI>
  arch_cpu_idle+0xf/0x11
  cpu_startup_entry+0x22f/0x3cb
  rest_init+0x80/0x84
  start_kernel+0x470/0x47d
  x86_64_start_reservations+0x2a/0x2c
  x86_64_start_kernel+0x14b/0x15a

Move DMAR faults clearing out of irq-disabled critical section by
proceeding with that in a workqueue thread.
The next patch will correct the definition of dmar_fault().

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 drivers/iommu/dmar.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/iommu/intel-iommu.c |  6 ++++++
 include/linux/dmar.h        |  1 +
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 33fb4244e438..34a53aeede38 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -71,6 +71,12 @@ struct acpi_table_header * __initdata dmar_tbl;
 static int dmar_dev_scope_status = 1;
 static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
 
+static struct workqueue_struct *fault_wq;
+struct fault {
+	struct work_struct work;
+	struct intel_iommu *iommu;
+};
+
 static int alloc_iommu(struct dmar_drhd_unit *drhd);
 static void free_iommu(struct intel_iommu *iommu);
 
@@ -811,6 +817,14 @@ void __init dmar_register_bus_notifier(void)
 	bus_register_notifier(&pci_bus_type, &dmar_pci_bus_nb);
 }
 
+int __init dmar_init_workqueue(void)
+{
+	fault_wq = alloc_workqueue("dmar_fault", WQ_MEM_RECLAIM | WQ_UNBOUND, 0);
+	if (fault_wq == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
 
 int __init dmar_table_init(void)
 {
@@ -1695,6 +1709,38 @@ irqreturn_t dmar_fault(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void dmar_fault_work(struct work_struct *work)
+{
+	struct fault *fault = container_of(work, struct fault, work);
+
+	dmar_fault(-1, (void*)fault->iommu);
+}
+
+static irqreturn_t dmar_fault_handler(int irq, void *dev_id)
+{
+	struct intel_iommu *iommu = dev_id;
+	struct fault *fault;
+
+	fault = kzalloc(sizeof(*fault), GFP_ATOMIC);
+	if (fault == NULL) {
+		unsigned long flag;
+
+		/* During OOM - clear faults and let device re-fault */
+		raw_spin_lock_irqsave(&iommu->register_lock, flag);
+		clear_primary_faults(iommu);
+		raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
+
+		return IRQ_HANDLED;
+	}
+
+	fault->iommu = iommu;
+	INIT_WORK(&fault->work, dmar_fault_work);
+
+	queue_work(fault_wq, &fault->work);
+
+	return IRQ_HANDLED;
+}
+
 int dmar_set_interrupt(struct intel_iommu *iommu)
 {
 	int irq, ret;
@@ -1713,7 +1759,7 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
 		return -EINVAL;
 	}
 
-	ret = request_irq(irq, dmar_fault, IRQF_NO_THREAD, iommu->name, iommu);
+	ret = request_irq(irq, dmar_fault_handler, IRQF_NO_THREAD, iommu->name, iommu);
 	if (ret)
 		pr_err("Can't request irq\n");
 	return ret;
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4a2de34895ec..4869c02652cc 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4784,6 +4784,12 @@ int __init intel_iommu_init(void)
 		goto out_free_dmar;
 	}
 
+	if (dmar_init_workqueue()) {
+		if (force_on)
+			panic("tboot: Failed to init workqueue for DMARs\n");
+		goto out_free_dmar;
+	}
+
 	if (list_empty(&dmar_rmrr_units))
 		pr_info("No RMRR found\n");
 
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index e2433bc50210..5de4932a6ad8 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -113,6 +113,7 @@ static inline bool dmar_rcu_check(void)
 extern int dmar_table_init(void);
 extern int dmar_dev_scope_init(void);
 extern void dmar_register_bus_notifier(void);
+extern int dmar_init_workqueue(void);
 extern int dmar_parse_dev_scope(void *start, void *end, int *cnt,
 				struct dmar_dev_scope **devices, u16 segment);
 extern void *dmar_alloc_dev_scope(void *start, void *end, int *cnt);
-- 
2.13.6

[PATCH 5/5] iommu/intel: Rename dmar_fault() => dmar_serve_faults()

[Dmitry Safonov]

Fix the return value, parameters and a bit better naming.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 drivers/iommu/dmar.c                | 8 +++-----
 drivers/iommu/intel-iommu.c         | 2 +-
 drivers/iommu/intel_irq_remapping.c | 2 +-
 include/linux/dmar.h                | 2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 34a53aeede38..37a1147a7592 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1635,9 +1635,8 @@ static void clear_primary_faults(struct intel_iommu *iommu)
 }
 
 #define PRIMARY_FAULT_REG_LEN (16)
-irqreturn_t dmar_fault(int irq, void *dev_id)
+void dmar_serve_faults(struct intel_iommu *iommu)
 {
-	struct intel_iommu *iommu = dev_id;
 	int reg, fault_index;
 	u32 fault_status;
 	unsigned long flag;
@@ -1706,14 +1705,13 @@ irqreturn_t dmar_fault(int irq, void *dev_id)
 
 unlock_exit:
 	raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
-	return IRQ_HANDLED;
 }
 
 static void dmar_fault_work(struct work_struct *work)
 {
 	struct fault *fault = container_of(work, struct fault, work);
 
-	dmar_fault(-1, (void*)fault->iommu);
+	dmar_serve_faults(fault->iommu);
 }
 
 static irqreturn_t dmar_fault_handler(int irq, void *dev_id)
@@ -1786,7 +1784,7 @@ int __init enable_drhd_fault_handling(void)
 		/*
 		 * Clear any previous faults.
 		 */
-		dmar_fault(iommu->irq, iommu);
+		dmar_serve_faults(iommu);
 		fault_status = readl(iommu->reg + DMAR_FSTS_REG);
 		writel(fault_status, iommu->reg + DMAR_FSTS_REG);
 	}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4869c02652cc..ee98cbce54bd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3016,7 +3016,7 @@ static void intel_iommu_init_qi(struct intel_iommu *iommu)
 		/*
 		 * Clear any previous faults.
 		 */
-		dmar_fault(-1, iommu);
+		dmar_serve_faults(iommu);
 		/*
 		 * Disable queued invalidation if supported and already enabled
 		 * before OS handover.
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 66f69af2c219..0958fdc76564 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -558,7 +558,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)
 		/*
 		 * Clear previous faults.
 		 */
-		dmar_fault(-1, iommu);
+		dmar_serve_faults(iommu);
 		dmar_disable_qi(iommu);
 
 		if (dmar_enable_qi(iommu)) {
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 5de4932a6ad8..6e9a5e4ac73a 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -280,7 +280,7 @@ extern void dmar_msi_mask(struct irq_data *data);
 extern void dmar_msi_read(int irq, struct msi_msg *msg);
 extern void dmar_msi_write(int irq, struct msi_msg *msg);
 extern int dmar_set_interrupt(struct intel_iommu *iommu);
-extern irqreturn_t dmar_fault(int irq, void *dev_id);
+extern void dmar_serve_faults(struct intel_iommu *iommu);
 extern int dmar_alloc_hwirq(int id, int node, void *arg);
 extern void dmar_free_hwirq(int irq);
 
-- 
2.13.6