Re: [Xen-devel] [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

Jan,

The current patch does not handle the case where the "handle" value is 
mis-configured in the IVRS.
I have also included the patch which has additional check.  Below is the 
output from the patch.
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x1 handle 0xff used_id 
0xa0
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC 
0x8, bdf=0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x2 handle 0 used_id 0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0
(XEN) AMD-Vi: IVHD Special: 0000:00:00.1 variety 0x1 handle 0xff used_id 0x1
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC 
0x8, bdf=0xa0
(XEN) AMD-Vi: IOMMU 0 Enabled.
(XEN) I/O virtualisation enabled

Suravee

------- PATCH  SNIPPET-------
From 86bf8c318a43e409d404377f4534fe586785a5c4 Mon Sep 17 00:00:00 2001
From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
Date: Wed, 28 Aug 2013 09:49:32 -0500
Subject: [PATCH] Add additional check invalid special->handle

This patch add an additional logic to check for the case when the 
speciail->handle
is invalid due to firmware bugs, and use the overide value instead.

Signed-off: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
---
 xen/drivers/passthrough/amd/iommu_acpi.c |   53 
++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
b/xen/drivers/passthrough/amd/iommu_acpi.c
index 89b359c..8fb4c3f 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -698,14 +698,16 @@ static u16 __init parse_ivhd_device_special(
         return 0;
     }

-    AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle 
%#x\n",
+    AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle 
%#x used_id %#x\n",
                     seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
-                    special->variety, special->handle);
+                    special->variety, special->handle, special->used_id);
     add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);

     switch ( special->variety )
     {
     case ACPI_IVHD_IOAPIC:
+    {
+        unsigned int apic_id = 0;
         if ( !iommu_intremap )
             break;
         /*
@@ -715,29 +717,43 @@ static u16 __init parse_ivhd_device_special(
          */
         for ( apic = 0; apic < nr_ioapics; apic++ )
         {
-            if ( IO_APIC_ID(apic) != special->handle )
-                continue;
+            apic_id = special->handle;

-            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
+            if ( IO_APIC_ID(apic) != apic_id )
+            {
+                /* Some BIOSes have invalid value in the special->handle.
+                 * Here we check to see if the user is overiding them 
from commandline
+                 */
+                if ( test_bit(IO_APIC_ID(apic), ioapic_cmdline) )
+                {
+                    apic_id = IO_APIC_ID(apic);
+                    AMD_IOMMU_DEBUG ("IVHD Special: Usinging command 
override "
+                        "value for IO-APIC %#x, bdf=%#x\n",
+                        apic_id, ioapic_sbdf[apic_id].bdf);
+                }
+                else
+                    continue;
+            }
+
+            if ( apic_id >= ARRAY_SIZE(ioapic_sbdf) )
             {
                 printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry 
beyond bounds\n",
-                       special->handle);
+                       apic_id);
                 return 0;
             }

-            if ( test_bit(special->handle, ioapic_cmdline) )
-                AMD_IOMMU_DEBUG("IVHD: Command line override present 
for IO-APIC %#x\n",
-                                special->handle);
-            else if ( ioapic_sbdf[special->handle].pin_2_idx )
+            if ( test_bit(apic_id, ioapic_cmdline) )
+                break;
+            else if ( ioapic_sbdf[apic_id].pin_2_idx )
             {
-                if ( ioapic_sbdf[special->handle].bdf == bdf &&
-                     ioapic_sbdf[special->handle].seg == seg )
+                if ( ioapic_sbdf[apic_id].bdf == bdf &&
+                     ioapic_sbdf[apic_id].seg == seg )
                     AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC 
%#x entries\n",
-                                    special->handle);
+                                    apic_id);
                 else
                 {
                     printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC 
%#x entries\n",
-                           special->handle);
+                           apic_id);
                     if ( amd_iommu_perdev_intremap )
                         return 0;
                 }
@@ -745,10 +761,10 @@ static u16 __init parse_ivhd_device_special(
             else
             {
                 /* set device id of ioapic */
-                ioapic_sbdf[special->handle].bdf = bdf;
-                ioapic_sbdf[special->handle].seg = seg;
+                ioapic_sbdf[apic_id].bdf = bdf;
+                ioapic_sbdf[apic_id].seg = seg;

-                ioapic_sbdf[special->handle].pin_2_idx = xmalloc_array(
+                ioapic_sbdf[apic_id].pin_2_idx = xmalloc_array(
                     u16, nr_ioapic_entries[apic]);
                 if ( nr_ioapic_entries[apic] &&
                      !ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
@@ -765,10 +781,11 @@ static u16 __init parse_ivhd_device_special(
         if ( apic == nr_ioapics )
         {
             printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n",
-                   special->handle);
+                   apic_id);
             return 0;
         }
         break;
+    }
     case ACPI_IVHD_HPET:
         /* set device id of hpet */
         if ( hpet_sbdf.iommu ||
--
1.7.10.4

Attachment: 0001-Add-additional-check-for-when-the-handle-is-mis-conf.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel