|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 4/6] x86/vtd: Drop struct ir_ctrl
It is unclear why this abstraction exists, but iommu_ir_ctrl() returns
possibly NULL and every user unconditionally dereferences the result. In
practice, I can't spot a path where iommu is NULL, so I think it is mostly
dead.
Move the fields into struct vtd_iommu, and delete iommu_ir_ctrl().
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
---
xen/drivers/passthrough/vtd/intremap.c | 90 ++++++++++++++++------------------
xen/drivers/passthrough/vtd/iommu.c | 3 +-
xen/drivers/passthrough/vtd/iommu.h | 16 ++----
xen/drivers/passthrough/vtd/utils.c | 13 +++--
4 files changed, 52 insertions(+), 70 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/intremap.c
b/xen/drivers/passthrough/vtd/intremap.c
index 30b8f90..ce32bb1 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -179,7 +179,7 @@ bool_t __init iommu_supports_eim(void)
static void update_irte(struct vtd_iommu *iommu, struct iremap_entry *entry,
const struct iremap_entry *new_ire, bool atomic)
{
- ASSERT(spin_is_locked(&iommu_ir_ctrl(iommu)->iremap_lock));
+ ASSERT(spin_is_locked(&iommu->iremap_lock));
if ( cpu_has_cx16 )
{
@@ -220,14 +220,13 @@ static void update_irte(struct vtd_iommu *iommu, struct
iremap_entry *entry,
static void free_remap_entry(struct vtd_iommu *iommu, int index)
{
struct iremap_entry *iremap_entry = NULL, *iremap_entries, new_ire = { };
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
if ( index < 0 || index > IREMAP_ENTRY_NR - 1 )
return;
- ASSERT( spin_is_locked(&ir_ctrl->iremap_lock) );
+ ASSERT(spin_is_locked(&iommu->iremap_lock));
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, index,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, index,
iremap_entries, iremap_entry);
update_irte(iommu, iremap_entry, &new_ire, false);
@@ -235,7 +234,7 @@ static void free_remap_entry(struct vtd_iommu *iommu, int
index)
iommu_flush_iec_index(iommu, 0, index);
unmap_vtd_domain_page(iremap_entries);
- ir_ctrl->iremap_num--;
+ iommu->iremap_num--;
}
/*
@@ -245,10 +244,9 @@ static void free_remap_entry(struct vtd_iommu *iommu, int
index)
static unsigned int alloc_remap_entry(struct vtd_iommu *iommu, unsigned int nr)
{
struct iremap_entry *iremap_entries = NULL;
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
unsigned int i, found;
- ASSERT( spin_is_locked(&ir_ctrl->iremap_lock) );
+ ASSERT(spin_is_locked(&iommu->iremap_lock));
for ( found = i = 0; i < IREMAP_ENTRY_NR; i++ )
{
@@ -259,7 +257,7 @@ static unsigned int alloc_remap_entry(struct vtd_iommu
*iommu, unsigned int nr)
if ( iremap_entries )
unmap_vtd_domain_page(iremap_entries);
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, i,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, i,
iremap_entries, p);
}
else
@@ -274,8 +272,9 @@ static unsigned int alloc_remap_entry(struct vtd_iommu
*iommu, unsigned int nr)
if ( iremap_entries )
unmap_vtd_domain_page(iremap_entries);
- if ( i < IREMAP_ENTRY_NR )
- ir_ctrl->iremap_num += nr;
+ if ( i < IREMAP_ENTRY_NR )
+ iommu->iremap_num += nr;
+
return i;
}
@@ -284,7 +283,6 @@ static int remap_entry_to_ioapic_rte(
{
struct iremap_entry *iremap_entry = NULL, *iremap_entries;
unsigned long flags;
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
if ( index < 0 || index > IREMAP_ENTRY_NR - 1 )
{
@@ -294,9 +292,9 @@ static int remap_entry_to_ioapic_rte(
return -EFAULT;
}
- spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
+ spin_lock_irqsave(&iommu->iremap_lock, flags);
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, index,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, index,
iremap_entries, iremap_entry);
if ( iremap_entry->val == 0 )
@@ -305,7 +303,7 @@ static int remap_entry_to_ioapic_rte(
"IO-APIC index (%d) has an empty entry\n",
index);
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return -EFAULT;
}
@@ -318,7 +316,8 @@ static int remap_entry_to_ioapic_rte(
old_rte->dest.logical.logical_dest = iremap_entry->remap.dst >> 8;
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
+
return 0;
}
@@ -332,11 +331,10 @@ static int ioapic_rte_to_remap_entry(struct vtd_iommu
*iommu,
struct IO_xAPIC_route_entry new_rte;
int index;
unsigned long flags;
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
bool init = false;
remap_rte = (struct IO_APIC_route_remap_entry *) old_rte;
- spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
+ spin_lock_irqsave(&iommu->iremap_lock, flags);
index = apic_pin_2_ir_idx[apic][ioapic_pin];
if ( index < 0 )
@@ -352,11 +350,11 @@ static int ioapic_rte_to_remap_entry(struct vtd_iommu
*iommu,
dprintk(XENLOG_ERR VTDPREFIX,
"IO-APIC intremap index (%d) larger than maximum index (%d)\n",
index, IREMAP_ENTRY_NR - 1);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return -EFAULT;
}
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, index,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, index,
iremap_entries, iremap_entry);
new_ire = *iremap_entry;
@@ -407,7 +405,7 @@ static int ioapic_rte_to_remap_entry(struct vtd_iommu
*iommu,
iommu_flush_iec_index(iommu, 0, index);
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return 0;
}
@@ -419,9 +417,8 @@ unsigned int io_apic_read_remap_rte(
struct IO_xAPIC_route_entry old_rte = { 0 };
int rte_upper = (reg & 1) ? 1 : 0;
struct vtd_iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
- if ( !ir_ctrl->iremap_num ||
+ if ( !iommu->iremap_num ||
( (index = apic_pin_2_ir_idx[apic][ioapic_pin]) < 0 ) )
return __io_apic_read(apic, reg);
@@ -539,7 +536,6 @@ static int remap_entry_to_msi_msg(
struct iremap_entry *iremap_entry = NULL, *iremap_entries;
struct msi_msg_remap_entry *remap_rte;
unsigned long flags;
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
remap_rte = (struct msi_msg_remap_entry *) msg;
index += (remap_rte->address_lo.index_15 << 15) |
@@ -553,9 +549,9 @@ static int remap_entry_to_msi_msg(
return -EFAULT;
}
- spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
+ spin_lock_irqsave(&iommu->iremap_lock, flags);
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, index,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, index,
iremap_entries, iremap_entry);
if ( iremap_entry->val == 0 )
@@ -564,7 +560,7 @@ static int remap_entry_to_msi_msg(
"MSI index (%d) has an empty entry\n",
index);
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return -EFAULT;
}
@@ -592,7 +588,7 @@ static int remap_entry_to_msi_msg(
iremap_entry->remap.vector;
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return 0;
}
@@ -604,13 +600,12 @@ static int msi_msg_to_remap_entry(
struct msi_msg_remap_entry *remap_rte;
unsigned int index, i, nr = 1;
unsigned long flags;
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
const struct pi_desc *pi_desc = msi_desc->pi_desc;
if ( msi_desc->msi_attrib.type == PCI_CAP_ID_MSI )
nr = msi_desc->msi.nvec;
- spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
+ spin_lock_irqsave(&iommu->iremap_lock, flags);
if ( msg == NULL )
{
@@ -620,7 +615,7 @@ static int msi_msg_to_remap_entry(
free_remap_entry(iommu, msi_desc->remap_index + i);
msi_desc[i].irte_initialized = false;
}
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return 0;
}
@@ -640,11 +635,12 @@ static int msi_msg_to_remap_entry(
index, IREMAP_ENTRY_NR - 1);
for ( i = 0; i < nr; ++i )
msi_desc[i].remap_index = -1;
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
+
return -EFAULT;
}
- GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, index,
+ GET_IREMAP_ENTRY(iommu->iremap_maddr, index,
iremap_entries, iremap_entry);
if ( !pi_desc )
@@ -698,7 +694,8 @@ static int msi_msg_to_remap_entry(
iommu_flush_iec_index(iommu, 0, index);
unmap_vtd_domain_page(iremap_entries);
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
+
return 0;
}
@@ -731,14 +728,13 @@ int msi_msg_write_remap_rte(
int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
{
struct vtd_iommu *iommu = hpet_to_iommu(msi_desc->hpet_id);
- struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
unsigned long flags;
int rc = 0;
- if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
+ if ( !iommu->iremap_maddr )
return 0;
- spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
+ spin_lock_irqsave(&iommu->iremap_lock, flags);
msi_desc->remap_index = alloc_remap_entry(iommu, 1);
if ( msi_desc->remap_index >= IREMAP_ENTRY_NR )
{
@@ -748,7 +744,7 @@ int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
msi_desc->remap_index = -1;
rc = -ENXIO;
}
- spin_unlock_irqrestore(&ir_ctrl->iremap_lock, flags);
+ spin_unlock_irqrestore(&iommu->iremap_lock, flags);
return rc;
}
@@ -756,7 +752,6 @@ int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
int enable_intremap(struct vtd_iommu *iommu, bool eim)
{
struct acpi_drhd_unit *drhd;
- struct ir_ctrl *ir_ctrl;
u32 sts, gcmd;
unsigned long flags;
@@ -769,11 +764,10 @@ int enable_intremap(struct vtd_iommu *iommu, bool eim)
return -EINVAL;
}
- ir_ctrl = iommu_ir_ctrl(iommu);
sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
/* Return if already enabled by Xen */
- if ( (sts & DMA_GSTS_IRES) && ir_ctrl->iremap_maddr )
+ if ( (sts & DMA_GSTS_IRES) && iommu->iremap_maddr )
return 0;
if ( !(sts & DMA_GSTS_QIES) )
@@ -789,17 +783,15 @@ int enable_intremap(struct vtd_iommu *iommu, bool eim)
" Compatibility Format Interrupts permitted on IOMMU #%u:"
" Device pass-through will be insecure\n", iommu->index);
- if ( ir_ctrl->iremap_maddr == 0 )
+ if ( iommu->iremap_maddr == 0 )
{
drhd = iommu_to_drhd(iommu);
- ir_ctrl->iremap_maddr = alloc_pgtable_maddr(drhd, IREMAP_ARCH_PAGE_NR);
- if ( ir_ctrl->iremap_maddr == 0 )
- {
- dprintk(XENLOG_WARNING VTDPREFIX,
- "Cannot allocate memory for ir_ctrl->iremap_maddr\n");
+
+ iommu->iremap_maddr = alloc_pgtable_maddr(drhd, IREMAP_ARCH_PAGE_NR);
+ if ( iommu->iremap_maddr == 0 )
return -ENOMEM;
- }
- ir_ctrl->iremap_num = 0;
+
+ iommu->iremap_num = 0;
}
spin_lock_irqsave(&iommu->register_lock, flags);
@@ -809,7 +801,7 @@ int enable_intremap(struct vtd_iommu *iommu, bool eim)
* Interrupt Mode.
*/
dmar_writeq(iommu->reg, DMAR_IRTA_REG,
- ir_ctrl->iremap_maddr | IRTA_REG_TABLE_SIZE |
+ iommu->iremap_maddr | IRTA_REG_TABLE_SIZE |
(eim ? IRTA_EIME : 0));
/* set SIRTP */
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index e5b33d2..05dc7ff 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -147,8 +147,6 @@ static struct intel_iommu *__init alloc_intel_iommu(void)
if ( intel == NULL )
return NULL;
- spin_lock_init(&intel->ir_ctrl.iremap_lock);
-
return intel;
}
@@ -1184,6 +1182,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
iommu->msi.irq = -1; /* No irq assigned yet. */
INIT_LIST_HEAD(&iommu->ats_devices);
+ spin_lock_init(&iommu->iremap_lock);
iommu->intel = alloc_intel_iommu();
if ( iommu->intel == NULL )
diff --git a/xen/drivers/passthrough/vtd/iommu.h
b/xen/drivers/passthrough/vtd/iommu.h
index 12b531c..97d0e6b 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -506,12 +506,6 @@ extern struct list_head acpi_drhd_units;
extern struct list_head acpi_rmrr_units;
extern struct list_head acpi_ioapic_units;
-struct ir_ctrl {
- u64 iremap_maddr; /* interrupt remap table machine address */
- int iremap_num; /* total num of used interrupt remap entry */
- spinlock_t iremap_lock; /* lock for irq remapping table */
-};
-
struct iommu_flush {
int __must_check (*context)(void *iommu, u16 did, u16 source_id,
u8 function_mask, u64 type,
@@ -523,7 +517,6 @@ struct iommu_flush {
};
struct intel_iommu {
- struct ir_ctrl ir_ctrl;
struct iommu_flush flush;
struct acpi_drhd_unit *drhd;
};
@@ -543,16 +536,15 @@ struct vtd_iommu {
uint64_t qinval_maddr; /* queue invalidation page machine address */
+ uint64_t iremap_maddr; /* interrupt remap table machine address */
+ unsigned int iremap_num; /* total num of used interrupt remap entry */
+ spinlock_t iremap_lock; /* lock for irq remapping table */
+
struct list_head ats_devices;
unsigned long *domid_bitmap; /* domain id bitmap */
u16 *domid_map; /* domain id mapping array */
};
-static inline struct ir_ctrl *iommu_ir_ctrl(struct vtd_iommu *iommu)
-{
- return iommu ? &iommu->intel->ir_ctrl : NULL;
-}
-
static inline struct iommu_flush *iommu_get_flush(struct vtd_iommu *iommu)
{
return iommu ? &iommu->intel->flush : NULL;
diff --git a/xen/drivers/passthrough/vtd/utils.c
b/xen/drivers/passthrough/vtd/utils.c
index 705e51b..72d2235 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -208,7 +208,7 @@ void vtd_dump_iommu_info(unsigned char key)
uint64_t iremap_maddr = irta & PAGE_MASK;
unsigned int nr_entry = 1 << ((irta & 0xF) + 1);
struct iremap_entry *iremap_entries = NULL;
- int print_cnt = 0;
+ unsigned int print_cnt = 0;
printk(" Interrupt remapping table (nr_entry=%#x. "
"Only dump P=1 entries here):\n", nr_entry);
@@ -251,9 +251,9 @@ void vtd_dump_iommu_info(unsigned char key)
}
if ( iremap_entries )
unmap_vtd_domain_page(iremap_entries);
- if ( iommu_ir_ctrl(iommu)->iremap_num != print_cnt )
- printk("Warning: Print %d IRTE (actually have %d)!\n",
- print_cnt, iommu_ir_ctrl(iommu)->iremap_num);
+ if ( iommu->iremap_num != print_cnt )
+ printk("Warning: Print %u IRTE (actually have %u)!\n",
+ print_cnt, iommu->iremap_num);
}
}
@@ -264,13 +264,12 @@ void vtd_dump_iommu_info(unsigned char key)
int apic;
union IO_APIC_reg_01 reg_01;
struct IO_APIC_route_remap_entry *remap;
- struct ir_ctrl *ir_ctrl;
for ( apic = 0; apic < nr_ioapics; apic++ )
{
iommu = ioapic_to_iommu(mp_ioapics[apic].mpc_apicid);
- ir_ctrl = iommu_ir_ctrl(iommu);
- if ( !ir_ctrl || !ir_ctrl->iremap_maddr || !ir_ctrl->iremap_num )
+
+ if ( !iommu->iremap_maddr || !iommu->iremap_num )
continue;
printk( "\nRedirection table of IOAPIC %x:\n", apic);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |