From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
"Wei Liu" <wl@xen.org>, "Jan Beulich" <JBeulich@suse.com>,
"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [Xen-devel] [PATCH] AMD/IOMMU: Common the #732/#733 errata handling in iommu_read_log()
Date: Fri, 14 Feb 2020 18:55:39 +0000 [thread overview]
Message-ID: <20200214185539.7444-1-andrew.cooper3@citrix.com> (raw)
There is no need to have both helpers implement the same workaround. The size
and layout of the the Event and PPR logs (and others for that matter) share a
lot of commonality.
Use MASK_EXTR() to locate the code field, and use ACCESS_ONCE() rather than
barrier() to prevent hoisting of the repeated read.
Avoid unnecessary zeroing by only clobbering the 'code' field - this alone is
sufficient to spot the errata when the rings wrap.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
xen/drivers/passthrough/amd/iommu_init.c | 80 ++++++++++++--------------------
1 file changed, 29 insertions(+), 51 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index c42b608f07..5de5315d8b 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -300,7 +300,7 @@ static int iommu_read_log(struct amd_iommu *iommu,
unsigned int entry_size,
void (*parse_func)(struct amd_iommu *, u32 *))
{
- u32 tail, *entry, tail_offest, head_offset;
+ unsigned int tail, tail_offest, head_offset;
BUG_ON(!iommu || ((log != &iommu->event_log) && (log != &iommu->ppr_log)));
@@ -319,11 +319,36 @@ static int iommu_read_log(struct amd_iommu *iommu,
while ( tail != log->head )
{
- /* read event log entry */
- entry = log->buffer + log->head;
+ uint32_t *entry = log->buffer + log->head;
+ unsigned int count = 0;
+
+ /* Event and PPR logs have their code field in the same position. */
+ unsigned int code = MASK_EXTR(entry[1], IOMMU_EVENT_CODE_MASK);
+
+ /*
+ * Workaround for errata #732, #733:
+ *
+ * It can happen that the tail pointer is updated before the actual
+ * entry got written. As suggested by RevGuide, we initialize the
+ * buffer to all zeros and clear entries after processing them.
+ */
+ while ( unlikely(code == 0) )
+ {
+ if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) )
+ {
+ AMD_IOMMU_DEBUG("AMD-Vi: No entry written to %s Log\n",
+ log == &iommu->event_log ? "Event" : "PPR");
+ return 0;
+ }
+ udelay(1);
+ code = MASK_EXTR(ACCESS_ONCE(entry[1]), IOMMU_EVENT_CODE_MASK);
+ }
parse_func(iommu, entry);
+ /* Clear 'code' to be able to spot the erratum when the ring wraps. */
+ ACCESS_ONCE(entry[1]) = 0;
+
log->head += entry_size;
if ( log->head == log->size )
log->head = 0;
@@ -503,7 +528,6 @@ static hw_irq_controller iommu_x2apic_type = {
static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[])
{
u32 code;
- int count = 0;
static const char *const event_str[] = {
#define EVENT_STR(name) [IOMMU_EVENT_##name - 1] = #name
EVENT_STR(ILLEGAL_DEV_TABLE_ENTRY),
@@ -521,25 +545,6 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[])
code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK,
IOMMU_EVENT_CODE_SHIFT);
- /*
- * Workaround for erratum 732:
- * It can happen that the tail pointer is updated before the actual entry
- * got written. As suggested by RevGuide, we initialize the event log
- * buffer to all zeros and clear event log entries after processing them.
- */
- while ( code == 0 )
- {
- if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) )
- {
- AMD_IOMMU_DEBUG("AMD-Vi: No event written to log\n");
- return;
- }
- udelay(1);
- barrier(); /* Prevent hoisting of the entry[] read. */
- code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK,
- IOMMU_EVENT_CODE_SHIFT);
- }
-
/* Look up the symbolic name for code. */
if ( code <= ARRAY_SIZE(event_str) )
code_str = event_str[code - 1];
@@ -575,8 +580,6 @@ static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[])
else
printk(XENLOG_ERR "%s %08x %08x %08x %08x\n",
code_str, entry[0], entry[1], entry[2], entry[3]);
-
- memset(entry, 0, IOMMU_EVENT_LOG_ENTRY_SIZE);
}
static void iommu_check_event_log(struct amd_iommu *iommu)
@@ -627,31 +630,8 @@ void parse_ppr_log_entry(struct amd_iommu *iommu, u32 entry[])
{
u16 device_id;
- u8 bus, devfn, code;
+ u8 bus, devfn;
struct pci_dev *pdev;
- int count = 0;
-
- code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK,
- IOMMU_PPR_LOG_CODE_SHIFT);
-
- /*
- * Workaround for erratum 733:
- * It can happen that the tail pointer is updated before the actual entry
- * got written. As suggested by RevGuide, we initialize the event log
- * buffer to all zeros and clear ppr log entries after processing them.
- */
- while ( code == 0 )
- {
- if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) )
- {
- AMD_IOMMU_DEBUG("AMD-Vi: No ppr written to log\n");
- return;
- }
- udelay(1);
- barrier(); /* Prevent hoisting of the entry[] read. */
- code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK,
- IOMMU_PPR_LOG_CODE_SHIFT);
- }
/* here device_id is physical value */
device_id = iommu_get_devid_from_cmd(entry[0]);
@@ -664,8 +644,6 @@ void parse_ppr_log_entry(struct amd_iommu *iommu, u32 entry[])
if ( pdev )
guest_iommu_add_ppr_log(pdev->domain, entry);
-
- memset(entry, 0, IOMMU_PPR_LOG_ENTRY_SIZE);
}
static void iommu_check_ppr_log(struct amd_iommu *iommu)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next reply other threads:[~2020-02-14 18:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-14 18:55 Andrew Cooper [this message]
2020-02-17 16:18 ` [Xen-devel] [PATCH] AMD/IOMMU: Common the #732/#733 errata handling in iommu_read_log() Jan Beulich
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=20200214185539.7444-1-andrew.cooper3@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.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).