iommu/amd: Fix logic for clearing the IOMMU interrupt bits

The IOMMU interrupt bits in the IOMMU status registers are
"read-only, and write-1-to-clear (RW1C).  Therefore, the existing
logic which reads the register, set the bit, and then writing back
the values could accidentally clear certain bits if it has been set.

The correct logic would just be writing only the value which only
set the interrupt bits, and leave the rest to zeros.

This patch also, clean up #define masks as Jan has suggested.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
With iommu_interrupt_handler() properly having got switched its readl()
from status to control register, the subsequent writel() needed to be
switched too (and the RW1C comment there was bogus).

Some of the cleanup went too far - undone.

Further, with iommu_interrupt_handler() now actually disabling the
interrupt sources, they also need to get re-enabled by the tasklet once
it finished processing the respective log. This also implies re-running
the tasklet so that log entries added between reading the log and re-
enabling the interrupt will get handled in a timely manner.

Finally, guest write emulation to the status register needs to be done
with the RW1C (and RO for all other bits) semantics in mind too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
master commit: 2823a0c7dfc979db316787e1dd42a8845e5825c0
master date: 2013-07-02 08:49:43 +0200

diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index fcb7e87f5fa11fc0a9c1e5535bac3ce373ce16af..b87b61301cb1254b56e8a507e92138c54cfe0533 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -336,11 +336,9 @@ static void amd_iommu_reset_event_log(struct amd_iommu *iommu)
     /* read event log for debugging */
     amd_iommu_read_event_log(iommu);
 
-    /*clear overflow bit */
-    set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, entry,
-                         IOMMU_STATUS_EVENT_OVERFLOW_MASK,
-                         IOMMU_STATUS_EVENT_OVERFLOW_SHIFT, &entry);
-    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+    /* RW1C overflow bit */
+    writel(IOMMU_STATUS_EVENT_OVERFLOW_MASK,
+           iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /*reset event log base address */
     iommu->event_log_head = 0;
@@ -564,6 +562,11 @@ static void do_amd_iommu_irq(unsigned long data)
         int of;
 
         spin_lock_irqsave(&iommu->lock, flags);
+
+        /* RW1C interrupt status bit */
+        writel(IOMMU_STATUS_EVENT_LOG_INT_MASK,
+               iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+
         amd_iommu_read_event_log(iommu);
 
         /* check event overflow */
@@ -575,13 +578,21 @@ static void do_amd_iommu_irq(unsigned long data)
         /* reset event log if event overflow */
         if ( of )
             amd_iommu_reset_event_log(iommu);
+        else
+        {
+            entry = readl(iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
+            if ( !(entry & IOMMU_CONTROL_EVENT_LOG_INT_MASK) )
+            {
+                entry |= IOMMU_CONTROL_EVENT_LOG_INT_MASK;
+                writel(entry, iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
+                /*
+                 * Re-schedule the tasklet to handle eventual log entries added
+                 * between reading the log above and re-enabling the interrupt.
+                 */
+                tasklet_schedule(&amd_iommu_irq_tasklet);
+            }
+        }
 
-        /* reset interrupt status bit */
-        entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-        set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, entry,
-                             IOMMU_STATUS_EVENT_LOG_INT_MASK,
-                             IOMMU_STATUS_EVENT_LOG_INT_SHIFT, &entry);
-        writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
         spin_unlock_irqrestore(&iommu->lock, flags);
     }
 }
@@ -595,12 +606,13 @@ static void amd_iommu_page_fault(int irq, void *dev_id,
 
     spin_lock_irqsave(&iommu->lock, flags);
 
-    /* Silence interrupts from both event logging */
-    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, entry,
-                         IOMMU_STATUS_EVENT_LOG_INT_MASK,
-                         IOMMU_STATUS_EVENT_LOG_INT_SHIFT, &entry);
-    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+    /*
+     * Silence interrupts from both event and PPR by clearing the
+     * enable logging bits in the control register
+     */
+    entry = readl(iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
+    entry &= ~IOMMU_CONTROL_EVENT_LOG_INT_MASK;
+    writel(entry, iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
 

diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index a65e01ac1f5b222c71e4c5c55c4f1eafeb47273f..ea1f2733ce5fdbb8cb5dd4f7fd16df5398a41ec3 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -131,11 +131,9 @@ void flush_command_buffer(struct amd_iommu *iommu)
     u32 cmd[4], status;
     int loop_count, comp_wait;
 
-    /* clear 'ComWaitInt' in status register (WIC) */
-    set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, 0,
-                         IOMMU_STATUS_COMP_WAIT_INT_MASK,
-                         IOMMU_STATUS_COMP_WAIT_INT_SHIFT, &status);
-    writel(status, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    /* RW1C 'ComWaitInt' in status register */
+    writel(IOMMU_STATUS_COMP_WAIT_INT_MASK,
+           iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /* send an empty COMPLETION_WAIT command to flush command buffer */
     cmd[3] = cmd[2] = 0;
@@ -159,9 +157,9 @@ void flush_command_buffer(struct amd_iommu *iommu)
 
     if ( comp_wait )
     {
-        /* clear 'ComWaitInt' in status register (WIC) */
-        status &= IOMMU_STATUS_COMP_WAIT_INT_MASK;
-        writel(status, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+        /* RW1C 'ComWaitInt' in status register */
+        writel(IOMMU_STATUS_COMP_WAIT_INT_MASK,
+               iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
         return;
     }
     AMD_IOMMU_DEBUG("Warning: ComWaitInt bit did not assert!\n");