[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 1/2 V2] iommu/amd: Fix logic for clearing the IOMMU interrupt bits



From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

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@xxxxxxx>
---
V2 changes:
        - Additional fixes as pointed out by Jan.
        - Clean up unnecessary #define mask as suggested by Jan.

 xen/drivers/passthrough/amd/iommu_cmd.c      |   18 +++--
 xen/drivers/passthrough/amd/iommu_init.c     |   31 ++++-----
 xen/include/asm-x86/hvm/svm/amd-iommu-defs.h |   95 +++++++++++---------------
 3 files changed, 63 insertions(+), 81 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c 
b/xen/drivers/passthrough/amd/iommu_cmd.c
index 4c60149..f0283d4 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -75,11 +75,9 @@ static 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((1 << IOMMU_STATUS_COMP_WAIT_INT_SHIFT),
+        iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /* send an empty COMPLETION_WAIT command to flush command buffer */
     cmd[3] = cmd[2] = 0;
@@ -96,16 +94,16 @@ static void flush_command_buffer(struct amd_iommu *iommu)
     do {
         status = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
         comp_wait = get_field_from_reg_u32(status,
-                                           IOMMU_STATUS_COMP_WAIT_INT_MASK,
-                                           IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
+                                (1 << IOMMU_STATUS_COMP_WAIT_INT_SHIFT),
+                                IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
         --loop_count;
     } while ( !comp_wait && loop_count );
 
     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((1 << IOMMU_STATUS_COMP_WAIT_INT_SHIFT),
+            iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
         return;
     }
     AMD_IOMMU_DEBUG("Warning: ComWaitInt bit did not assert!\n");
diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index a939c73..a85c63f 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -439,8 +439,8 @@ static void iommu_reset_log(struct amd_iommu *iommu,
 
     ctrl_func(iommu, IOMMU_CONTROL_DISABLED);
 
-    /*clear overflow bit */
-    iommu_clear_bit(&entry, of_bit);
+    /* RW1C overflow bit */
+    iommu_set_bit(&entry, of_bit);
     writel(entry, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /*reset event log base address */
@@ -622,11 +622,9 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
     if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) )
         iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control);
 
-    /* reset interrupt status bit */
-    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    iommu_set_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
-
-    writel(entry, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    /* RW1C interrupt status bit */
+    writel((1 << IOMMU_STATUS_EVENT_LOG_INT_SHIFT),
+        iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
@@ -692,11 +690,9 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
     if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) )
         iommu_reset_log(iommu, &iommu->ppr_log, set_iommu_ppr_log_control);
 
-    /* reset interrupt status bit */
-    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    iommu_set_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
-
-    writel(entry, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    /* RW1C interrupt status bit */
+    writel((1 << IOMMU_STATUS_PPR_LOG_INT_SHIFT),
+        iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
@@ -733,10 +729,13 @@ static void iommu_interrupt_handler(int irq, void *dev_id,
 
     spin_lock_irqsave(&iommu->lock, flags);
 
-    /* Silence interrupts from both event and PPR logging */
-    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
-    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
+    /* 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);
+    iommu_clear_bit(&entry, IOMMU_CONTROL_EVENT_LOG_INT_SHIFT);
+    iommu_clear_bit(&entry, IOMMU_CONTROL_PPR_LOG_INT_SHIFT);
+    /* RW1C status bit */
     writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h 
b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
index d2176d0..2f2d740 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
@@ -313,31 +313,21 @@
 #define IOMMU_LOG_ENTRY_TIMEOUT                         1000
 
 /* Control Register */
-#define IOMMU_CONTROL_MMIO_OFFSET                      0x18
-#define IOMMU_CONTROL_TRANSLATION_ENABLE_MASK          0x00000001
-#define IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT         0
-#define IOMMU_CONTROL_HT_TUNNEL_TRANSLATION_MASK       0x00000002
-#define IOMMU_CONTROL_HT_TUNNEL_TRANSLATION_SHIFT      1
-#define IOMMU_CONTROL_EVENT_LOG_ENABLE_MASK            0x00000004
-#define IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT           2
-#define IOMMU_CONTROL_EVENT_LOG_INT_MASK               0x00000008
-#define IOMMU_CONTROL_EVENT_LOG_INT_SHIFT              3
-#define IOMMU_CONTROL_COMP_WAIT_INT_MASK               0x00000010
-#define IOMMU_CONTROL_COMP_WAIT_INT_SHIFT              4
-#define IOMMU_CONTROL_INVALIDATION_TIMEOUT_MASK                0x000000E0
-#define IOMMU_CONTROL_INVALIDATION_TIMEOUT_SHIFT       5
-#define IOMMU_CONTROL_PASS_POSTED_WRITE_MASK           0x00000100
-#define IOMMU_CONTROL_PASS_POSTED_WRITE_SHIFT          8
-#define IOMMU_CONTROL_RESP_PASS_POSTED_WRITE_MASK      0x00000200
-#define IOMMU_CONTROL_RESP_PASS_POSTED_WRITE_SHIFT     9
-#define IOMMU_CONTROL_COHERENT_MASK                    0x00000400
-#define IOMMU_CONTROL_COHERENT_SHIFT                   10
-#define IOMMU_CONTROL_ISOCHRONOUS_MASK                 0x00000800
-#define IOMMU_CONTROL_ISOCHRONOUS_SHIFT                        11
-#define IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_MASK       0x00001000
-#define IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT      12
-#define IOMMU_CONTROL_RESTART_MASK                     0x80000000
-#define IOMMU_CONTROL_RESTART_SHIFT                    31
+#define IOMMU_CONTROL_MMIO_OFFSET                       0x18
+#define IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT          0
+#define IOMMU_CONTROL_HT_TUNNEL_TRANSLATION_SHIFT       1
+#define IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT            2
+#define IOMMU_CONTROL_EVENT_LOG_INT_SHIFT               3
+#define IOMMU_CONTROL_COMP_WAIT_INT_SHIFT               4
+#define IOMMU_CONTROL_INVALIDATION_TIMEOUT_SHIFT        5
+#define IOMMU_CONTROL_PASS_POSTED_WRITE_SHIFT           8
+#define IOMMU_CONTROL_RESP_PASS_POSTED_WRITE_SHIFT      9
+#define IOMMU_CONTROL_COHERENT_SHIFT                    10
+#define IOMMU_CONTROL_ISOCHRONOUS_SHIFT                 11
+#define IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT       12
+#define IOMMU_CONTROL_PPR_LOG_ENABLE_SHIFT              13
+#define IOMMU_CONTROL_PPR_LOG_INT_SHIFT                 14
+#define IOMMU_CONTROL_RESTART_SHIFT                     31
 
 #define IOMMU_CONTROL_PPR_LOG_ENABLE_SHIFT      13
 #define IOMMU_CONTROL_PPR_INT_SHIFT             14
@@ -363,38 +353,33 @@
 #define IOMMU_EXCLUSION_LIMIT_HIGH_SHIFT       0
 
 /* Extended Feature Register*/
-#define IOMMU_EXT_FEATURE_MMIO_OFFSET                   0x30
-#define IOMMU_EXT_FEATURE_PREFSUP_SHIFT                 0x0
-#define IOMMU_EXT_FEATURE_PPRSUP_SHIFT                  0x1
-#define IOMMU_EXT_FEATURE_XTSUP_SHIFT                   0x2
-#define IOMMU_EXT_FEATURE_NXSUP_SHIFT                   0x3
-#define IOMMU_EXT_FEATURE_GTSUP_SHIFT                   0x4
-#define IOMMU_EXT_FEATURE_IASUP_SHIFT                   0x6
-#define IOMMU_EXT_FEATURE_GASUP_SHIFT                   0x7
-#define IOMMU_EXT_FEATURE_HESUP_SHIFT                   0x8
-#define IOMMU_EXT_FEATURE_PCSUP_SHIFT                   0x9
-#define IOMMU_EXT_FEATURE_HATS_SHIFT                    0x10
-#define IOMMU_EXT_FEATURE_HATS_MASK                     0x00000C00
-#define IOMMU_EXT_FEATURE_GATS_SHIFT                    0x12
-#define IOMMU_EXT_FEATURE_GATS_MASK                     0x00003000
-#define IOMMU_EXT_FEATURE_GLXSUP_SHIFT                  0x14
-#define IOMMU_EXT_FEATURE_GLXSUP_MASK                   0x0000C000
-
-#define IOMMU_EXT_FEATURE_PASMAX_SHIFT                  0x0
-#define IOMMU_EXT_FEATURE_PASMAX_MASK                   0x0000001F
+#define IOMMU_EXT_FEATURE_MMIO_OFFSET           0x30
+#define IOMMU_EXT_FEATURE_PREFSUP_SHIFT         0x0
+#define IOMMU_EXT_FEATURE_PPRSUP_SHIFT          0x1
+#define IOMMU_EXT_FEATURE_XTSUP_SHIFT           0x2
+#define IOMMU_EXT_FEATURE_NXSUP_SHIFT           0x3
+#define IOMMU_EXT_FEATURE_GTSUP_SHIFT           0x4
+#define IOMMU_EXT_FEATURE_IASUP_SHIFT           0x6
+#define IOMMU_EXT_FEATURE_GASUP_SHIFT           0x7
+#define IOMMU_EXT_FEATURE_HESUP_SHIFT           0x8
+#define IOMMU_EXT_FEATURE_PCSUP_SHIFT           0x9
+#define IOMMU_EXT_FEATURE_HATS_SHIFT            0x10
+#define IOMMU_EXT_FEATURE_HATS_MASK             0x00000C00
+#define IOMMU_EXT_FEATURE_GATS_SHIFT            0x12
+#define IOMMU_EXT_FEATURE_GATS_MASK             0x00003000
+#define IOMMU_EXT_FEATURE_GLXSUP_SHIFT          0x14
+#define IOMMU_EXT_FEATURE_GLXSUP_MASK           0x0000C000
+
+#define IOMMU_EXT_FEATURE_PASMAX_SHIFT          0x0
+#define IOMMU_EXT_FEATURE_PASMAX_MASK           0x0000001F
 
 /* Status Register*/
-#define IOMMU_STATUS_MMIO_OFFSET               0x2020
-#define IOMMU_STATUS_EVENT_OVERFLOW_MASK       0x00000001
-#define IOMMU_STATUS_EVENT_OVERFLOW_SHIFT      0
-#define IOMMU_STATUS_EVENT_LOG_INT_MASK                0x00000002
-#define IOMMU_STATUS_EVENT_LOG_INT_SHIFT       1
-#define IOMMU_STATUS_COMP_WAIT_INT_MASK                0x00000004
-#define IOMMU_STATUS_COMP_WAIT_INT_SHIFT       2
-#define IOMMU_STATUS_EVENT_LOG_RUN_MASK                0x00000008
-#define IOMMU_STATUS_EVENT_LOG_RUN_SHIFT       3
-#define IOMMU_STATUS_CMD_BUFFER_RUN_MASK       0x00000010
-#define IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT      4
+#define IOMMU_STATUS_MMIO_OFFSET                0x2020
+#define IOMMU_STATUS_EVENT_OVERFLOW_SHIFT       0
+#define IOMMU_STATUS_EVENT_LOG_INT_SHIFT        1
+#define IOMMU_STATUS_COMP_WAIT_INT_SHIFT        2
+#define IOMMU_STATUS_EVENT_LOG_RUN_SHIFT        3
+#define IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT       4
 #define IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT     5
 #define IOMMU_STATUS_PPR_LOG_INT_SHIFT          6
 #define IOMMU_STATUS_PPR_LOG_RUN_SHIFT          7
-- 
1.7.10.4



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.