Mailing List Archive

[xen-unstable] VT-d: various initialization fixes
# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1268327754 0
# Node ID c9f795ff78b1740d69f2dd392d7886a3e00ba951
# Parent 099aac2cfab614c62e3bba9eb30aee7dc950d8fd
VT-d: various initialization fixes

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@novell.com>
---
xen/drivers/passthrough/vtd/iommu.c | 37 +++++++++++++----------
xen/drivers/passthrough/vtd/utils.c | 57 ++++++++++++------------------------
2 files changed, 42 insertions(+), 52 deletions(-)

diff -r 099aac2cfab6 -r c9f795ff78b1 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c Thu Mar 11 17:15:27 2010 +0000
+++ b/xen/drivers/passthrough/vtd/iommu.c Thu Mar 11 17:15:54 2010 +0000
@@ -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);
}
diff -r 099aac2cfab6 -r c9f795ff78b1 xen/drivers/passthrough/vtd/utils.c
--- a/xen/drivers/passthrough/vtd/utils.c Thu Mar 11 17:15:27 2010 +0000
+++ b/xen/drivers/passthrough/vtd/utils.c Thu Mar 11 17:15:54 2010 +0000
@@ -59,45 +59,28 @@ void print_iommu_regs(struct acpi_drhd_u
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-changelog mailing list
Xen-changelog@lists.xensource.com
http://lists.xensource.com/xen-changelog