On 22.05.12 at 15:56, Wei Wang<wei.wang2@xxxxxxx> wrote:
Hi Jan
This patch implements the suggested workaround for erratum 732& 733. It
is mostly derived from the Linux patch recently posted. Please review it.
Thanks,
Wei
Looks good in principle, but came out with newline damage. Can
you please resend as attachment, ideally at once fixing a few
(mostly cosmetic) things:
# HG changeset patch
# User Wei Wang<wei.wang2@xxxxxxx>
# Date 1337690747 -7200
# Node ID 06aebc361de7f308b262b008153ae4549c4480c2
# Parent 592d15bd4d5ec58486d32ee9998319e7c95fcd66
amd iommu: Add workaround for erratum 733& 733
732& 733
Signed-off-by: Wei Wang<wei.wang2@xxxxxxx>
diff -r 592d15bd4d5e -r 06aebc361de7
xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c Fri May 18 16:19:21
2012 +0100
+++ b/xen/drivers/passthrough/amd/iommu_init.c Tue May 22 14:45:47
2012 +0200
@@ -29,6 +29,7 @@
#include<asm/hvm/svm/amd-iommu-proto.h>
#include<asm-x86/fixmap.h>
#include<mach_apic.h>
+#include<xen/delay.h>
static int __initdata nr_amd_iommus;
@@ -566,6 +567,7 @@ static void parse_event_log_entry(struct
u16 domain_id, device_id, bdf, cword;
u32 code;
u64 *addr;
+ int count = 0;
char * event_str[] = {"ILLEGAL_DEV_TABLE_ENTRY",
"IO_PAGE_FAULT",
"DEV_TABLE_HW_ERROR",
@@ -575,9 +577,27 @@ static void parse_event_log_entry(struct
"IOTLB_INV_TIMEOUT",
"INVALID_DEV_REQUEST"};
+retry:
code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK,
IOMMU_EVENT_CODE_SHIFT);
+ /* Workaround for erratum 732.
/*
* Workaround for erratum 732.
+ * it can happen that the tail pointer is updated before the actual
entry
+ * is written. Suggested by RevGuide, we initialize the event log
buffer to
+ * all zeros and write event log entries to zero after they are
processed.
+ */
+
+ if ( code == 0 )
+ {
+ if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) )
+ {
+ AMD_IOMMU_DEBUG("AMD-Vi: No event written to event log\n");
Perhaps remove the second "event".
+ return;
+ }
+ udelay(1);
+ goto retry;
+ }
+
if ( (code> IOMMU_EVENT_INVALID_DEV_REQUEST) ||
(code< IOMMU_EVENT_ILLEGAL_DEV_TABLE_ENTRY) )
{
@@ -615,6 +635,8 @@ static void parse_event_log_entry(struct
AMD_IOMMU_DEBUG("event 0x%08x 0x%08x 0x%08x 0x%08x\n", 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)
@@ -646,10 +668,32 @@ void parse_ppr_log_entry(struct amd_iomm
{
u16 device_id;
- u8 bus, devfn;
+ u8 bus, devfn, code;
struct pci_dev *pdev;
struct domain *d;
+ int count = 0;
+retry:
+ code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK,
+ IOMMU_PPR_LOG_CODE_SHIFT);
+
+ /* Workaround for erratum 733.
See above.
+ * it can happen that the tail pointer is updated before the actual
entry
+ * is written. Suggested by RevGuide, we initialize the ppr log
buffer to
+ * all zeros and write ppr log entries to zero after they are
processed.
+ */
+
+ if ( code == 0 )
+ {
+ if ( unlikely(++count == IOMMU_LOG_ENTRY_TIMEOUT) )
+ {
+ AMD_IOMMU_DEBUG("AMD-Vi: No ppr written to ppr log\n");
Perhaps remove the second "ppr".
+ return;
+ }
+ udelay(1);
+ goto retry;
+ }
+
/* here device_id is physical value */
device_id = iommu_get_devid_from_cmd(entry[0]);
bus = PCI_BUS(device_id);
@@ -665,6 +709,8 @@ void parse_ppr_log_entry(struct amd_iomm
d = pdev->domain;
guest_iommu_add_ppr_log(d, entry);
+
+ memset(entry, 0, IOMMU_PPR_LOG_ENTRY_SIZE);
}
static void iommu_check_ppr_log(struct amd_iommu *iommu)
I'd personally also prefer the loops to be written as such (i.e.
without goto-s).
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h Fri May 18
16:19:21 2012 +0100
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h Tue May 22
14:45:47 2012 +0200
@@ -301,6 +301,10 @@
#define IOMMU_PPR_LOG_TAIL_OFFSET 0x2038
#define IOMMU_PPR_LOG_DEVICE_ID_MASK 0x0000FFFF
#define IOMMU_PPR_LOG_DEVICE_ID_SHIFT 0
+#define IOMMU_PPR_LOG_CODE_MASK 0xF0000000
+#define IOMMU_PPR_LOG_CODE_SHIFT 28
+
+#define IOMMU_LOG_ENTRY_TIMEOUT 100000
That's rather long a timeout (100ms) for a busy loop - is that
really necessary?
Jan
/* Control Register */
#define IOMMU_CONTROL_MMIO_OFFSET 0x18
