[PATCH 0 of 1] Intel VT-d fix.

[PATCH 0 of 1] Intel VT-d fix.

[Konrad Rzeszutek 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.

[PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC

[Konrad Rzeszutek 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) ||

Re: [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC

[Alex Williamson]

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) )

Re: [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if there is a missing DRHD entry for the IOAPIC

[Konrad Rzeszutek Wilk]

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) )
>