* [PATCH 0 of 1] Intel VT-d fix.
@ 2010-03-09 17:04 Konrad Rzeszutek Wilk
2010-03-09 17:04 ` [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-03-09 17:04 UTC (permalink / raw)
To: xen-devel, keir.fraser; +Cc: konrad.wilk
On a SuperMicro X8DTN+ I found Linux baremetal booting fine, while
Xen would hang while enabling PCI-X devices, such as:
(XEN) CPU 10 initiali[ 5.118255] scsi0 : 3ware Storage Controller
[ 5.118898] 3w-xxxx: scsi0: Found a 3ware Storage Controller at 0xc100, IRQ: 52.
(XEN) do_IRQ: 1.40 No irq handler for vector (irq -1)
[ 25.711139] scsi 0:0:0:0: WARNING: Command (0x12) timed out, resetting card.
[ 47.191274] scsi 0:0:0:0: WARNING: Command (0x0) timed out, resetting card.
[ 58.671124] scsi 0:0:0:0: Device offlined - not ready after error recovery
[ 79.711194] scsi 0:0:1:0: WARNING: Command (0x12) timed out, resetting card.
[ 101.191071] scsi 0:0:1:0: WARNING: Command (0x0) timed out, resetting card.
[ 112.671065] scsi 0:0:1:0: Device offlined - not ready after error recovery
[ 133.710995] scsi 0:0:2:0: WARNING: Command (0x12) timed out, resetting card.
[ 155.190891] scsi 0:0:2:0: WARNING: Command (0x0) timed out, resetting card.
Traced it down the fact that I've three IOAPICs, but only one DRHD. The
DRHD covers the first IOAPIC, but not the other two. Well, the PCI-X device
was on the last IOAPIC, which was not under the DRHD control.
Linux upstream has a check where it looks for all the IOAPICs covered by the DRHDs
and compares them with the IOAPICs it found via MADT. If the count is off it
disables x2APIC.
This patch follows that logic.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC
2010-03-09 17:04 [PATCH 0 of 1] Intel VT-d fix Konrad Rzeszutek Wilk
@ 2010-03-09 17:04 ` Konrad Rzeszutek Wilk
2010-03-11 6:04 ` Alex Williamson
0 siblings, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-03-09 17:04 UTC (permalink / raw)
To: xen-devel, keir.fraser; +Cc: konrad.wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
# Date 1268154140 18000
# Node ID 81272f761f631d3c0929642fc833b66243b7d2bc
# Parent b8d2a4134a6823f6d5179928a0618eaf33be4684
Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC.
Follow the Linux kernel lead in which the x2APIC is
only turned on only if there is an DRHD entry for all
IOAPICs in the system. If we don't do this
we might enable x2APIC and see various devices not covered by the
IOAPIC mentioned in DRHD, not receive any interrupts.
Workaround is to use 'x2apic=0' on command line.
diff -r b8d2a4134a68 -r 81272f761f63 xen/drivers/passthrough/vtd/intremap.c
--- a/xen/drivers/passthrough/vtd/intremap.c Wed Mar 03 17:41:58 2010 +0000
+++ b/xen/drivers/passthrough/vtd/intremap.c Tue Mar 09 12:02:20 2010 -0500
@@ -127,10 +127,17 @@
int iommu_supports_eim(void)
{
struct acpi_drhd_unit *drhd;
+ int apic;
if ( !iommu_enabled || !iommu_qinval || !iommu_intremap )
return 0;
+ // We MUST have a DRHD unit for each IOAPIC.
+ for ( apic = 0; apic < nr_ioapics; apic++ )
+ {
+ if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
+ return 0;
+ }
for_each_drhd_unit ( drhd )
if ( !ecap_queued_inval(drhd->ecap) ||
!ecap_intr_remap(drhd->ecap) ||
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC
2010-03-09 17:04 ` [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC Konrad Rzeszutek Wilk
@ 2010-03-11 6:04 ` Alex Williamson
2010-03-11 20:25 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 4+ messages in thread
From: Alex Williamson @ 2010-03-11 6:04 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, keir.fraser
On Tue, 2010-03-09 at 12:04 -0500, Konrad Rzeszutek Wilk wrote:
> # HG changeset patch
> # User Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> # Date 1268154140 18000
> # Node ID 81272f761f631d3c0929642fc833b66243b7d2bc
> # Parent b8d2a4134a6823f6d5179928a0618eaf33be4684
> Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC.
>
> Follow the Linux kernel lead in which the x2APIC is
> only turned on only if there is an DRHD entry for all
> IOAPICs in the system. If we don't do this
> we might enable x2APIC and see various devices not covered by the
> IOAPIC mentioned in DRHD, not receive any interrupts.
>
> Workaround is to use 'x2apic=0' on command line.
>
> diff -r b8d2a4134a68 -r 81272f761f63 xen/drivers/passthrough/vtd/intremap.c
> --- a/xen/drivers/passthrough/vtd/intremap.c Wed Mar 03 17:41:58 2010 +0000
> +++ b/xen/drivers/passthrough/vtd/intremap.c Tue Mar 09 12:02:20 2010 -0500
> @@ -127,10 +127,17 @@
> int iommu_supports_eim(void)
> {
> struct acpi_drhd_unit *drhd;
> + int apic;
>
> if ( !iommu_enabled || !iommu_qinval || !iommu_intremap )
> return 0;
>
> + // We MUST have a DRHD unit for each IOAPIC.
> + for ( apic = 0; apic < nr_ioapics; apic++ )
> + {
> + if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
This has a pretty serious bug. ioapic_to_iommu() gets returned
drhd->iommu. However, drhd->iommu isn't allocated until part of
iommu_setup(), which is called after enable_x2apic(). Has this ever
worked? Here's the fix.
Signed-off-by: Alex Williamson <alex.williamson@hp.com>
--
diff -r 132ac04cbdba xen/drivers/passthrough/vtd/intremap.c
--- a/xen/drivers/passthrough/vtd/intremap.c Tue Mar 09 18:18:19 2010 +0000
+++ b/xen/drivers/passthrough/vtd/intremap.c Wed Mar 10 22:58:08 2010 -0700
@@ -134,7 +134,7 @@
/* We MUST have a DRHD unit for each IOAPIC. */
for ( apic = 0; apic < nr_ioapics; apic++ )
- if ( !ioapic_to_iommu(IO_APIC_ID(apic)) )
+ if ( !ioapic_to_drhd(IO_APIC_ID(apic)) )
return 0;
if ( list_empty(&acpi_drhd_units) )
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC
2010-03-11 6:04 ` Alex Williamson
@ 2010-03-11 20:25 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-03-11 20:25 UTC (permalink / raw)
To: Alex Williamson; +Cc: xen-devel, keir.fraser
On Wed, Mar 10, 2010 at 11:04:39PM -0700, Alex Williamson wrote:
> On Tue, 2010-03-09 at 12:04 -0500, Konrad Rzeszutek Wilk wrote:
> > # HG changeset patch
> > # User Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > # Date 1268154140 18000
> > # Node ID 81272f761f631d3c0929642fc833b66243b7d2bc
> > # Parent b8d2a4134a6823f6d5179928a0618eaf33be4684
> > Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC.
> >
> > Follow the Linux kernel lead in which the x2APIC is
> > only turned on only if there is an DRHD entry for all
> > IOAPICs in the system. If we don't do this
> > we might enable x2APIC and see various devices not covered by the
> > IOAPIC mentioned in DRHD, not receive any interrupts.
> >
> > Workaround is to use 'x2apic=0' on command line.
> >
> > diff -r b8d2a4134a68 -r 81272f761f63 xen/drivers/passthrough/vtd/intremap.c
> > --- a/xen/drivers/passthrough/vtd/intremap.c Wed Mar 03 17:41:58 2010 +0000
> > +++ b/xen/drivers/passthrough/vtd/intremap.c Tue Mar 09 12:02:20 2010 -0500
> > @@ -127,10 +127,17 @@
> > int iommu_supports_eim(void)
> > {
> > struct acpi_drhd_unit *drhd;
> > + int apic;
> >
> > if ( !iommu_enabled || !iommu_qinval || !iommu_intremap )
> > return 0;
> >
> > + // We MUST have a DRHD unit for each IOAPIC.
> > + for ( apic = 0; apic < nr_ioapics; apic++ )
> > + {
> > + if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
>
>
> This has a pretty serious bug. ioapic_to_iommu() gets returned
> drhd->iommu. However, drhd->iommu isn't allocated until part of
Yikes!
> iommu_setup(), which is called after enable_x2apic(). Has this ever
> worked? Here's the fix.
Yes. But I only have faulty hardware (ie, the is one IOAPIC in the DRHD,
and three existing IOAPICS), so I couldn't test it on working hardware.
Thank you for spotting this mistake and coming to resuce with a proper
patch.
Much appreciated.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> --
>
> diff -r 132ac04cbdba xen/drivers/passthrough/vtd/intremap.c
> --- a/xen/drivers/passthrough/vtd/intremap.c Tue Mar 09 18:18:19 2010 +0000
> +++ b/xen/drivers/passthrough/vtd/intremap.c Wed Mar 10 22:58:08 2010 -0700
> @@ -134,7 +134,7 @@
>
> /* We MUST have a DRHD unit for each IOAPIC. */
> for ( apic = 0; apic < nr_ioapics; apic++ )
> - if ( !ioapic_to_iommu(IO_APIC_ID(apic)) )
> + if ( !ioapic_to_drhd(IO_APIC_ID(apic)) )
> return 0;
>
> if ( list_empty(&acpi_drhd_units) )
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-11 20:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-09 17:04 [PATCH 0 of 1] Intel VT-d fix Konrad Rzeszutek Wilk
2010-03-09 17:04 ` [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC Konrad Rzeszutek Wilk
2010-03-11 6:04 ` Alex Williamson
2010-03-11 20:25 ` Konrad Rzeszutek Wilk
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).