[PATCH] vt-d: ensure x2apic is not enabled accidently if no DRHD at all.

[PATCH] vt-d: ensure x2apic is not enabled accidently if no DRHD at all.

[Cui, Dexuan]

[-- Attachment #1: Type: text/plain, Size: 66 bytes --]

Thank Jan Beulich for pointing this out

Thanks,
-- Dexuan
 

[-- Attachment #2: do_not_enable_x2apic_if_no_drhd.patch --]
[-- Type: application/octet-stream, Size: 692 bytes --]

vt-d: ensure x2apic is not enabled accidently if no DRHD at all.

Thank Jan Beulich for pointing this out.

Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>

diff -r b8d2a4134a68 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	Thu Mar 04 12:05:31 2010 +0800
@@ -131,6 +131,9 @@ int iommu_supports_eim(void)
     if ( !iommu_enabled || !iommu_qinval || !iommu_intremap )
         return 0;
 
+    if ( list_empty(&acpi_drhd_units) )
+        return 0;
+
     for_each_drhd_unit ( drhd )
         if ( !ecap_queued_inval(drhd->ecap) ||
              !ecap_intr_remap(drhd->ecap) ||

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] vt-d: ensure x2apic is not enabled accidently if no DRHD at all.

[Konrad Rzeszutek Wilk]

On Thu, Mar 04, 2010 at 12:18:19PM +0800, Cui, Dexuan wrote:
> Thank Jan Beulich for pointing this out

You need more than that.

Here is what I see

(XEN) Enabling APIC mode:  Phys.  Using 3 I/O APICs
(XEN) ACPI: HPET id: 0x8086a301 base: 0xfed00000
(XEN) [VT-D]dmar.c:637: Host address width 40
(XEN) [VT-D]dmar.c:646: found ACPI_DMAR_DRHD:
(XEN) [VT-D]dmar.c:379:   dmaru->address = fe710000
(XEN) [VT-D]dmar.c:336:   IOAPIC: 0:1f.7
(XEN) [VT-D]dmar.c:391:   flags: INCLUDE_ALL

So one DRHD for three IOAPICs. In Linux kernel it would turn off x2APIC
(XEN) [VT-D]dmar.c:650: found ACPI_DMAR_RMRR:
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1d.0
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1d.1
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1d.2
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1d.7
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1a.0
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1a.1
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1a.2
(XEN) [VT-D]dmar.c:331:   endpoint: 0:1a.7
(XEN) [VT-D]dmar.c:540:   RMRR region: base_addr bf3f9000 end_address bf40c548
(XEN) PCI: MCFG configuration 0: base c0000000 segment 0 buses 0 - 255
(XEN) PCI: MCFG area at c0000000 reserved in E820
(XEN) iommu_supports_eim: 1 1 1
(XEN) iommu_supports_eim: 1 1 1
(XEN) Switched to APIC driver x2apic_cluster.

Whoopsie. Here it calls the routine you patched, which just checks to
see if there are DRHDs, but doesn't do a DRHD.count == IOAPCI.count.

(XEN) x2APIC mode enabled.
(XEN) Using ACPI (MADT) for SMP configuration information
(XEN) Using scheduler: SMP Credit Scheduler (credit)
(XEN) Initializing CPU#0
(XEN) Detected 2000.131 MHz processor.
(XEN) Initing memory sharing.
(XEN) CPU: L1 I cache: 32K, L1 D cache: 32K
(XEN) CPU: L2 cache: 256K
(XEN) CPU: L3 cache: 8192K
(XEN) CPU: Physical Processor ID: 0
(XEN) CPU: Processor Core ID: 0
(XEN) VMX: Supported advanced features:
(XEN)  - APIC MMIO access virtualisation
(XEN)  - APIC TPR shadow
(XEN)  - Extended Page Tables (EPT)
(XEN)  - Virtual-Processor Identifiers (VPID)
(XEN)  - Virtual NMI
(XEN)  - MSR direct-access bitmap
(XEN) HVM: ASIDs enabled. 
(XEN) HVM: VMX enabled
(XEN) HVM: Hardware Assisted Paging detected.
(XEN) Intel machine check reporting enabled on CPU#0.
(XEN) CPU0: Thermal monitoring enabled (TM1)
(XEN) [VT-D]iommu.c:1072: drhd->address = fe710000
(XEN) [VT-D]iommu.c:1073: iommu->reg = ffff82c3fff57000
(XEN) Intel VT-d Snoop Control supported.
(XEN) Intel VT-d DMA Passthrough not supported.
(XEN) Intel VT-d Queued Invalidation supported.
(XEN) Intel VT-d Interrupt Remapping supported.
(XEN) [VT-D]iommu.c:1838:  apic:0x0/0x3, 0x0
(XEN) [VT-D]iommu.c:1838:  apic:0x1/0x3, 0x2
(XEN) [VT-D]iommu.c:1845: ioapic_to_iommu: ioapic 0x1 (id: 0x2) is NULL! Will not try to enable Interrupt Remapping.

.. Here later on, it does the check and turns iommu_intr off.
(XEN) Intel VT-d Snoop Control enabled.
(XEN) Intel VT-d DMA Passthrough disabled .
(XEN) Intel VT-d Queued Invalidation enabled.
(XEN) Intel VT-d Interrupt Remapping disabled .
(XEN) Intel VT-D turned on!???
(XEN) IOMMU rc: 0, 1
(XEN) I/O virtualisation enabled
(XEN) I/O virtualisation for PV guests disabled
(XEN) CPU0: Intel Genuine Intel(R) CPU           @ 0000 @ 2.00GHz stepping 02
(XEN) Booting processor 1/1 eip 88000
(XEN) Initializing CPU#1
(XEN) x2APIC mode enabled.
(XEN) CPU: L1 I cache: 32K, L1 D cache: 32K

Here is the little debug patch I used

diff -r b8d2a4134a68 xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Wed Mar 03 17:41:58 2010 +0000
+++ b/xen/drivers/passthrough/iommu.c	Fri Mar 05 12:50:36 2010 -0500
@@ -282,6 +282,7 @@
     {
         rc = iommu_hardware_setup();
         iommu_enabled = (rc == 0);
+	printk("IOMMU rc: %d, %d\n", rc, iommu_enabled);
     }
 
     if ( force_iommu && !iommu_enabled )
diff -r b8d2a4134a68 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	Fri Mar 05 12:50:36 2010 -0500
@@ -128,9 +128,15 @@
 {
     struct acpi_drhd_unit *drhd;
 
+
+    printk("%s: %d %d %d\n", __FUNCTION__,iommu_enabled, iommu_qinval, iommu_intremap);
+
     if ( !iommu_enabled || !iommu_qinval || !iommu_intremap )
         return 0;
 
+    if ( list_empty(&acpi_drhd_units) )
+        return 0;
+
     for_each_drhd_unit ( drhd )
         if ( !ecap_queued_inval(drhd->ecap) ||
              !ecap_intr_remap(drhd->ecap) ||
diff -r b8d2a4134a68 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed Mar 03 17:41:58 2010 +0000
+++ b/xen/drivers/passthrough/vtd/iommu.c	Fri Mar 05 12:50:36 2010 -0500
@@ -1834,6 +1834,8 @@
         int apic;
         for ( apic = 0; apic < nr_ioapics; apic++ )
         {
+	    dprintk(XENLOG_INFO VTDPREFIX," apic:0x%x/0x%x, 0x%x\n",
+			apic, nr_ioapics, IO_APIC_ID(apic));
             if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
             {
                 iommu_intremap = 0;
@@ -1972,6 +1974,14 @@
     if ( init_vtd_hw() )
         goto error;
 
+#define P(p,s) printk("Intel VT-d %s %s.\n", s, (p)? "enabled" : "disabled ")
+    P(iommu_snoop, "Snoop Control");
+    P(iommu_passthrough, "DMA Passthrough");
+    P(iommu_qinval, "Queued Invalidation");
+    P(iommu_intremap, "Interrupt Remapping");
+#undef P
+
+    printk("Intel VT-D turned on!???\n");
     register_keyhandler('V', &dump_iommu_info_keyhandler);
 
     return 0;
@@ -1984,6 +1994,7 @@
     iommu_passthrough = 0;
     iommu_qinval = 0;
     iommu_intremap = 0;
+    printk("Intel VT-d is turned off due to errors.\n");
     return -ENOMEM;
 }
 
> 
> Thanks,
> -- Dexuan
>  


> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel