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

[Alex Williamson <alex.williamson@hp.com>]

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