This is to improve dealing with systems that have incorrect ACPI tables, an erratum workaround ported over from Linux, and a little bit of cleanup. 1: don''t BUG() when we don''t have to 2: cover all functions of a device even if ACPI only tells us of func 0 3: Family15h Model10-1Fh erratum 746 Workaround 4: use __ioapic_read_entry() instead of open coding it Signed-off-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich
2013-Feb-15 16:19 UTC
[PATCH 1/4] AMD IOMMU: don''t BUG() when we don''t have to
find_iommu_for_device() can easily return NULL instead, as all of its
callers are prepared for that.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -32,8 +32,8 @@ struct amd_iommu *find_iommu_for_device(
{
struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg);
- BUG_ON ( bdf >= ivrs_bdf_entries );
- return ivrs_mappings ? ivrs_mappings[bdf].iommu : NULL;
+ return ivrs_mappings && bdf < ivrs_bdf_entries ?
ivrs_mappings[bdf].iommu
+ : NULL;
}
/*
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Jan Beulich
2013-Feb-15 16:20 UTC
[PATCH 2/4] AMD IOMMU: cover all functions of a device even if ACPI only tells us of func 0
This ought to work as all functions of a device have the same place in
the bus topology, i.e. use the same IOMMU.
Also fix the type of ivrs_bdf_entries (when it''s ''unsigned
short'' and
the last device found on a segment is ff:1f.x, it would otherwise end
up being zero).
And drop the bogus ''last_bdf'' static variable, which
conflicted anyway
with various functions'' parameters.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -54,8 +54,6 @@ union acpi_ivhd_device {
struct acpi_ivrs_device8c special;
};
-static unsigned short __initdata last_bdf;
-
static void __init add_ivrs_mapping_entry(
u16 bdf, u16 alias_id, u8 flags, struct amd_iommu *iommu)
{
@@ -991,6 +989,7 @@ static int __init get_last_bdf_ivhd(
{
const union acpi_ivhd_device *ivhd_device;
u16 block_length, dev_length;
+ int last_bdf = 0;
if ( ivhd_block->header.length < sizeof(*ivhd_block) )
{
@@ -1051,27 +1050,34 @@ static int __init get_last_bdf_ivhd(
return -ENODEV;
}
- return 0;
+ return last_bdf;
}
static int __init get_last_bdf_acpi(struct acpi_table_header *table)
{
const struct acpi_ivrs_header *ivrs_block;
unsigned long length = sizeof(struct acpi_table_ivrs);
+ int last_bdf = 0;
while ( table->length > (length + sizeof(*ivrs_block)) )
{
ivrs_block = (struct acpi_ivrs_header *)((u8 *)table + length);
if ( table->length < (length + ivrs_block->length) )
return -ENODEV;
- if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE &&
- get_last_bdf_ivhd(
+ if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE )
+ {
+ int ret = get_last_bdf_ivhd(
container_of(ivrs_block, const struct acpi_ivrs_hardware,
- header)) != 0 )
- return -ENODEV;
+ header));
+
+ if ( ret < 0 )
+ return ret;
+ UPDATE_LAST_BDF(ret);
+ }
length += ivrs_block->length;
}
- return 0;
+
+ return last_bdf;
}
int __init amd_iommu_detect_acpi(void)
@@ -1081,8 +1087,9 @@ int __init amd_iommu_detect_acpi(void)
int __init amd_iommu_get_ivrs_dev_entries(void)
{
- acpi_table_parse(ACPI_SIG_IVRS, get_last_bdf_acpi);
- return last_bdf + 1;
+ int ret = acpi_table_parse(ACPI_SIG_IVRS, get_last_bdf_acpi);
+
+ return ret < 0 ? ret : (ret | PCI_FUNC(~0)) + 1;
}
int __init amd_iommu_update_ivrs_mapping_acpi(void)
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -35,7 +35,7 @@ static int __initdata nr_amd_iommus;
static struct tasklet amd_iommu_irq_tasklet;
-unsigned short ivrs_bdf_entries;
+unsigned int __read_mostly ivrs_bdf_entries;
static struct radix_tree_root ivrs_maps;
struct list_head amd_iommu_head;
struct table_struct device_table;
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -28,12 +28,38 @@
#include <asm/hvm/svm/amd-iommu-proto.h>
#include "../ats.h"
+static bool_t __read_mostly init_done;
+
struct amd_iommu *find_iommu_for_device(int seg, int bdf)
{
struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg);
- return ivrs_mappings && bdf < ivrs_bdf_entries ?
ivrs_mappings[bdf].iommu
- : NULL;
+ if ( !ivrs_mappings || bdf >= ivrs_bdf_entries )
+ return NULL;
+
+ if ( unlikely(!ivrs_mappings[bdf].iommu) && likely(init_done) )
+ {
+ unsigned int bd0 = bdf & ~PCI_FUNC(~0);
+
+ if ( ivrs_mappings[bd0].iommu )
+ {
+ struct ivrs_mappings tmp = ivrs_mappings[bd0];
+
+ tmp.iommu = NULL;
+ if ( tmp.dte_requestor_id == bd0 )
+ tmp.dte_requestor_id = bdf;
+ ivrs_mappings[bdf] = tmp;
+
+ printk(XENLOG_WARNING "%04x:%02x:%02x.%u not found in ACPI
tables;"
+ " using same IOMMU as function 0\n",
+ seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+
+ /* write iommu field last */
+ ivrs_mappings[bdf].iommu = ivrs_mappings[bd0].iommu;
+ }
+ }
+
+ return ivrs_mappings[bdf].iommu;
}
/*
@@ -179,6 +205,8 @@ int __init amd_iov_detect(void)
return -ENODEV;
}
+ init_done = 1;
+
/*
* AMD IOMMUs don''t distinguish between vectors destined for
* different cpus when doing interrupt remapping. This means
--- a/xen/include/asm-x86/amd-iommu.h
+++ b/xen/include/asm-x86/amd-iommu.h
@@ -125,7 +125,7 @@ struct ivrs_mappings {
u8 device_flags;
};
-extern unsigned short ivrs_bdf_entries;
+extern unsigned int ivrs_bdf_entries;
struct ivrs_mappings *get_ivrs_mappings(u16 seg);
int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Jan Beulich
2013-Feb-15 16:21 UTC
[PATCH 3/4] IOMMU, AMD Family15h Model10-1Fh erratum 746 Workaround
The IOMMU may stop processing page translations due to a perceived lack
of credits for writing upstream peripheral page service request (PPR)
or event logs. If the L2B miscellaneous clock gating feature is enabled
the IOMMU does not properly register credits after the log request has
completed, leading to a potential system hang.
BIOSes are supposed to disable L2B micellaneous clock gating by setting
L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b. This
patch corrects that for those which do not enable this workaround.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -792,6 +792,42 @@ static bool_t __init set_iommu_interrupt
return 1;
}
+/*
+ * Family15h Model 10h-1fh erratum 746 (IOMMU Logging May Stall Translations)
+ * Workaround:
+ * BIOS should disable L2B micellaneous clock gating by setting
+ * L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b
+ */
+static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
+{
+ u32 value;
+ u8 bus = PCI_BUS(iommu->bdf);
+ u8 dev = PCI_SLOT(iommu->bdf);
+ u8 func = PCI_FUNC(iommu->bdf);
+
+ if ( (boot_cpu_data.x86 != 0x15) ||
+ (boot_cpu_data.x86_model < 0x10) ||
+ (boot_cpu_data.x86_model > 0x1f) )
+ return;
+
+ pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
+ value = pci_conf_read32(iommu->seg, bus, dev, func, 0xf4);
+
+ if ( value & (1 << 2) )
+ return;
+
+ /* Select NB indirect register 0x90 and enable writing */
+ pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90 | (1 <<
8));
+
+ pci_conf_write32(iommu->seg, bus, dev, func, 0xf4, value | (1 <<
2));
+ printk(XENLOG_INFO
+ "AMD-Vi: Applying erratum 746 workaround for IOMMU at
%04x:%02x:%02x.%u\n",
+ iommu->seg, bus, dev, func);
+
+ /* Clear the enable writing bit */
+ pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90);
+}
+
static void enable_iommu(struct amd_iommu *iommu)
{
unsigned long flags;
@@ -805,6 +841,8 @@ static void enable_iommu(struct amd_iomm
return;
}
+ amd_iommu_erratum_746_workaround(iommu);
+
register_iommu_dev_table_in_mmio_space(iommu);
register_iommu_cmd_buffer_in_mmio_space(iommu);
register_iommu_event_log_in_mmio_space(iommu);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Jan Beulich
2013-Feb-15 16:22 UTC
[PATCH 4/4] AMD IOMMU: use __ioapic_read_entry() instead of open coding it
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -144,7 +144,7 @@ static void update_intremap_entry_from_i
int __init amd_iommu_setup_ioapic_remapping(void)
{
- struct IO_APIC_route_entry rte = {0};
+ struct IO_APIC_route_entry rte;
unsigned long flags;
u32* entry;
int apic, pin;
@@ -159,9 +159,7 @@ int __init amd_iommu_setup_ioapic_remapp
{
for ( pin = 0; pin < nr_ioapic_entries[apic]; pin++ )
{
- *(((int *)&rte) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
- *(((int *)&rte) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
-
+ rte = __ioapic_read_entry(apic, pin, 1);
if ( rte.mask == 1 )
continue;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Boris Ostrovsky
2013-Feb-15 17:21 UTC
Re: [PATCH 2/4] AMD IOMMU: cover all functions of a device even if ACPI only tells us of func 0
On 02/15/2013 11:20 AM, Jan Beulich wrote:> > static int __init get_last_bdf_acpi(struct acpi_table_header *table) > { > const struct acpi_ivrs_header *ivrs_block; > unsigned long length = sizeof(struct acpi_table_ivrs); > + int last_bdf = 0; > > while ( table->length > (length + sizeof(*ivrs_block)) ) > { > ivrs_block = (struct acpi_ivrs_header *)((u8 *)table + length); > if ( table->length < (length + ivrs_block->length) ) > return -ENODEV; > - if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE && > - get_last_bdf_ivhd( > + if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE ) > + { > + int ret = get_last_bdf_ivhd( > container_of(ivrs_block, const struct acpi_ivrs_hardware, > - header)) != 0 ) > - return -ENODEV; > + header)); > + > + if ( ret < 0 ) > + return ret; > + UPDATE_LAST_BDF(ret);Why do we need UPDATE_LAST_BDF () here? It is updated in get_last_bdf_ivhd () above.> + } > length += ivrs_block->length; > } > - return 0; > + > + return last_bdf; > }...> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c > @@ -28,12 +28,38 @@ > #include <asm/hvm/svm/amd-iommu-proto.h> > #include "../ats.h" > > +static bool_t __read_mostly init_done; > + > struct amd_iommu *find_iommu_for_device(int seg, int bdf) > { > struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg); > > - return ivrs_mappings && bdf < ivrs_bdf_entries ? ivrs_mappings[bdf].iommu > - : NULL; > + if ( !ivrs_mappings || bdf >= ivrs_bdf_entries ) > + return NULL; > + > + if ( unlikely(!ivrs_mappings[bdf].iommu) && likely(init_done) ) > + { > + unsigned int bd0 = bdf & ~PCI_FUNC(~0); > + > + if ( ivrs_mappings[bd0].iommu ) > + { > + struct ivrs_mappings tmp = ivrs_mappings[bd0]; > + > + tmp.iommu = NULL; > + if ( tmp.dte_requestor_id == bd0 ) > + tmp.dte_requestor_id = bdf;Is it possible to have tmp.dte_requestor_id != bd0 -boris
Boris Ostrovsky
2013-Feb-15 17:28 UTC
Re: [PATCH 4/4] AMD IOMMU: use __ioapic_read_entry() instead of open coding it
On 02/15/2013 11:22 AM, Jan Beulich wrote:> Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>> > --- a/xen/drivers/passthrough/amd/iommu_intr.c > +++ b/xen/drivers/passthrough/amd/iommu_intr.c > @@ -144,7 +144,7 @@ static void update_intremap_entry_from_i > > int __init amd_iommu_setup_ioapic_remapping(void) > { > - struct IO_APIC_route_entry rte = {0}; > + struct IO_APIC_route_entry rte; > unsigned long flags; > u32* entry; > int apic, pin; > @@ -159,9 +159,7 @@ int __init amd_iommu_setup_ioapic_remapp > { > for ( pin = 0; pin < nr_ioapic_entries[apic]; pin++ ) > { > - *(((int *)&rte) + 1) = io_apic_read(apic, 0x11 + 2 * pin); > - *(((int *)&rte) + 0) = io_apic_read(apic, 0x10 + 2 * pin); > - > + rte = __ioapic_read_entry(apic, pin, 1); > if ( rte.mask == 1 ) > continue; > > > >
Boris Ostrovsky
2013-Feb-15 17:34 UTC
Re: [PATCH 1/4] AMD IOMMU: don''t BUG() when we don''t have to
On 02/15/2013 11:19 AM, Jan Beulich wrote:> find_iommu_for_device() can easily return NULL instead, as all of its > callers are prepared for that.This patch is obsoleted by the second patch ("[PATCH 2/4] AMD IOMMU: cover all functions of a device even if ACPI only tells us of func 0"), isn''t it? -boris> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c > @@ -32,8 +32,8 @@ struct amd_iommu *find_iommu_for_device( > { > struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg); > > - BUG_ON ( bdf >= ivrs_bdf_entries ); > - return ivrs_mappings ? ivrs_mappings[bdf].iommu : NULL; > + return ivrs_mappings && bdf < ivrs_bdf_entries ? ivrs_mappings[bdf].iommu > + : NULL; > } > > /* > > >
Jan Beulich
2013-Feb-18 07:58 UTC
Re: [PATCH 1/4] AMD IOMMU: don''t BUG() when we don''t have to
>>> On 15.02.13 at 18:34, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote: > On 02/15/2013 11:19 AM, Jan Beulich wrote: >> find_iommu_for_device() can easily return NULL instead, as all of its >> callers are prepared for that. > > This patch is obsoleted by the second patch ("[PATCH 2/4] AMD IOMMU: > cover all functions of a device even if ACPI only tells us of func 0"), > isn''t it?Yes it is, but it is an (I think) obviously correct thing to do, and hence much more of a backporting candidate than the other one. Jan
Jan Beulich
2013-Feb-18 08:00 UTC
Re: [PATCH 2/4] AMD IOMMU: cover all functions of a device even if ACPI only tells us of func 0
>>> On 15.02.13 at 18:21, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote: > On 02/15/2013 11:20 AM, Jan Beulich wrote: >> >> static int __init get_last_bdf_acpi(struct acpi_table_header *table) >> { >> const struct acpi_ivrs_header *ivrs_block; >> unsigned long length = sizeof(struct acpi_table_ivrs); >> + int last_bdf = 0; >> >> while ( table->length > (length + sizeof(*ivrs_block)) ) >> { >> ivrs_block = (struct acpi_ivrs_header *)((u8 *)table + length); >> if ( table->length < (length + ivrs_block->length) ) >> return -ENODEV; >> - if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE && >> - get_last_bdf_ivhd( >> + if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE ) >> + { >> + int ret = get_last_bdf_ivhd( >> container_of(ivrs_block, const struct acpi_ivrs_hardware, >> - header)) != 0 ) >> - return -ENODEV; >> + header)); >> + >> + if ( ret < 0 ) >> + return ret; >> + UPDATE_LAST_BDF(ret); > > Why do we need UPDATE_LAST_BDF () here? It is updated in > get_last_bdf_ivhd () above.No, because "last_bdf" now is a local variable.>> + } >> length += ivrs_block->length; >> } >> - return 0; >> + >> + return last_bdf; >> } > > ... > >> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c >> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c >> @@ -28,12 +28,38 @@ >> #include <asm/hvm/svm/amd-iommu-proto.h> >> #include "../ats.h" >> >> +static bool_t __read_mostly init_done; >> + >> struct amd_iommu *find_iommu_for_device(int seg, int bdf) >> { >> struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg); >> >> - return ivrs_mappings && bdf < ivrs_bdf_entries ? ivrs_mappings[bdf].iommu >> - : NULL; >> + if ( !ivrs_mappings || bdf >= ivrs_bdf_entries ) >> + return NULL; >> + >> + if ( unlikely(!ivrs_mappings[bdf].iommu) && likely(init_done) ) >> + { >> + unsigned int bd0 = bdf & ~PCI_FUNC(~0); >> + >> + if ( ivrs_mappings[bd0].iommu ) >> + { >> + struct ivrs_mappings tmp = ivrs_mappings[bd0]; >> + >> + tmp.iommu = NULL; >> + if ( tmp.dte_requestor_id == bd0 ) >> + tmp.dte_requestor_id = bdf; > > Is it possible to have tmp.dte_requestor_id != bd0Sure - when there was an alias entry for it (I assume e.g. in the case of the device sitting behind a legacy PCI bridge). Jan
Suravee Suthikulanit
2013-Feb-25 21:33 UTC
Re: [PATCH 3/4] IOMMU, AMD Family15h Model10-1Fh erratum 746 Workaround
Jan, This patch looks good. I have tested on the system I have and it does what it''s supposed to. Thank you for porting the patch over from Linux. Suravee On 2/15/2013 10:21 AM, Jan Beulich wrote:> The IOMMU may stop processing page translations due to a perceived lack > of credits for writing upstream peripheral page service request (PPR) > or event logs. If the L2B miscellaneous clock gating feature is enabled > the IOMMU does not properly register credits after the log request has > completed, leading to a potential system hang. > > BIOSes are supposed to disable L2B micellaneous clock gating by setting > L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b. This > patch corrects that for those which do not enable this workaround. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -792,6 +792,42 @@ static bool_t __init set_iommu_interrupt > return 1; > } > > +/* > + * Family15h Model 10h-1fh erratum 746 (IOMMU Logging May Stall Translations) > + * Workaround: > + * BIOS should disable L2B micellaneous clock gating by setting > + * L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b > + */ > +static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu) > +{ > + u32 value; > + u8 bus = PCI_BUS(iommu->bdf); > + u8 dev = PCI_SLOT(iommu->bdf); > + u8 func = PCI_FUNC(iommu->bdf); > + > + if ( (boot_cpu_data.x86 != 0x15) || > + (boot_cpu_data.x86_model < 0x10) || > + (boot_cpu_data.x86_model > 0x1f) ) > + return; > + > + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90); > + value = pci_conf_read32(iommu->seg, bus, dev, func, 0xf4); > + > + if ( value & (1 << 2) ) > + return; > + > + /* Select NB indirect register 0x90 and enable writing */ > + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90 | (1 << 8)); > + > + pci_conf_write32(iommu->seg, bus, dev, func, 0xf4, value | (1 << 2)); > + printk(XENLOG_INFO > + "AMD-Vi: Applying erratum 746 workaround for IOMMU at %04x:%02x:%02x.%u\n", > + iommu->seg, bus, dev, func); > + > + /* Clear the enable writing bit */ > + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90); > +} > + > static void enable_iommu(struct amd_iommu *iommu) > { > unsigned long flags; > @@ -805,6 +841,8 @@ static void enable_iommu(struct amd_iomm > return; > } > > + amd_iommu_erratum_746_workaround(iommu); > + > register_iommu_dev_table_in_mmio_space(iommu); > register_iommu_cmd_buffer_in_mmio_space(iommu); > register_iommu_event_log_in_mmio_space(iommu); > > >
Jan Beulich
2013-Feb-26 07:42 UTC
Re: [PATCH 3/4] IOMMU, AMD Family15h Model10-1Fh erratum 746 Workaround
>>> On 25.02.13 at 22:33, Suravee Suthikulanit <suravee.suthikulpanit@amd.com> wrote: > This patch looks good. I have tested on the system I have and it does > what it''s supposed to. Thank you for porting the patch over from Linux.Thanks - I take this as an ack. Jan> On 2/15/2013 10:21 AM, Jan Beulich wrote: >> The IOMMU may stop processing page translations due to a perceived lack >> of credits for writing upstream peripheral page service request (PPR) >> or event logs. If the L2B miscellaneous clock gating feature is enabled >> the IOMMU does not properly register credits after the log request has >> completed, leading to a potential system hang. >> >> BIOSes are supposed to disable L2B micellaneous clock gating by setting >> L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b. This >> patch corrects that for those which do not enable this workaround. >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/drivers/passthrough/amd/iommu_init.c >> +++ b/xen/drivers/passthrough/amd/iommu_init.c >> @@ -792,6 +792,42 @@ static bool_t __init set_iommu_interrupt >> return 1; >> } >> >> +/* >> + * Family15h Model 10h-1fh erratum 746 (IOMMU Logging May Stall > Translations) >> + * Workaround: >> + * BIOS should disable L2B micellaneous clock gating by setting >> + * L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b >> + */ >> +static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu) >> +{ >> + u32 value; >> + u8 bus = PCI_BUS(iommu->bdf); >> + u8 dev = PCI_SLOT(iommu->bdf); >> + u8 func = PCI_FUNC(iommu->bdf); >> + >> + if ( (boot_cpu_data.x86 != 0x15) || >> + (boot_cpu_data.x86_model < 0x10) || >> + (boot_cpu_data.x86_model > 0x1f) ) >> + return; >> + >> + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90); >> + value = pci_conf_read32(iommu->seg, bus, dev, func, 0xf4); >> + >> + if ( value & (1 << 2) ) >> + return; >> + >> + /* Select NB indirect register 0x90 and enable writing */ >> + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90 | (1 << 8)); >> + >> + pci_conf_write32(iommu->seg, bus, dev, func, 0xf4, value | (1 << 2)); >> + printk(XENLOG_INFO >> + "AMD-Vi: Applying erratum 746 workaround for IOMMU at > %04x:%02x:%02x.%u\n", >> + iommu->seg, bus, dev, func); >> + >> + /* Clear the enable writing bit */ >> + pci_conf_write32(iommu->seg, bus, dev, func, 0xf0, 0x90); >> +} >> + >> static void enable_iommu(struct amd_iommu *iommu) >> { >> unsigned long flags; >> @@ -805,6 +841,8 @@ static void enable_iommu(struct amd_iomm >> return; >> } >> >> + amd_iommu_erratum_746_workaround(iommu); >> + >> register_iommu_dev_table_in_mmio_space(iommu); >> register_iommu_cmd_buffer_in_mmio_space(iommu); >> register_iommu_event_log_in_mmio_space(iommu); >> >> >>