From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex Williamson <alex.williamson@hp.com>
Subject: Re: [PATCH 1 of 1] Intel VT-D: Don't turn x2APIC if
	there is a missing DRHD entry for the IOAPIC
Date: Wed, 10 Mar 2010 23:04:39 -0700
Message-ID: <1268287479.3015.96.camel@2710p.home>
References: <patchbomb.1268154240@phenom.dumpdata.com>
	<81272f761f631d3c0929.1268154241@phenom.dumpdata.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <81272f761f631d3c0929.1268154241@phenom.dumpdata.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com, keir.fraser@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org

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