From: <suravee.suthikulpanit@amd.com>
To: xen-devel@lists.xen.org, JBeulich@suse.com
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: [PATCH 1/2 V2] iommu/amd: Fix logic for clearing the IOMMU interrupt bits
Date: Mon, 10 Jun 2013 00:05:50 -0500 [thread overview]
Message-ID: <1370840751-11277-1-git-send-email-suravee.suthikulpanit@amd.com> (raw)
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
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>
---
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
next reply other threads:[~2013-06-10 5:05 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-10 5:05 suravee.suthikulpanit [this message]
2013-06-10 5:05 ` [PATCH 2/2 V2] iommu/amd: Workaround for erratum 787 suravee.suthikulpanit
2013-06-10 9:35 ` Tim Deegan
2013-06-10 9:47 ` Jan Beulich
2013-06-10 10:40 ` Tim Deegan
2013-06-10 10:53 ` Jan Beulich
2013-06-10 12:43 ` Tim Deegan
2013-06-10 13:23 ` Jan Beulich
2013-06-10 13:41 ` Jan Beulich
2013-06-10 13:56 ` Tim Deegan
2013-06-10 13:55 ` Jan Beulich
2013-06-10 15:03 ` Jan Beulich
2013-06-10 16:31 ` Tim Deegan
2013-06-10 23:13 ` Suravee Suthikulanit
2013-06-11 6:45 ` Jan Beulich
2013-06-11 6:40 ` Jan Beulich
2013-06-11 8:53 ` Tim Deegan
2013-06-10 13:53 ` Suravee Suthikulanit
2013-06-10 13:59 ` Jan Beulich
2013-06-10 15:11 ` Suravee Suthikulanit
2013-06-10 15:21 ` Jan Beulich
2013-06-10 10:59 ` [PATCH 2/2 v3] " Jan Beulich
2013-06-11 6:47 ` [PATCH 2/2 v5] " Jan Beulich
2013-06-17 18:57 ` Suravee Suthikulanit
2013-06-10 10:05 ` [PATCH 1/2 V2] iommu/amd: Fix logic for clearing the IOMMU interrupt bits Jan Beulich
2013-06-10 10:56 ` [PATCH 1/2 v3] " Jan Beulich
2013-06-10 11:02 ` Jan Beulich
2013-06-10 12:18 ` Tim Deegan
2013-06-10 12:31 ` Jan Beulich
2013-06-10 13:58 ` Suravee Suthikulanit
2013-06-10 12:41 ` [PATCH 1/2 v4] " Jan Beulich
2013-06-10 12:46 ` Tim Deegan
2013-06-10 13:49 ` George Dunlap
2013-06-10 14:08 ` Jan Beulich
2013-06-11 6:47 ` [PATCH 1/2 v5] " Jan Beulich
2013-06-11 23:03 ` Suravee Suthikulanit
2013-06-12 6:24 ` Jan Beulich
2013-06-12 22:37 ` Suravee Suthikulpanit
2013-06-13 1:44 ` Suravee Suthikulpanit
2013-06-13 7:54 ` Jan Beulich
2013-06-13 13:48 ` Suravee Suthikulpanit
2013-06-13 14:20 ` George Dunlap
2013-06-13 14:30 ` Processed: " xen
2013-06-13 15:58 ` Jan Beulich
2013-06-13 16:34 ` Suravee Suthikulanit
2013-06-14 6:27 ` Jan Beulich
2013-06-14 6:40 ` Jan Beulich
2013-06-14 7:14 ` [PATCH] AMD IOMMU: make interrupt work again Jan Beulich
2013-06-14 16:10 ` Suravee Suthikulanit
2013-06-17 18:59 ` [PATCH 1/2 v5] iommu/amd: Fix logic for clearing the IOMMU interrupt bits Suravee Suthikulanit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1370840751-11277-1-git-send-email-suravee.suthikulpanit@amd.com \
--to=suravee.suthikulpanit@amd.com \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).