WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] VT-d: various initialization fixes

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] VT-d: various initialization fixes
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Thu, 11 Mar 2010 16:42:29 +0000
Delivery-date: Thu, 11 Mar 2010 08:43:10 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Detect invalid/unsupported configurations in iommu_alloc() - offsets
read from hardware must not lead to exceeding a single page (since
only that much gets mapped). This covers the apparently not uncommon
case of the address pointed to by a DMAR reading as all ones (Linux
for example also checks for this).

Further correct error handling of that function: Without storing the
allocated "struct iommu" instance in the drhd, iommu_free() won't do
anything, and hence all successfully set up pieces would be leaked.

Also keep iommu_free() from calling destroy_irq() when no irq was
ever set up.

Additionally, clear_fault_bits() has no need to read the capabilities
field from I/O memory - it's already cached in "struct iommu".

Finally, simplify print_iommu_regs() and its output, and actually use
this function.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

--- 2010-03-02.orig/xen/drivers/passthrough/vtd/iommu.c 2010-03-09 
13:51:48.000000000 +0100
+++ 2010-03-02/xen/drivers/passthrough/vtd/iommu.c      2010-03-09 
13:51:03.000000000 +0100
@@ -1068,9 +1068,21 @@ static int iommu_alloc(struct acpi_drhd_
     iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
     iommu->ecap = dmar_readq(iommu->reg, DMAR_ECAP_REG);
 
-    dprintk(XENLOG_INFO VTDPREFIX,
-             "drhd->address = %"PRIx64"\n", drhd->address);
-    dprintk(XENLOG_INFO VTDPREFIX, "iommu->reg = %p\n", iommu->reg);
+    drhd->iommu = iommu;
+
+    dprintk(XENLOG_DEBUG VTDPREFIX,
+            "drhd->address = %"PRIx64" iommu->reg = %p\n",
+            drhd->address, iommu->reg);
+    dprintk(XENLOG_DEBUG VTDPREFIX,
+            "cap = %"PRIx64" ecap = %"PRIx64"\n", iommu->cap, iommu->ecap);
+    if ( cap_fault_reg_offset(iommu->cap) +
+         cap_num_fault_regs(iommu->cap) * PRIMARY_FAULT_REG_LEN >= PAGE_SIZE ||
+         ecap_iotlb_offset(iommu->ecap) >= PAGE_SIZE )
+    {
+        dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: unsupported\n");
+        print_iommu_regs(drhd);
+        return -ENODEV;
+    }
 
     /* Calculate number of pagetable levels: between 2 and 4. */
     sagaw = cap_sagaw(iommu->cap);
@@ -1081,7 +1093,7 @@ static int iommu_alloc(struct acpi_drhd_
     {
         dprintk(XENLOG_ERR VTDPREFIX,
                  "IOMMU: unsupported sagaw %lx\n", sagaw);
-        xfree(iommu);
+        print_iommu_regs(drhd);
         return -ENODEV;
     }
     iommu->nr_pt_levels = agaw_to_level(agaw);
@@ -1111,7 +1123,6 @@ static int iommu_alloc(struct acpi_drhd_
     spin_lock_init(&iommu->lock);
     spin_lock_init(&iommu->register_lock);
 
-    drhd->iommu = iommu;
     return 0;
 }
 
@@ -1122,6 +1133,8 @@ static void iommu_free(struct acpi_drhd_
     if ( iommu == NULL )
         return;
 
+    drhd->iommu = NULL;
+
     if ( iommu->root_maddr != 0 )
     {
         free_pgtable_maddr(iommu->root_maddr);
@@ -1135,10 +1148,9 @@ static void iommu_free(struct acpi_drhd_
     xfree(iommu->domid_map);
 
     free_intel_iommu(iommu->intel);
-    destroy_irq(iommu->irq);
+    if ( iommu->irq >= 0 )
+        destroy_irq(iommu->irq);
     xfree(iommu);
-
-    drhd->iommu = NULL;
 }
 
 #define guestwidth_to_adjustwidth(gaw) ({       \
@@ -1765,13 +1777,8 @@ void clear_fault_bits(struct iommu *iomm
     unsigned long flags;
 
     spin_lock_irqsave(&iommu->register_lock, flags);
-    val = dmar_readq(
-        iommu->reg,
-        cap_fault_reg_offset(dmar_readq(iommu->reg,DMAR_CAP_REG))+0x8);
-    dmar_writeq(
-        iommu->reg,
-        cap_fault_reg_offset(dmar_readq(iommu->reg,DMAR_CAP_REG))+8,
-        val);
+    val = dmar_readq(iommu->reg, cap_fault_reg_offset(iommu->cap) + 8);
+    dmar_writeq(iommu->reg, cap_fault_reg_offset(iommu->cap) + 8, val);
     dmar_writel(iommu->reg, DMAR_FSTS_REG, DMA_FSTS_FAULTS);
     spin_unlock_irqrestore(&iommu->register_lock, flags);
 }
--- 2010-03-02.orig/xen/drivers/passthrough/vtd/utils.c 2010-03-09 
13:51:48.000000000 +0100
+++ 2010-03-02/xen/drivers/passthrough/vtd/utils.c      2010-03-09 
13:42:27.000000000 +0100
@@ -59,45 +59,28 @@ void disable_pmr(struct iommu *iommu)
 void print_iommu_regs(struct acpi_drhd_unit *drhd)
 {
     struct iommu *iommu = drhd->iommu;
+    u64 cap;
 
     printk("---- print_iommu_regs ----\n");
-    printk("print_iommu_regs: drhd->address = %"PRIx64"\n", drhd->address);
-    printk("print_iommu_regs: DMAR_VER_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_VER_REG));
-    printk("print_iommu_regs: DMAR_CAP_REG = %"PRIx64"\n",
-           dmar_readq(iommu->reg,DMAR_CAP_REG));
-    printk("print_iommu_regs: n_fault_reg = %"PRIx64"\n",
-           cap_num_fault_regs(dmar_readq(iommu->reg, DMAR_CAP_REG)));
-    printk("print_iommu_regs: fault_recording_offset_l = %"PRIx64"\n",
-           cap_fault_reg_offset(dmar_readq(iommu->reg, DMAR_CAP_REG)));
-    printk("print_iommu_regs: fault_recording_offset_h = %"PRIx64"\n",
-           cap_fault_reg_offset(dmar_readq(iommu->reg, DMAR_CAP_REG)) + 8);
-    printk("print_iommu_regs: fault_recording_reg_l = %"PRIx64"\n",
-           dmar_readq(iommu->reg,
-               cap_fault_reg_offset(dmar_readq(iommu->reg, DMAR_CAP_REG))));
-    printk("print_iommu_regs: fault_recording_reg_h = %"PRIx64"\n",
-           dmar_readq(iommu->reg,
-               cap_fault_reg_offset(dmar_readq(iommu->reg, DMAR_CAP_REG)) + 
8));
-    printk("print_iommu_regs: DMAR_ECAP_REG = %"PRIx64"\n",
-           dmar_readq(iommu->reg,DMAR_ECAP_REG));
-    printk("print_iommu_regs: DMAR_GCMD_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_GCMD_REG));
-    printk("print_iommu_regs: DMAR_GSTS_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_GSTS_REG));
-    printk("print_iommu_regs: DMAR_RTADDR_REG = %"PRIx64"\n",
-           dmar_readq(iommu->reg,DMAR_RTADDR_REG));
-    printk("print_iommu_regs: DMAR_CCMD_REG = %"PRIx64"\n",
-           dmar_readq(iommu->reg,DMAR_CCMD_REG));
-    printk("print_iommu_regs: DMAR_FSTS_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_FSTS_REG));
-    printk("print_iommu_regs: DMAR_FECTL_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_FECTL_REG));
-    printk("print_iommu_regs: DMAR_FEDATA_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_FEDATA_REG));
-    printk("print_iommu_regs: DMAR_FEADDR_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_FEADDR_REG));
-    printk("print_iommu_regs: DMAR_FEUADDR_REG = %x\n",
-           dmar_readl(iommu->reg,DMAR_FEUADDR_REG));
+    printk(" drhd->address = %"PRIx64"\n", drhd->address);
+    printk(" VER = %x\n", dmar_readl(iommu->reg, DMAR_VER_REG));
+    printk(" CAP = %"PRIx64"\n", cap = dmar_readq(iommu->reg, DMAR_CAP_REG));
+    printk(" n_fault_reg = %"PRIx64"\n", cap_num_fault_regs(cap));
+    printk(" fault_recording_offset = %"PRIx64"\n", cap_fault_reg_offset(cap));
+    printk(" fault_recording_reg_l = %"PRIx64"\n",
+           dmar_readq(iommu->reg, cap_fault_reg_offset(cap)));
+    printk(" fault_recording_reg_h = %"PRIx64"\n",
+           dmar_readq(iommu->reg, cap_fault_reg_offset(cap) + 8));
+    printk(" ECAP = %"PRIx64"\n", dmar_readq(iommu->reg, DMAR_ECAP_REG));
+    printk(" GCMD = %x\n", dmar_readl(iommu->reg, DMAR_GCMD_REG));
+    printk(" GSTS = %x\n", dmar_readl(iommu->reg, DMAR_GSTS_REG));
+    printk(" RTADDR = %"PRIx64"\n", dmar_readq(iommu->reg,DMAR_RTADDR_REG));
+    printk(" CCMD = %"PRIx64"\n", dmar_readq(iommu->reg, DMAR_CCMD_REG));
+    printk(" FSTS = %x\n", dmar_readl(iommu->reg, DMAR_FSTS_REG));
+    printk(" FECTL = %x\n", dmar_readl(iommu->reg, DMAR_FECTL_REG));
+    printk(" FEDATA = %x\n", dmar_readl(iommu->reg, DMAR_FEDATA_REG));
+    printk(" FEADDR = %x\n", dmar_readl(iommu->reg, DMAR_FEADDR_REG));
+    printk(" FEUADDR = %x\n", dmar_readl(iommu->reg, DMAR_FEUADDR_REG));
 }
 
 static u32 get_level_index(unsigned long gmfn, int level)



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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] VT-d: various initialization fixes, Jan Beulich <=