* [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions
@ 2019-02-22 19:13 Andrew Cooper
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
` (5 more replies)
0 siblings, 6 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Kevin Tian, Wei Liu, Andrew Cooper, Paul Durrant,
Jan Beulich, Roger Pau Monné
Patch 1 of this series was XSA-283 before people pointed out that I'd got my
maths wrong. The rest of the series was the work I was doing at the time, to
try and clean up the IOMMU code.
This series comes with a net bloat-o-meter reduction of -536, a reduction in
code volume, runtime memory usage, and has no practical change in behaviour.
Andrew Cooper (6):
x86/vtd: Don't include control register state in the table pointers
x86/vtd: Rename struct iommu to vtd_iommu
x86/vtd: Drop struct qi_ctrl
x86/vtd: Drop struct ir_ctrl
x86/vtd: Drop struct iommu_flush
x86/vtd: Drop struct intel_iommu
xen/drivers/passthrough/vtd/dmar.c | 6 +-
xen/drivers/passthrough/vtd/dmar.h | 4 +-
xen/drivers/passthrough/vtd/extern.h | 36 +++----
xen/drivers/passthrough/vtd/intremap.c | 127 ++++++++++++-------------
xen/drivers/passthrough/vtd/iommu.c | 168 ++++++++++++++-------------------
xen/drivers/passthrough/vtd/iommu.h | 61 ++++--------
xen/drivers/passthrough/vtd/qinval.c | 99 +++++++++----------
xen/drivers/passthrough/vtd/quirks.c | 19 ++--
xen/drivers/passthrough/vtd/utils.c | 28 +++---
xen/drivers/passthrough/vtd/x86/ats.c | 6 +-
10 files changed, 237 insertions(+), 317 deletions(-)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 11:39 ` George Dunlap
2019-02-28 5:54 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu Andrew Cooper
` (4 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian
iremap_maddr and qinval_maddr point to the base of a block of contiguous RAM,
allocated by the driver, holding the Interrupt Remapping table, and the Queued
Invalidation ring.
Despite their name, they are actually the values of the hardware register,
including control metadata in the lower 12 bits. While uses of these fields
do appear to correctly shift out the metadata, this is very subtle behaviour
and confusing to follow.
Nothing uses the metadata, so make the fields actually point at the base of
the relevant tables.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Kevin Tian <kevin.tian@intel.com>
This was XSA-283 before people pointed out that I'd got my maths wrong, and
there wasn't actually a vulnerability with x2apic mode. Either way, the
current code is downright dangerous.
---
xen/drivers/passthrough/vtd/intremap.c | 13 +++++++------
xen/drivers/passthrough/vtd/qinval.c | 8 ++++----
xen/drivers/passthrough/vtd/utils.c | 5 +++--
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index 838268d..1d19856 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -802,14 +802,15 @@ int enable_intremap(struct iommu *iommu, int eim)
ir_ctrl->iremap_num = 0;
}
- /* set extended interrupt mode bit */
- ir_ctrl->iremap_maddr |= eim ? IRTA_EIME : 0;
-
spin_lock_irqsave(&iommu->register_lock, flags);
- /* set size of the interrupt remapping table */
- ir_ctrl->iremap_maddr |= IRTA_REG_TABLE_SIZE;
- dmar_writeq(iommu->reg, DMAR_IRTA_REG, ir_ctrl->iremap_maddr);
+ /*
+ * Set size of the interrupt remapping table and optionally Extended
+ * Interrupt Mode.
+ */
+ dmar_writeq(iommu->reg, DMAR_IRTA_REG,
+ ir_ctrl->iremap_maddr | IRTA_REG_TABLE_SIZE |
+ (eim ? IRTA_EIME : 0));
/* set SIRTP */
gcmd = dmar_readl(iommu->reg, DMAR_GSTS_REG);
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index e95dc54..01447cf 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -428,6 +428,8 @@ int enable_qinval(struct iommu *iommu)
flush->context = flush_context_qi;
flush->iotlb = flush_iotlb_qi;
+ spin_lock_irqsave(&iommu->register_lock, flags);
+
/* Setup Invalidation Queue Address(IQA) register with the
* address of the page we just allocated. QS field at
* bits[2:0] to indicate size of queue is one 4KB page.
@@ -435,10 +437,8 @@ int enable_qinval(struct iommu *iommu)
* registers are automatically reset to 0 with write
* to IQA register.
*/
- qi_ctrl->qinval_maddr |= QINVAL_PAGE_ORDER;
-
- spin_lock_irqsave(&iommu->register_lock, flags);
- dmar_writeq(iommu->reg, DMAR_IQA_REG, qi_ctrl->qinval_maddr);
+ dmar_writeq(iommu->reg, DMAR_IQA_REG,
+ qi_ctrl->qinval_maddr | QINVAL_PAGE_ORDER);
dmar_writeq(iommu->reg, DMAR_IQT_REG, 0);
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
index 85e0f41..94a6e4e 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -204,8 +204,9 @@ void vtd_dump_iommu_info(unsigned char key)
if ( status & DMA_GSTS_IRES )
{
/* Dump interrupt remapping table. */
- u64 iremap_maddr = dmar_readq(iommu->reg, DMAR_IRTA_REG);
- int nr_entry = 1 << ((iremap_maddr & 0xF) + 1);
+ uint64_t irta = dmar_readq(iommu->reg, DMAR_IRTA_REG);
+ 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;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 9:56 ` Paul Durrant
2019-02-28 5:57 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 3/6] x86/vtd: Drop struct qi_ctrl Andrew Cooper
` (3 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Kevin Tian, Paul Durrant, Jun Nakajima, Jan Beulich
VT-d's local struct iommu is an overly-generic name, for a structure which in
practice maps 1-to-1 with the real IOMMUs in the system.
Additionally, address style issues on impacted lines. This is mostly
positioning of * for pointers and unnecessay casts with void pointers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/drivers/passthrough/vtd/dmar.c | 6 +--
xen/drivers/passthrough/vtd/dmar.h | 4 +-
xen/drivers/passthrough/vtd/extern.h | 36 ++++++++---------
xen/drivers/passthrough/vtd/intremap.c | 26 ++++++------
xen/drivers/passthrough/vtd/iommu.c | 74 +++++++++++++++++-----------------
xen/drivers/passthrough/vtd/iommu.h | 8 ++--
xen/drivers/passthrough/vtd/qinval.c | 34 ++++++++--------
xen/drivers/passthrough/vtd/quirks.c | 10 ++---
xen/drivers/passthrough/vtd/utils.c | 8 ++--
xen/drivers/passthrough/vtd/x86/ats.c | 6 +--
10 files changed, 106 insertions(+), 106 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 81afa54..ce1b8ce 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -137,7 +137,7 @@ struct acpi_drhd_unit * ioapic_to_drhd(unsigned int apic_id)
return NULL;
}
-struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu)
+struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu)
{
struct acpi_drhd_unit *drhd;
@@ -151,7 +151,7 @@ struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu)
return NULL;
}
-struct iommu * ioapic_to_iommu(unsigned int apic_id)
+struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id)
{
struct acpi_drhd_unit *drhd;
@@ -182,7 +182,7 @@ struct acpi_drhd_unit *hpet_to_drhd(unsigned int hpet_id)
return NULL;
}
-struct iommu *hpet_to_iommu(unsigned int hpet_id)
+struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id)
{
struct acpi_drhd_unit *drhd = hpet_to_drhd(hpet_id);
diff --git a/xen/drivers/passthrough/vtd/dmar.h b/xen/drivers/passthrough/vtd/dmar.h
index 95bb132..1a9c965 100644
--- a/xen/drivers/passthrough/vtd/dmar.h
+++ b/xen/drivers/passthrough/vtd/dmar.h
@@ -63,7 +63,7 @@ struct acpi_drhd_unit {
u64 address; /* register base address of the unit */
u16 segment;
u8 include_all:1;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
struct list_head ioapic_list;
struct list_head hpet_list;
};
@@ -128,7 +128,7 @@ do { \
} while (0)
int vtd_hw_check(void);
-void disable_pmr(struct iommu *iommu);
+void disable_pmr(struct vtd_iommu *iommu);
int is_igd_drhd(struct acpi_drhd_unit *drhd);
#endif /* _DMAR_H_ */
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index 16eada9..cfef9a3 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -30,38 +30,38 @@ extern bool_t rwbf_quirk;
extern const struct iommu_ops intel_iommu_ops;
void print_iommu_regs(struct acpi_drhd_unit *drhd);
-void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn);
+void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64 gmfn);
keyhandler_fn_t vtd_dump_iommu_info;
-int enable_qinval(struct iommu *iommu);
-void disable_qinval(struct iommu *iommu);
-int enable_intremap(struct iommu *iommu, int eim);
-void disable_intremap(struct iommu *iommu);
+int enable_qinval(struct vtd_iommu *iommu);
+void disable_qinval(struct vtd_iommu *iommu);
+int enable_intremap(struct vtd_iommu *iommu, bool eim);
+void disable_intremap(struct vtd_iommu *iommu);
void iommu_flush_cache_entry(void *addr, unsigned int size);
void iommu_flush_cache_page(void *addr, unsigned long npages);
int iommu_alloc(struct acpi_drhd_unit *drhd);
void iommu_free(struct acpi_drhd_unit *drhd);
-int iommu_flush_iec_global(struct iommu *iommu);
-int iommu_flush_iec_index(struct iommu *iommu, u8 im, u16 iidx);
-void clear_fault_bits(struct iommu *iommu);
+int iommu_flush_iec_global(struct vtd_iommu *iommu);
+int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx);
+void clear_fault_bits(struct vtd_iommu *iommu);
-struct iommu * ioapic_to_iommu(unsigned int apic_id);
-struct iommu * hpet_to_iommu(unsigned int hpet_id);
+struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id);
+struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id);
struct acpi_drhd_unit * ioapic_to_drhd(unsigned int apic_id);
struct acpi_drhd_unit * hpet_to_drhd(unsigned int hpet_id);
-struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu);
+struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu);
struct acpi_rhsa_unit * drhd_to_rhsa(struct acpi_drhd_unit *drhd);
-struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu);
+struct acpi_drhd_unit *find_ats_dev_drhd(struct vtd_iommu *iommu);
int ats_device(const struct pci_dev *, const struct acpi_drhd_unit *);
-int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
+int dev_invalidate_iotlb(struct vtd_iommu *iommu, u16 did,
u64 addr, unsigned int size_order, u64 type);
-int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
+int __must_check qinval_device_iotlb_sync(struct vtd_iommu *iommu,
struct pci_dev *pdev,
u16 did, u16 size, u64 addr);
@@ -73,9 +73,9 @@ u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned long npages);
void free_pgtable_maddr(u64 maddr);
void *map_vtd_domain_page(u64 maddr);
void unmap_vtd_domain_page(void *va);
-int domain_context_mapping_one(struct domain *domain, struct iommu *iommu,
+int domain_context_mapping_one(struct domain *domain, struct vtd_iommu *iommu,
u8 bus, u8 devfn, const struct pci_dev *);
-int domain_context_unmap_one(struct domain *domain, struct iommu *iommu,
+int domain_context_unmap_one(struct domain *domain, struct vtd_iommu *iommu,
u8 bus, u8 devfn);
int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt);
@@ -92,8 +92,8 @@ int intel_setup_hpet_msi(struct msi_desc *);
int is_igd_vt_enabled_quirk(void);
void platform_quirks_init(void);
-void vtd_ops_preamble_quirk(struct iommu* iommu);
-void vtd_ops_postamble_quirk(struct iommu* iommu);
+void vtd_ops_preamble_quirk(struct vtd_iommu *iommu);
+void vtd_ops_postamble_quirk(struct vtd_iommu *iommu);
int __must_check me_wifi_quirk(struct domain *domain,
u8 bus, u8 devfn, int map);
void pci_vtd_quirk(const struct pci_dev *);
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index 1d19856..30b8f90 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -176,7 +176,7 @@ bool_t __init iommu_supports_eim(void)
* present an atomic update to VT-d hardware even when cmpxchg16b
* instruction is not supported.
*/
-static void update_irte(struct iommu *iommu, struct iremap_entry *entry,
+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));
@@ -217,7 +217,7 @@ static void update_irte(struct iommu *iommu, struct iremap_entry *entry,
}
/* Mark specified intr remap entry as free */
-static void free_remap_entry(struct iommu *iommu, int index)
+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);
@@ -242,7 +242,7 @@ static void free_remap_entry(struct iommu *iommu, int index)
* Look for a free intr remap entry (or a contiguous set thereof).
* Need hold iremap_lock, and setup returned entry before releasing lock.
*/
-static unsigned int alloc_remap_entry(struct iommu *iommu, unsigned int nr)
+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);
@@ -280,7 +280,7 @@ static unsigned int alloc_remap_entry(struct iommu *iommu, unsigned int nr)
}
static int remap_entry_to_ioapic_rte(
- struct iommu *iommu, int index, struct IO_xAPIC_route_entry *old_rte)
+ struct vtd_iommu *iommu, int index, struct IO_xAPIC_route_entry *old_rte)
{
struct iremap_entry *iremap_entry = NULL, *iremap_entries;
unsigned long flags;
@@ -322,7 +322,7 @@ static int remap_entry_to_ioapic_rte(
return 0;
}
-static int ioapic_rte_to_remap_entry(struct iommu *iommu,
+static int ioapic_rte_to_remap_entry(struct vtd_iommu *iommu,
int apic, unsigned int ioapic_pin, struct IO_xAPIC_route_entry *old_rte,
unsigned int rte_upper, unsigned int value)
{
@@ -418,7 +418,7 @@ unsigned int io_apic_read_remap_rte(
int index;
struct IO_xAPIC_route_entry old_rte = { 0 };
int rte_upper = (reg & 1) ? 1 : 0;
- struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
+ 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 ||
@@ -443,7 +443,7 @@ void io_apic_write_remap_rte(
struct IO_xAPIC_route_entry old_rte = { 0 };
struct IO_APIC_route_remap_entry *remap_rte;
unsigned int rte_upper = (reg & 1) ? 1 : 0;
- struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
+ struct vtd_iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
int saved_mask;
old_rte = __ioapic_read_entry(apic, ioapic_pin, 1);
@@ -534,7 +534,7 @@ static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire)
}
static int remap_entry_to_msi_msg(
- struct iommu *iommu, struct msi_msg *msg, unsigned int index)
+ struct vtd_iommu *iommu, struct msi_msg *msg, unsigned int index)
{
struct iremap_entry *iremap_entry = NULL, *iremap_entries;
struct msi_msg_remap_entry *remap_rte;
@@ -597,7 +597,7 @@ static int remap_entry_to_msi_msg(
}
static int msi_msg_to_remap_entry(
- struct iommu *iommu, struct pci_dev *pdev,
+ struct vtd_iommu *iommu, struct pci_dev *pdev,
struct msi_desc *msi_desc, struct msi_msg *msg)
{
struct iremap_entry *iremap_entry = NULL, *iremap_entries, new_ire = { };
@@ -730,7 +730,7 @@ int msi_msg_write_remap_rte(
int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
{
- struct iommu *iommu = hpet_to_iommu(msi_desc->hpet_id);
+ 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;
@@ -753,7 +753,7 @@ int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
return rc;
}
-int enable_intremap(struct iommu *iommu, int eim)
+int enable_intremap(struct vtd_iommu *iommu, bool eim)
{
struct acpi_drhd_unit *drhd;
struct ir_ctrl *ir_ctrl;
@@ -836,7 +836,7 @@ int enable_intremap(struct iommu *iommu, int eim)
return init_apic_pin_2_ir_idx();
}
-void disable_intremap(struct iommu *iommu)
+void disable_intremap(struct vtd_iommu *iommu)
{
u32 sts;
u64 irta;
@@ -885,7 +885,7 @@ void disable_intremap(struct iommu *iommu)
int iommu_enable_x2apic_IR(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
if ( system_state < SYS_STATE_active )
{
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 50a0e25..e5b33d2 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -58,7 +58,7 @@ static int setup_hwdom_device(u8 devfn, struct pci_dev *);
static void setup_hwdom_rmrr(struct domain *d);
static int domain_iommu_domid(struct domain *d,
- struct iommu *iommu)
+ struct vtd_iommu *iommu)
{
unsigned long nr_dom, i;
@@ -82,7 +82,7 @@ static int domain_iommu_domid(struct domain *d,
#define DID_HIGH_OFFSET 8
static int context_set_domain_id(struct context_entry *context,
struct domain *d,
- struct iommu *iommu)
+ struct vtd_iommu *iommu)
{
unsigned long nr_dom, i;
int found = 0;
@@ -118,7 +118,7 @@ static int context_set_domain_id(struct context_entry *context,
}
static int context_get_domain_id(struct context_entry *context,
- struct iommu *iommu)
+ struct vtd_iommu *iommu)
{
unsigned long dom_index, nr_dom;
int domid = -1;
@@ -222,7 +222,7 @@ void free_pgtable_maddr(u64 maddr)
}
/* context entry handling */
-static u64 bus_to_context_maddr(struct iommu *iommu, u8 bus)
+static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
{
struct acpi_drhd_unit *drhd;
struct root_entry *root, *root_entries;
@@ -316,7 +316,7 @@ static u64 addr_to_dma_page_maddr(struct domain *domain, u64 addr, int alloc)
return pte_maddr;
}
-static void iommu_flush_write_buffer(struct iommu *iommu)
+static void iommu_flush_write_buffer(struct vtd_iommu *iommu)
{
u32 val;
unsigned long flags;
@@ -340,7 +340,7 @@ static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
u8 function_mask, u64 type,
bool_t flush_non_present_entry)
{
- struct iommu *iommu = (struct iommu *) _iommu;
+ struct vtd_iommu *iommu = _iommu;
u64 val = 0;
unsigned long flags;
@@ -388,7 +388,7 @@ static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
return 0;
}
-static int __must_check iommu_flush_context_global(struct iommu *iommu,
+static int __must_check iommu_flush_context_global(struct vtd_iommu *iommu,
bool_t flush_non_present_entry)
{
struct iommu_flush *flush = iommu_get_flush(iommu);
@@ -396,7 +396,7 @@ static int __must_check iommu_flush_context_global(struct iommu *iommu,
flush_non_present_entry);
}
-static int __must_check iommu_flush_context_device(struct iommu *iommu,
+static int __must_check iommu_flush_context_device(struct vtd_iommu *iommu,
u16 did, u16 source_id,
u8 function_mask,
bool_t flush_non_present_entry)
@@ -413,7 +413,7 @@ static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
{
- struct iommu *iommu = (struct iommu *) _iommu;
+ struct vtd_iommu *iommu = _iommu;
int tlb_offset = ecap_iotlb_offset(iommu->ecap);
u64 val = 0;
unsigned long flags;
@@ -475,7 +475,7 @@ static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
return 0;
}
-static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
+static int __must_check iommu_flush_iotlb_global(struct vtd_iommu *iommu,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
{
@@ -494,7 +494,7 @@ static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
return status;
}
-static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
+static int __must_check iommu_flush_iotlb_dsi(struct vtd_iommu *iommu, u16 did,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
{
@@ -513,7 +513,7 @@ static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
return status;
}
-static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
+static int __must_check iommu_flush_iotlb_psi(struct vtd_iommu *iommu, u16 did,
u64 addr, unsigned int order,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
@@ -549,7 +549,7 @@ static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
static int __must_check iommu_flush_all(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
bool_t flush_dev_iotlb;
int rc = 0;
@@ -590,7 +590,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn,
{
struct domain_iommu *hd = dom_iommu(d);
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
bool_t flush_dev_iotlb;
int iommu_domid;
int rc = 0;
@@ -726,7 +726,7 @@ static void iommu_free_page_table(struct page_info *pg)
free_pgtable_maddr(pt_maddr);
}
-static int iommu_set_root_entry(struct iommu *iommu)
+static int iommu_set_root_entry(struct vtd_iommu *iommu)
{
u32 sts;
unsigned long flags;
@@ -749,7 +749,7 @@ static void iommu_enable_translation(struct acpi_drhd_unit *drhd)
{
u32 sts;
unsigned long flags;
- struct iommu *iommu = drhd->iommu;
+ struct vtd_iommu *iommu = drhd->iommu;
if ( is_igd_drhd(drhd) )
{
@@ -793,7 +793,7 @@ static void iommu_enable_translation(struct acpi_drhd_unit *drhd)
disable_pmr(iommu);
}
-static void iommu_disable_translation(struct iommu *iommu)
+static void iommu_disable_translation(struct vtd_iommu *iommu)
{
u32 sts;
unsigned long flags;
@@ -870,7 +870,7 @@ static const char *iommu_get_fault_reason(u8 fault_reason,
}
}
-static int iommu_page_fault_do_one(struct iommu *iommu, int type,
+static int iommu_page_fault_do_one(struct vtd_iommu *iommu, int type,
u8 fault_reason, u16 source_id, u64 addr)
{
const char *reason, *kind;
@@ -936,7 +936,7 @@ static void iommu_fault_status(u32 fault_status)
}
#define PRIMARY_FAULT_REG_LEN (16)
-static void __do_iommu_page_fault(struct iommu *iommu)
+static void __do_iommu_page_fault(struct vtd_iommu *iommu)
{
int reg, fault_index;
u32 fault_status;
@@ -1039,7 +1039,7 @@ static void iommu_page_fault(int irq, void *dev_id,
static void dma_msi_unmask(struct irq_desc *desc)
{
- struct iommu *iommu = desc->action->dev_id;
+ struct vtd_iommu *iommu = desc->action->dev_id;
unsigned long flags;
u32 sts;
@@ -1055,7 +1055,7 @@ static void dma_msi_unmask(struct irq_desc *desc)
static void dma_msi_mask(struct irq_desc *desc)
{
unsigned long flags;
- struct iommu *iommu = desc->action->dev_id;
+ struct vtd_iommu *iommu = desc->action->dev_id;
u32 sts;
/* mask it */
@@ -1091,7 +1091,7 @@ static void dma_msi_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
struct msi_msg msg;
unsigned int dest;
unsigned long flags;
- struct iommu *iommu = desc->action->dev_id;
+ struct vtd_iommu *iommu = desc->action->dev_id;
dest = set_desc_affinity(desc, mask);
if (dest == BAD_APICID){
@@ -1134,7 +1134,7 @@ static int __init iommu_set_interrupt(struct acpi_drhd_unit *drhd)
{
int irq, ret;
struct acpi_rhsa_unit *rhsa = drhd_to_rhsa(drhd);
- struct iommu *iommu = drhd->iommu;
+ struct vtd_iommu *iommu = drhd->iommu;
struct irq_desc *desc;
irq = create_irq(rhsa ? pxm_to_node(rhsa->proximity_domain)
@@ -1167,7 +1167,7 @@ static int __init iommu_set_interrupt(struct acpi_drhd_unit *drhd)
int __init iommu_alloc(struct acpi_drhd_unit *drhd)
{
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
unsigned long sagaw, nr_dom;
int agaw;
@@ -1178,7 +1178,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
return -ENOMEM;
}
- iommu = xzalloc(struct iommu);
+ iommu = xzalloc(struct vtd_iommu);
if ( iommu == NULL )
return -ENOMEM;
@@ -1265,7 +1265,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
void __init iommu_free(struct acpi_drhd_unit *drhd)
{
- struct iommu *iommu = drhd->iommu;
+ struct vtd_iommu *iommu = drhd->iommu;
if ( iommu == NULL )
return;
@@ -1328,7 +1328,7 @@ static void __hwdom_init intel_iommu_hwdom_init(struct domain *d)
int domain_context_mapping_one(
struct domain *domain,
- struct iommu *iommu,
+ struct vtd_iommu *iommu,
u8 bus, u8 devfn, const struct pci_dev *pdev)
{
struct domain_iommu *hd = dom_iommu(domain);
@@ -1565,7 +1565,7 @@ static int domain_context_mapping(struct domain *domain, u8 devfn,
int domain_context_unmap_one(
struct domain *domain,
- struct iommu *iommu,
+ struct vtd_iommu *iommu,
u8 bus, u8 devfn)
{
struct context_entry *context, *context_entries;
@@ -1633,7 +1633,7 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
struct pci_dev *pdev)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
int ret = 0;
u8 seg = pdev->seg, bus = pdev->bus, tmp_bus, tmp_devfn, secbus;
int found = 0;
@@ -1893,7 +1893,7 @@ int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
int order, int present)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu = NULL;
+ struct vtd_iommu *iommu = NULL;
struct domain_iommu *hd = dom_iommu(d);
bool_t flush_dev_iotlb;
int iommu_domid;
@@ -1936,7 +1936,7 @@ int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
return rc;
}
-static int __init vtd_ept_page_compatible(struct iommu *iommu)
+static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
{
u64 ept_cap, vtd_cap = iommu->cap;
@@ -2116,7 +2116,7 @@ static int __hwdom_init setup_hwdom_device(u8 devfn, struct pci_dev *pdev)
return domain_context_mapping(pdev->domain, devfn, pdev);
}
-void clear_fault_bits(struct iommu *iommu)
+void clear_fault_bits(struct vtd_iommu *iommu)
{
u64 val;
unsigned long flags;
@@ -2158,7 +2158,7 @@ __initcall(adjust_vtd_irq_affinities);
static int __must_check init_vtd_hw(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
struct iommu_flush *flush = NULL;
int ret;
unsigned long flags;
@@ -2283,7 +2283,7 @@ static void __hwdom_init setup_hwdom_rmrr(struct domain *d)
int __init intel_vtd_setup(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
int ret;
if ( list_empty(&acpi_drhd_units) )
@@ -2551,7 +2551,7 @@ static u32 iommu_state[MAX_IOMMUS][MAX_IOMMU_REGS];
static int __must_check vtd_suspend(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
u32 i;
int rc;
@@ -2601,7 +2601,7 @@ static int __must_check vtd_suspend(void)
static void vtd_crash_shutdown(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
if ( !iommu_enabled )
return;
@@ -2622,7 +2622,7 @@ static void vtd_crash_shutdown(void)
static void vtd_resume(void)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
u32 i;
unsigned long flags;
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 1a992f7..556b3d6 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -533,7 +533,7 @@ struct intel_iommu {
struct acpi_drhd_unit *drhd;
};
-struct iommu {
+struct vtd_iommu {
struct list_head list;
void __iomem *reg; /* Pointer to hardware regs, virtual addr */
u32 index; /* Sequence number of iommu */
@@ -550,17 +550,17 @@ struct iommu {
u16 *domid_map; /* domain id mapping array */
};
-static inline struct qi_ctrl *iommu_qi_ctrl(struct iommu *iommu)
+static inline struct qi_ctrl *iommu_qi_ctrl(struct vtd_iommu *iommu)
{
return iommu ? &iommu->intel->qi_ctrl : NULL;
}
-static inline struct ir_ctrl *iommu_ir_ctrl(struct iommu *iommu)
+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 iommu *iommu)
+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/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index 01447cf..3ddb9b6 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -31,9 +31,9 @@
#define VTD_QI_TIMEOUT 1
-static int __must_check invalidate_sync(struct iommu *iommu);
+static int __must_check invalidate_sync(struct vtd_iommu *iommu);
-static void print_qi_regs(struct iommu *iommu)
+static void print_qi_regs(struct vtd_iommu *iommu)
{
u64 val;
@@ -47,7 +47,7 @@ static void print_qi_regs(struct iommu *iommu)
printk("DMAR_IQT_REG = %"PRIx64"\n", val);
}
-static unsigned int qinval_next_index(struct iommu *iommu)
+static unsigned int qinval_next_index(struct vtd_iommu *iommu)
{
u64 tail;
@@ -62,7 +62,7 @@ static unsigned int qinval_next_index(struct iommu *iommu)
return tail;
}
-static void qinval_update_qtail(struct iommu *iommu, unsigned int index)
+static void qinval_update_qtail(struct vtd_iommu *iommu, unsigned int index)
{
u64 val;
@@ -72,7 +72,7 @@ static void qinval_update_qtail(struct iommu *iommu, unsigned int index)
dmar_writeq(iommu->reg, DMAR_IQT_REG, (val << QINVAL_INDEX_SHIFT));
}
-static int __must_check queue_invalidate_context_sync(struct iommu *iommu,
+static int __must_check queue_invalidate_context_sync(struct vtd_iommu *iommu,
u16 did, u16 source_id,
u8 function_mask,
u8 granu)
@@ -106,7 +106,7 @@ static int __must_check queue_invalidate_context_sync(struct iommu *iommu,
return invalidate_sync(iommu);
}
-static int __must_check queue_invalidate_iotlb_sync(struct iommu *iommu,
+static int __must_check queue_invalidate_iotlb_sync(struct vtd_iommu *iommu,
u8 granu, u8 dr, u8 dw,
u16 did, u8 am, u8 ih,
u64 addr)
@@ -143,7 +143,7 @@ static int __must_check queue_invalidate_iotlb_sync(struct iommu *iommu,
return invalidate_sync(iommu);
}
-static int __must_check queue_invalidate_wait(struct iommu *iommu,
+static int __must_check queue_invalidate_wait(struct vtd_iommu *iommu,
u8 iflag, u8 sw, u8 fn,
bool_t flush_dev_iotlb)
{
@@ -199,7 +199,7 @@ static int __must_check queue_invalidate_wait(struct iommu *iommu,
return -EOPNOTSUPP;
}
-static int __must_check invalidate_sync(struct iommu *iommu)
+static int __must_check invalidate_sync(struct vtd_iommu *iommu)
{
struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
@@ -208,7 +208,7 @@ static int __must_check invalidate_sync(struct iommu *iommu)
return queue_invalidate_wait(iommu, 0, 1, 1, 0);
}
-static int __must_check dev_invalidate_sync(struct iommu *iommu,
+static int __must_check dev_invalidate_sync(struct vtd_iommu *iommu,
struct pci_dev *pdev, u16 did)
{
struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
@@ -237,7 +237,7 @@ static int __must_check dev_invalidate_sync(struct iommu *iommu,
return rc;
}
-int qinval_device_iotlb_sync(struct iommu *iommu, struct pci_dev *pdev,
+int qinval_device_iotlb_sync(struct vtd_iommu *iommu, struct pci_dev *pdev,
u16 did, u16 size, u64 addr)
{
unsigned long flags;
@@ -271,7 +271,7 @@ int qinval_device_iotlb_sync(struct iommu *iommu, struct pci_dev *pdev,
return dev_invalidate_sync(iommu, pdev, did);
}
-static int __must_check queue_invalidate_iec_sync(struct iommu *iommu,
+static int __must_check queue_invalidate_iec_sync(struct vtd_iommu *iommu,
u8 granu, u8 im, u16 iidx)
{
unsigned long flags;
@@ -310,12 +310,12 @@ static int __must_check queue_invalidate_iec_sync(struct iommu *iommu,
return ret;
}
-int iommu_flush_iec_global(struct iommu *iommu)
+int iommu_flush_iec_global(struct vtd_iommu *iommu)
{
return queue_invalidate_iec_sync(iommu, IEC_GLOBAL_INVL, 0, 0);
}
-int iommu_flush_iec_index(struct iommu *iommu, u8 im, u16 iidx)
+int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx)
{
return queue_invalidate_iec_sync(iommu, IEC_INDEX_INVL, im, iidx);
}
@@ -324,7 +324,7 @@ static int __must_check flush_context_qi(void *_iommu, u16 did,
u16 sid, u8 fm, u64 type,
bool_t flush_non_present_entry)
{
- struct iommu *iommu = (struct iommu *)_iommu;
+ struct vtd_iommu *iommu = _iommu;
struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
ASSERT(qi_ctrl->qinval_maddr);
@@ -354,7 +354,7 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
{
u8 dr = 0, dw = 0;
int ret = 0, rc;
- struct iommu *iommu = (struct iommu *)_iommu;
+ struct vtd_iommu *iommu = _iommu;
struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
ASSERT(qi_ctrl->qinval_maddr);
@@ -394,7 +394,7 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
return ret;
}
-int enable_qinval(struct iommu *iommu)
+int enable_qinval(struct vtd_iommu *iommu)
{
struct acpi_drhd_unit *drhd;
struct qi_ctrl *qi_ctrl;
@@ -454,7 +454,7 @@ int enable_qinval(struct iommu *iommu)
return 0;
}
-void disable_qinval(struct iommu *iommu)
+void disable_qinval(struct vtd_iommu *iommu)
{
u32 sts;
unsigned long flags;
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index d6db862..79209f3 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -137,7 +137,7 @@ static void __init map_igd_reg(void)
/*
* force IGD to exit low power mode by accessing a IGD 3D regsiter.
*/
-static int cantiga_vtd_ops_preamble(struct iommu* iommu)
+static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
{
struct intel_iommu *intel = iommu->intel;
struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
@@ -172,7 +172,7 @@ static int cantiga_vtd_ops_preamble(struct iommu* iommu)
* parameter to a numerical value enables the quirk and
* sets the timeout to that numerical number of msecs.
*/
-static void snb_vtd_ops_preamble(struct iommu* iommu)
+static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
{
struct intel_iommu *intel = iommu->intel;
struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
@@ -202,7 +202,7 @@ static void snb_vtd_ops_preamble(struct iommu* iommu)
*(volatile u32 *)(igd_reg_va + 0x2050) = 0x10001;
}
-static void snb_vtd_ops_postamble(struct iommu* iommu)
+static void snb_vtd_ops_postamble(struct vtd_iommu *iommu)
{
struct intel_iommu *intel = iommu->intel;
struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
@@ -221,7 +221,7 @@ static void snb_vtd_ops_postamble(struct iommu* iommu)
* call before VT-d translation enable and IOTLB flush operations.
*/
-void vtd_ops_preamble_quirk(struct iommu* iommu)
+void vtd_ops_preamble_quirk(struct vtd_iommu *iommu)
{
cantiga_vtd_ops_preamble(iommu);
if ( snb_igd_timeout != 0 )
@@ -236,7 +236,7 @@ void vtd_ops_preamble_quirk(struct iommu* iommu)
/*
* call after VT-d translation enable and IOTLB flush operations.
*/
-void vtd_ops_postamble_quirk(struct iommu* iommu)
+void vtd_ops_postamble_quirk(struct vtd_iommu *iommu)
{
if ( snb_igd_timeout != 0 )
{
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
index 94a6e4e..705e51b 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -29,7 +29,7 @@
#include <asm/io_apic.h>
/* Disable vt-d protected memory registers. */
-void disable_pmr(struct iommu *iommu)
+void disable_pmr(struct vtd_iommu *iommu)
{
u32 val;
unsigned long flags;
@@ -51,7 +51,7 @@ void disable_pmr(struct iommu *iommu)
void print_iommu_regs(struct acpi_drhd_unit *drhd)
{
- struct iommu *iommu = drhd->iommu;
+ struct vtd_iommu *iommu = drhd->iommu;
u64 cap;
printk("---- print_iommu_regs ----\n");
@@ -87,7 +87,7 @@ static u32 get_level_index(unsigned long gmfn, int level)
return gmfn & LEVEL_MASK;
}
-void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn)
+void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64 gmfn)
{
struct context_entry *ctxt_entry;
struct root_entry *root_entry;
@@ -175,7 +175,7 @@ void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn)
void vtd_dump_iommu_info(unsigned char key)
{
struct acpi_drhd_unit *drhd;
- struct iommu *iommu;
+ struct vtd_iommu *iommu;
int i;
for_each_drhd_unit ( drhd )
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c
index 1a3adb4..1a07607 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/x86/ats.c
@@ -30,7 +30,7 @@
static LIST_HEAD(ats_dev_drhd_units);
-struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu)
+struct acpi_drhd_unit *find_ats_dev_drhd(struct vtd_iommu *iommu)
{
struct acpi_drhd_unit *drhd;
list_for_each_entry ( drhd, &ats_dev_drhd_units, list )
@@ -71,7 +71,7 @@ int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd)
return pos;
}
-static int device_in_domain(const struct iommu *iommu,
+static int device_in_domain(const struct vtd_iommu *iommu,
const struct pci_dev *pdev, u16 did)
{
struct root_entry *root_entry = NULL;
@@ -106,7 +106,7 @@ static int device_in_domain(const struct iommu *iommu,
return found;
}
-int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
+int dev_invalidate_iotlb(struct vtd_iommu *iommu, u16 did,
u64 addr, unsigned int size_order, u64 type)
{
struct pci_dev *pdev, *temp;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
2019-02-22 19:13 ` [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 10:02 ` Paul Durrant
2019-02-28 6:02 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 4/6] x86/vtd: Drop struct ir_ctrl Andrew Cooper
` (2 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Paul Durrant, Jan Beulich
It is unclear why this abstraction exists, but iommu_qi_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 sole member into struct vtd_iommu, and delete iommu_qi_ctrl().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/drivers/passthrough/vtd/iommu.h | 13 +++--------
xen/drivers/passthrough/vtd/qinval.c | 42 +++++++++++++++---------------------
2 files changed, 20 insertions(+), 35 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 556b3d6..12b531c 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -506,10 +506,6 @@ extern struct list_head acpi_drhd_units;
extern struct list_head acpi_rmrr_units;
extern struct list_head acpi_ioapic_units;
-struct qi_ctrl {
- u64 qinval_maddr; /* queue invalidation page machine address */
-};
-
struct ir_ctrl {
u64 iremap_maddr; /* interrupt remap table machine address */
int iremap_num; /* total num of used interrupt remap entry */
@@ -527,7 +523,6 @@ struct iommu_flush {
};
struct intel_iommu {
- struct qi_ctrl qi_ctrl;
struct ir_ctrl ir_ctrl;
struct iommu_flush flush;
struct acpi_drhd_unit *drhd;
@@ -545,16 +540,14 @@ struct vtd_iommu {
u64 root_maddr; /* root entry machine address */
struct msi_desc msi;
struct intel_iommu *intel;
+
+ uint64_t qinval_maddr; /* queue invalidation page machine address */
+
struct list_head ats_devices;
unsigned long *domid_bitmap; /* domain id bitmap */
u16 *domid_map; /* domain id mapping array */
};
-static inline struct qi_ctrl *iommu_qi_ctrl(struct vtd_iommu *iommu)
-{
- return iommu ? &iommu->intel->qi_ctrl : NULL;
-}
-
static inline struct ir_ctrl *iommu_ir_ctrl(struct vtd_iommu *iommu)
{
return iommu ? &iommu->intel->ir_ctrl : NULL;
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index 3ddb9b6..f6fcee5 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -84,7 +84,7 @@ static int __must_check queue_invalidate_context_sync(struct vtd_iommu *iommu,
spin_lock_irqsave(&iommu->register_lock, flags);
index = qinval_next_index(iommu);
- entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
+ entry_base = iommu->qinval_maddr +
((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
qinval_entries = map_vtd_domain_page(entry_base);
qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
@@ -118,7 +118,7 @@ static int __must_check queue_invalidate_iotlb_sync(struct vtd_iommu *iommu,
spin_lock_irqsave(&iommu->register_lock, flags);
index = qinval_next_index(iommu);
- entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
+ entry_base = iommu->qinval_maddr +
((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
qinval_entries = map_vtd_domain_page(entry_base);
qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
@@ -155,7 +155,7 @@ static int __must_check queue_invalidate_wait(struct vtd_iommu *iommu,
spin_lock_irqsave(&iommu->register_lock, flags);
index = qinval_next_index(iommu);
- entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
+ entry_base = iommu->qinval_maddr +
((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
qinval_entries = map_vtd_domain_page(entry_base);
qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
@@ -201,9 +201,7 @@ static int __must_check queue_invalidate_wait(struct vtd_iommu *iommu,
static int __must_check invalidate_sync(struct vtd_iommu *iommu)
{
- struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
-
- ASSERT(qi_ctrl->qinval_maddr);
+ ASSERT(iommu->qinval_maddr);
return queue_invalidate_wait(iommu, 0, 1, 1, 0);
}
@@ -211,10 +209,9 @@ static int __must_check invalidate_sync(struct vtd_iommu *iommu)
static int __must_check dev_invalidate_sync(struct vtd_iommu *iommu,
struct pci_dev *pdev, u16 did)
{
- struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
int rc;
- ASSERT(qi_ctrl->qinval_maddr);
+ ASSERT(iommu->qinval_maddr);
rc = queue_invalidate_wait(iommu, 0, 1, 1, 1);
if ( rc == -ETIMEDOUT )
{
@@ -248,7 +245,7 @@ int qinval_device_iotlb_sync(struct vtd_iommu *iommu, struct pci_dev *pdev,
ASSERT(pdev);
spin_lock_irqsave(&iommu->register_lock, flags);
index = qinval_next_index(iommu);
- entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
+ entry_base = iommu->qinval_maddr +
((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
qinval_entries = map_vtd_domain_page(entry_base);
qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
@@ -282,7 +279,7 @@ static int __must_check queue_invalidate_iec_sync(struct vtd_iommu *iommu,
spin_lock_irqsave(&iommu->register_lock, flags);
index = qinval_next_index(iommu);
- entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
+ entry_base = iommu->qinval_maddr +
((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
qinval_entries = map_vtd_domain_page(entry_base);
qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
@@ -325,9 +322,8 @@ static int __must_check flush_context_qi(void *_iommu, u16 did,
bool_t flush_non_present_entry)
{
struct vtd_iommu *iommu = _iommu;
- struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
- ASSERT(qi_ctrl->qinval_maddr);
+ ASSERT(iommu->qinval_maddr);
/*
* In the non-present entry flush case, if hardware doesn't cache
@@ -355,9 +351,8 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
u8 dr = 0, dw = 0;
int ret = 0, rc;
struct vtd_iommu *iommu = _iommu;
- struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
- ASSERT(qi_ctrl->qinval_maddr);
+ ASSERT(iommu->qinval_maddr);
/*
* In the non-present entry flush case, if hardware doesn't cache
@@ -397,7 +392,6 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
int enable_qinval(struct vtd_iommu *iommu)
{
struct acpi_drhd_unit *drhd;
- struct qi_ctrl *qi_ctrl;
struct iommu_flush *flush;
u32 sts;
unsigned long flags;
@@ -405,24 +399,22 @@ int enable_qinval(struct vtd_iommu *iommu)
if ( !ecap_queued_inval(iommu->ecap) || !iommu_qinval )
return -ENOENT;
- qi_ctrl = iommu_qi_ctrl(iommu);
flush = iommu_get_flush(iommu);
/* Return if already enabled by Xen */
sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
- if ( (sts & DMA_GSTS_QIES) && qi_ctrl->qinval_maddr )
+ if ( (sts & DMA_GSTS_QIES) && iommu->qinval_maddr )
return 0;
- if ( qi_ctrl->qinval_maddr == 0 )
+ if ( iommu->qinval_maddr == 0 )
{
drhd = iommu_to_drhd(iommu);
- qi_ctrl->qinval_maddr = alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR);
- if ( qi_ctrl->qinval_maddr == 0 )
- {
- dprintk(XENLOG_WARNING VTDPREFIX,
- "Cannot allocate memory for qi_ctrl->qinval_maddr\n");
+
+ iommu->qinval_maddr =
+ alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR);
+
+ if ( iommu->qinval_maddr == 0 )
return -ENOMEM;
- }
}
flush->context = flush_context_qi;
@@ -438,7 +430,7 @@ int enable_qinval(struct vtd_iommu *iommu)
* to IQA register.
*/
dmar_writeq(iommu->reg, DMAR_IQA_REG,
- qi_ctrl->qinval_maddr | QINVAL_PAGE_ORDER);
+ iommu->qinval_maddr | QINVAL_PAGE_ORDER);
dmar_writeq(iommu->reg, DMAR_IQT_REG, 0);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/6] x86/vtd: Drop struct ir_ctrl
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
` (2 preceding siblings ...)
2019-02-22 19:13 ` [PATCH 3/6] x86/vtd: Drop struct qi_ctrl Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 10:09 ` Paul Durrant
2019-02-22 19:13 ` [PATCH 5/6] x86/vtd: Drop struct iommu_flush Andrew Cooper
2019-02-22 19:13 ` [PATCH 6/6] x86/vtd: Drop struct intel_iommu Andrew Cooper
5 siblings, 1 reply; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Paul Durrant, Jan Beulich
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@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/6] x86/vtd: Drop struct iommu_flush
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
` (3 preceding siblings ...)
2019-02-22 19:13 ` [PATCH 4/6] x86/vtd: Drop struct ir_ctrl Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 10:17 ` Paul Durrant
2019-02-28 6:09 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 6/6] x86/vtd: Drop struct intel_iommu Andrew Cooper
5 siblings, 2 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Paul Durrant, Jan Beulich
It is unclear why this abstraction exists, but iommu_get_flush() 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 two function pointers into struct vtd_iommu (using a flush_ prefix),
and delete iommu_get_flush(). Furthermore, there is no need to pass the IOMMU
pointer to the callbacks via a void pointer, so change the parameter to be
correctly typed as struct vtd_iommu. Clean up bool_t to bool in surrounding
context.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/drivers/passthrough/vtd/iommu.c | 62 ++++++++++++++++--------------------
xen/drivers/passthrough/vtd/iommu.h | 24 +++++---------
xen/drivers/passthrough/vtd/qinval.c | 21 +++++-------
3 files changed, 44 insertions(+), 63 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 05dc7ff..7fc6fe0 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -334,11 +334,11 @@ static void iommu_flush_write_buffer(struct vtd_iommu *iommu)
}
/* return value determine if we need a write buffer flush */
-static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
- u8 function_mask, u64 type,
- bool_t flush_non_present_entry)
+static int __must_check flush_context_reg(struct vtd_iommu *iommu, u16 did,
+ u16 source_id, u8 function_mask,
+ u64 type,
+ bool flush_non_present_entry)
{
- struct vtd_iommu *iommu = _iommu;
u64 val = 0;
unsigned long flags;
@@ -387,31 +387,28 @@ static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
}
static int __must_check iommu_flush_context_global(struct vtd_iommu *iommu,
- bool_t flush_non_present_entry)
+ bool flush_non_present_entry)
{
- struct iommu_flush *flush = iommu_get_flush(iommu);
- return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
- flush_non_present_entry);
+ return iommu->flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
+ flush_non_present_entry);
}
static int __must_check iommu_flush_context_device(struct vtd_iommu *iommu,
u16 did, u16 source_id,
u8 function_mask,
- bool_t flush_non_present_entry)
+ bool flush_non_present_entry)
{
- struct iommu_flush *flush = iommu_get_flush(iommu);
- return flush->context(iommu, did, source_id, function_mask,
- DMA_CCMD_DEVICE_INVL,
- flush_non_present_entry);
+ return iommu->flush_context(iommu, did, source_id, function_mask,
+ DMA_CCMD_DEVICE_INVL, flush_non_present_entry);
}
/* return value determine if we need a write buffer flush */
-static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
+static int __must_check flush_iotlb_reg(struct vtd_iommu *iommu, u16 did,
+ u64 addr,
unsigned int size_order, u64 type,
- bool_t flush_non_present_entry,
- bool_t flush_dev_iotlb)
+ bool flush_non_present_entry,
+ bool flush_dev_iotlb)
{
- struct vtd_iommu *iommu = _iommu;
int tlb_offset = ecap_iotlb_offset(iommu->ecap);
u64 val = 0;
unsigned long flags;
@@ -474,17 +471,16 @@ static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
}
static int __must_check iommu_flush_iotlb_global(struct vtd_iommu *iommu,
- bool_t flush_non_present_entry,
- bool_t flush_dev_iotlb)
+ bool flush_non_present_entry,
+ bool flush_dev_iotlb)
{
- struct iommu_flush *flush = iommu_get_flush(iommu);
int status;
/* apply platform specific errata workarounds */
vtd_ops_preamble_quirk(iommu);
- status = flush->iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH,
- flush_non_present_entry, flush_dev_iotlb);
+ status = iommu->flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH,
+ flush_non_present_entry, flush_dev_iotlb);
/* undo platform specific errata workarounds */
vtd_ops_postamble_quirk(iommu);
@@ -496,14 +492,13 @@ static int __must_check iommu_flush_iotlb_dsi(struct vtd_iommu *iommu, u16 did,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
{
- struct iommu_flush *flush = iommu_get_flush(iommu);
int status;
/* apply platform specific errata workarounds */
vtd_ops_preamble_quirk(iommu);
- status = flush->iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH,
- flush_non_present_entry, flush_dev_iotlb);
+ status = iommu->flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH,
+ flush_non_present_entry, flush_dev_iotlb);
/* undo platform specific errata workarounds */
vtd_ops_postamble_quirk(iommu);
@@ -516,18 +511,19 @@ static int __must_check iommu_flush_iotlb_psi(struct vtd_iommu *iommu, u16 did,
bool_t flush_non_present_entry,
bool_t flush_dev_iotlb)
{
- struct iommu_flush *flush = iommu_get_flush(iommu);
int status;
ASSERT(!(addr & (~PAGE_MASK_4K)));
/* Fallback to domain selective flush if no PSI support */
if ( !cap_pgsel_inv(iommu->cap) )
- return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry, flush_dev_iotlb);
+ return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry,
+ flush_dev_iotlb);
/* Fallback to domain selective flush if size is too big */
if ( order > cap_max_amask_val(iommu->cap) )
- return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry, flush_dev_iotlb);
+ return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry,
+ flush_dev_iotlb);
addr >>= PAGE_SHIFT_4K + order;
addr <<= PAGE_SHIFT_4K + order;
@@ -535,8 +531,8 @@ static int __must_check iommu_flush_iotlb_psi(struct vtd_iommu *iommu, u16 did,
/* apply platform specific errata workarounds */
vtd_ops_preamble_quirk(iommu);
- status = flush->iotlb(iommu, did, addr, order, DMA_TLB_PSI_FLUSH,
- flush_non_present_entry, flush_dev_iotlb);
+ status = iommu->flush_iotlb(iommu, did, addr, order, DMA_TLB_PSI_FLUSH,
+ flush_non_present_entry, flush_dev_iotlb);
/* undo platform specific errata workarounds */
vtd_ops_postamble_quirk(iommu);
@@ -2158,7 +2154,6 @@ static int __must_check init_vtd_hw(void)
{
struct acpi_drhd_unit *drhd;
struct vtd_iommu *iommu;
- struct iommu_flush *flush = NULL;
int ret;
unsigned long flags;
u32 sts;
@@ -2193,9 +2188,8 @@ static int __must_check init_vtd_hw(void)
*/
if ( enable_qinval(iommu) != 0 )
{
- flush = iommu_get_flush(iommu);
- flush->context = flush_context_reg;
- flush->iotlb = flush_iotlb_reg;
+ iommu->flush_context = flush_context_reg;
+ iommu->flush_iotlb = flush_iotlb_reg;
}
}
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 97d0e6b..a8cffba 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -506,18 +506,7 @@ extern struct list_head acpi_drhd_units;
extern struct list_head acpi_rmrr_units;
extern struct list_head acpi_ioapic_units;
-struct iommu_flush {
- int __must_check (*context)(void *iommu, u16 did, u16 source_id,
- u8 function_mask, u64 type,
- bool_t non_present_entry_flush);
- int __must_check (*iotlb)(void *iommu, u16 did, u64 addr,
- unsigned int size_order, u64 type,
- bool_t flush_non_present_entry,
- bool_t flush_dev_iotlb);
-};
-
struct intel_iommu {
- struct iommu_flush flush;
struct acpi_drhd_unit *drhd;
};
@@ -540,16 +529,19 @@ struct vtd_iommu {
unsigned int iremap_num; /* total num of used interrupt remap entry */
spinlock_t iremap_lock; /* lock for irq remapping table */
+ int __must_check (*flush_context)(struct vtd_iommu *iommu, u16 did,
+ u16 source_id, u8 function_mask, u64 type,
+ bool non_present_entry_flush);
+ int __must_check (*flush_iotlb)(struct vtd_iommu *iommu, u16 did, u64 addr,
+ unsigned int size_order, u64 type,
+ bool flush_non_present_entry,
+ bool flush_dev_iotlb);
+
struct list_head ats_devices;
unsigned long *domid_bitmap; /* domain id bitmap */
u16 *domid_map; /* domain id mapping array */
};
-static inline struct iommu_flush *iommu_get_flush(struct vtd_iommu *iommu)
-{
- return iommu ? &iommu->intel->flush : NULL;
-}
-
#define INTEL_IOMMU_DEBUG(fmt, args...) \
do \
{ \
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index f6fcee5..99e98e7 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -317,12 +317,10 @@ int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx)
return queue_invalidate_iec_sync(iommu, IEC_INDEX_INVL, im, iidx);
}
-static int __must_check flush_context_qi(void *_iommu, u16 did,
+static int __must_check flush_context_qi(struct vtd_iommu *iommu, u16 did,
u16 sid, u8 fm, u64 type,
- bool_t flush_non_present_entry)
+ bool flush_non_present_entry)
{
- struct vtd_iommu *iommu = _iommu;
-
ASSERT(iommu->qinval_maddr);
/*
@@ -343,14 +341,14 @@ static int __must_check flush_context_qi(void *_iommu, u16 did,
type >> DMA_CCMD_INVL_GRANU_OFFSET);
}
-static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
+static int __must_check flush_iotlb_qi(struct vtd_iommu *iommu, u16 did,
+ u64 addr,
unsigned int size_order, u64 type,
- bool_t flush_non_present_entry,
- bool_t flush_dev_iotlb)
+ bool flush_non_present_entry,
+ bool flush_dev_iotlb)
{
u8 dr = 0, dw = 0;
int ret = 0, rc;
- struct vtd_iommu *iommu = _iommu;
ASSERT(iommu->qinval_maddr);
@@ -392,15 +390,12 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
int enable_qinval(struct vtd_iommu *iommu)
{
struct acpi_drhd_unit *drhd;
- struct iommu_flush *flush;
u32 sts;
unsigned long flags;
if ( !ecap_queued_inval(iommu->ecap) || !iommu_qinval )
return -ENOENT;
- flush = iommu_get_flush(iommu);
-
/* Return if already enabled by Xen */
sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
if ( (sts & DMA_GSTS_QIES) && iommu->qinval_maddr )
@@ -417,8 +412,8 @@ int enable_qinval(struct vtd_iommu *iommu)
return -ENOMEM;
}
- flush->context = flush_context_qi;
- flush->iotlb = flush_iotlb_qi;
+ iommu->flush_context = flush_context_qi;
+ iommu->flush_iotlb = flush_iotlb_qi;
spin_lock_irqsave(&iommu->register_lock, flags);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/6] x86/vtd: Drop struct intel_iommu
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
` (4 preceding siblings ...)
2019-02-22 19:13 ` [PATCH 5/6] x86/vtd: Drop struct iommu_flush Andrew Cooper
@ 2019-02-22 19:13 ` Andrew Cooper
2019-02-25 10:21 ` Paul Durrant
2019-02-28 6:10 ` Tian, Kevin
5 siblings, 2 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-22 19:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Paul Durrant, Jan Beulich
The sole remaining member of struct intel_iommu is the drhd backpointer. Move
this into struct vtd_iommu, replacing the the 'intel' pointer.
This removes one dynamic memory allocation per IOMMU on the system.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/drivers/passthrough/vtd/iommu.c | 33 +++++----------------------------
xen/drivers/passthrough/vtd/iommu.h | 6 +-----
xen/drivers/passthrough/vtd/quirks.c | 9 +++------
xen/drivers/passthrough/vtd/utils.c | 2 +-
4 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7fc6fe0..01e2574 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -139,22 +139,6 @@ static int context_get_domain_id(struct context_entry *context,
return domid;
}
-static struct intel_iommu *__init alloc_intel_iommu(void)
-{
- struct intel_iommu *intel;
-
- intel = xzalloc(struct intel_iommu);
- if ( intel == NULL )
- return NULL;
-
- return intel;
-}
-
-static void __init free_intel_iommu(struct intel_iommu *intel)
-{
- xfree(intel);
-}
-
static int iommus_incoherent;
static void __iommu_flush_cache(void *addr, unsigned int size)
{
@@ -869,7 +853,7 @@ static int iommu_page_fault_do_one(struct vtd_iommu *iommu, int type,
{
const char *reason, *kind;
enum faulttype fault_type;
- u16 seg = iommu->intel->drhd->segment;
+ u16 seg = iommu->drhd->segment;
reason = iommu_get_fault_reason(fault_reason, &fault_type);
switch ( fault_type )
@@ -982,7 +966,7 @@ static void __do_iommu_page_fault(struct vtd_iommu *iommu)
iommu_page_fault_do_one(iommu, type, fault_reason,
source_id, guest_addr);
- pci_check_disable_device(iommu->intel->drhd->segment,
+ pci_check_disable_device(iommu->drhd->segment,
PCI_BUS(source_id), PCI_DEVFN2(source_id));
fault_index++;
@@ -1180,13 +1164,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
INIT_LIST_HEAD(&iommu->ats_devices);
spin_lock_init(&iommu->iremap_lock);
- iommu->intel = alloc_intel_iommu();
- if ( iommu->intel == NULL )
- {
- xfree(iommu);
- return -ENOMEM;
- }
- iommu->intel->drhd = drhd;
+ iommu->drhd = drhd;
drhd->iommu = iommu;
if ( !(iommu->root_maddr = alloc_pgtable_maddr(drhd, 1)) )
@@ -1279,7 +1257,6 @@ void __init iommu_free(struct acpi_drhd_unit *drhd)
xfree(iommu->domid_bitmap);
xfree(iommu->domid_map);
- free_intel_iommu(iommu->intel);
if ( iommu->msi.irq >= 0 )
destroy_irq(iommu->msi.irq);
xfree(iommu);
@@ -1329,7 +1306,7 @@ int domain_context_mapping_one(
struct domain_iommu *hd = dom_iommu(domain);
struct context_entry *context, *context_entries;
u64 maddr, pgd_maddr;
- u16 seg = iommu->intel->drhd->segment;
+ u16 seg = iommu->drhd->segment;
int agaw, rc, ret;
bool_t flush_dev_iotlb;
@@ -1618,7 +1595,7 @@ int domain_context_unmap_one(
spin_unlock(&iommu->lock);
unmap_vtd_domain_page(context_entries);
- if ( !iommu->intel->drhd->segment && !rc )
+ if ( !iommu->drhd->segment && !rc )
rc = me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
return rc;
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index a8cffba..808dfcd 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -506,10 +506,6 @@ extern struct list_head acpi_drhd_units;
extern struct list_head acpi_rmrr_units;
extern struct list_head acpi_ioapic_units;
-struct intel_iommu {
- struct acpi_drhd_unit *drhd;
-};
-
struct vtd_iommu {
struct list_head list;
void __iomem *reg; /* Pointer to hardware regs, virtual addr */
@@ -521,7 +517,7 @@ struct vtd_iommu {
spinlock_t register_lock; /* protect iommu register handling */
u64 root_maddr; /* root entry machine address */
struct msi_desc msi;
- struct intel_iommu *intel;
+ struct acpi_drhd_unit *drhd;
uint64_t qinval_maddr; /* queue invalidation page machine address */
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 79209f3..f3a617f 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -139,8 +139,7 @@ static void __init map_igd_reg(void)
*/
static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
{
- struct intel_iommu *intel = iommu->intel;
- struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
+ struct acpi_drhd_unit *drhd = iommu->drhd;
if ( !is_igd_drhd(drhd) || !is_cantiga_b3 )
return 0;
@@ -174,8 +173,7 @@ static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
*/
static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
{
- struct intel_iommu *intel = iommu->intel;
- struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
+ struct acpi_drhd_unit *drhd = iommu->drhd;
s_time_t start_time;
if ( !is_igd_drhd(drhd) || !is_snb_gfx )
@@ -204,8 +202,7 @@ static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
static void snb_vtd_ops_postamble(struct vtd_iommu *iommu)
{
- struct intel_iommu *intel = iommu->intel;
- struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
+ struct acpi_drhd_unit *drhd = iommu->drhd;
if ( !is_igd_drhd(drhd) || !is_snb_gfx )
return;
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
index 72d2235..ee9e6cb 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -96,7 +96,7 @@ void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64 gmfn)
u32 l_index, level;
printk("print_vtd_entries: iommu #%u dev %04x:%02x:%02x.%u gmfn %"PRI_gfn"\n",
- iommu->index, iommu->intel->drhd->segment, bus,
+ iommu->index, iommu->drhd->segment, bus,
PCI_SLOT(devfn), PCI_FUNC(devfn), gmfn);
if ( iommu->root_maddr == 0 )
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu
2019-02-22 19:13 ` [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu Andrew Cooper
@ 2019-02-25 9:56 ` Paul Durrant
2019-02-28 5:57 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Paul Durrant @ 2019-02-25 9:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Jan Beulich
> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich
> <JBeulich@suse.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun Nakajima
> <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>
> Subject: [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu
>
> VT-d's local struct iommu is an overly-generic name, for a structure which
> in
> practice maps 1-to-1 with the real IOMMUs in the system.
>
> Additionally, address style issues on impacted lines. This is mostly
> positioning of * for pointers and unnecessay casts with void pointers.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/drivers/passthrough/vtd/dmar.c | 6 +--
> xen/drivers/passthrough/vtd/dmar.h | 4 +-
> xen/drivers/passthrough/vtd/extern.h | 36 ++++++++---------
> xen/drivers/passthrough/vtd/intremap.c | 26 ++++++------
> xen/drivers/passthrough/vtd/iommu.c | 74 +++++++++++++++++------------
> -----
> xen/drivers/passthrough/vtd/iommu.h | 8 ++--
> xen/drivers/passthrough/vtd/qinval.c | 34 ++++++++--------
> xen/drivers/passthrough/vtd/quirks.c | 10 ++---
> xen/drivers/passthrough/vtd/utils.c | 8 ++--
> xen/drivers/passthrough/vtd/x86/ats.c | 6 +--
> 10 files changed, 106 insertions(+), 106 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/dmar.c
> b/xen/drivers/passthrough/vtd/dmar.c
> index 81afa54..ce1b8ce 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -137,7 +137,7 @@ struct acpi_drhd_unit * ioapic_to_drhd(unsigned int
> apic_id)
> return NULL;
> }
>
> -struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu)
> +struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu)
> {
> struct acpi_drhd_unit *drhd;
>
> @@ -151,7 +151,7 @@ struct acpi_drhd_unit * iommu_to_drhd(struct iommu
> *iommu)
> return NULL;
> }
>
> -struct iommu * ioapic_to_iommu(unsigned int apic_id)
> +struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id)
> {
> struct acpi_drhd_unit *drhd;
>
> @@ -182,7 +182,7 @@ struct acpi_drhd_unit *hpet_to_drhd(unsigned int
> hpet_id)
> return NULL;
> }
>
> -struct iommu *hpet_to_iommu(unsigned int hpet_id)
> +struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id)
> {
> struct acpi_drhd_unit *drhd = hpet_to_drhd(hpet_id);
>
> diff --git a/xen/drivers/passthrough/vtd/dmar.h
> b/xen/drivers/passthrough/vtd/dmar.h
> index 95bb132..1a9c965 100644
> --- a/xen/drivers/passthrough/vtd/dmar.h
> +++ b/xen/drivers/passthrough/vtd/dmar.h
> @@ -63,7 +63,7 @@ struct acpi_drhd_unit {
> u64 address; /* register base address of the
> unit */
> u16 segment;
> u8 include_all:1;
> - struct iommu *iommu;
> + struct vtd_iommu *iommu;
> struct list_head ioapic_list;
> struct list_head hpet_list;
> };
> @@ -128,7 +128,7 @@ do { \
> } while (0)
>
> int vtd_hw_check(void);
> -void disable_pmr(struct iommu *iommu);
> +void disable_pmr(struct vtd_iommu *iommu);
> int is_igd_drhd(struct acpi_drhd_unit *drhd);
>
> #endif /* _DMAR_H_ */
> diff --git a/xen/drivers/passthrough/vtd/extern.h
> b/xen/drivers/passthrough/vtd/extern.h
> index 16eada9..cfef9a3 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -30,38 +30,38 @@ extern bool_t rwbf_quirk;
> extern const struct iommu_ops intel_iommu_ops;
>
> void print_iommu_regs(struct acpi_drhd_unit *drhd);
> -void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64
> gmfn);
> +void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64
> gmfn);
Should clean up the u64 here.
> keyhandler_fn_t vtd_dump_iommu_info;
>
> -int enable_qinval(struct iommu *iommu);
> -void disable_qinval(struct iommu *iommu);
> -int enable_intremap(struct iommu *iommu, int eim);
> -void disable_intremap(struct iommu *iommu);
> +int enable_qinval(struct vtd_iommu *iommu);
> +void disable_qinval(struct vtd_iommu *iommu);
> +int enable_intremap(struct vtd_iommu *iommu, bool eim);
> +void disable_intremap(struct vtd_iommu *iommu);
>
> void iommu_flush_cache_entry(void *addr, unsigned int size);
> void iommu_flush_cache_page(void *addr, unsigned long npages);
> int iommu_alloc(struct acpi_drhd_unit *drhd);
> void iommu_free(struct acpi_drhd_unit *drhd);
>
> -int iommu_flush_iec_global(struct iommu *iommu);
> -int iommu_flush_iec_index(struct iommu *iommu, u8 im, u16 iidx);
> -void clear_fault_bits(struct iommu *iommu);
> +int iommu_flush_iec_global(struct vtd_iommu *iommu);
> +int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx);
u8 and u16.
> +void clear_fault_bits(struct vtd_iommu *iommu);
>
> -struct iommu * ioapic_to_iommu(unsigned int apic_id);
> -struct iommu * hpet_to_iommu(unsigned int hpet_id);
> +struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id);
> +struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id);
> struct acpi_drhd_unit * ioapic_to_drhd(unsigned int apic_id);
> struct acpi_drhd_unit * hpet_to_drhd(unsigned int hpet_id);
> -struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu);
> +struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu);
> struct acpi_rhsa_unit * drhd_to_rhsa(struct acpi_drhd_unit *drhd);
>
I know this sort of thing tends to snowball, but would it not be better to fix the style for the whole block (for consistency)?
> -struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu);
> +struct acpi_drhd_unit *find_ats_dev_drhd(struct vtd_iommu *iommu);
>
> int ats_device(const struct pci_dev *, const struct acpi_drhd_unit *);
>
> -int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
> +int dev_invalidate_iotlb(struct vtd_iommu *iommu, u16 did,
> u64 addr, unsigned int size_order, u64 type);
u16 and u64.
>
> -int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
> +int __must_check qinval_device_iotlb_sync(struct vtd_iommu *iommu,
> struct pci_dev *pdev,
> u16 did, u16 size, u64 addr);
Likewise.
>
> @@ -73,9 +73,9 @@ u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd,
> unsigned long npages);
> void free_pgtable_maddr(u64 maddr);
> void *map_vtd_domain_page(u64 maddr);
> void unmap_vtd_domain_page(void *va);
> -int domain_context_mapping_one(struct domain *domain, struct iommu
> *iommu,
> +int domain_context_mapping_one(struct domain *domain, struct vtd_iommu
> *iommu,
> u8 bus, u8 devfn, const struct pci_dev *);
u8.
> -int domain_context_unmap_one(struct domain *domain, struct iommu *iommu,
> +int domain_context_unmap_one(struct domain *domain, struct vtd_iommu
> *iommu,
> u8 bus, u8 devfn);
And again. There are more but I won't comment individually any more. I don't see any other issues with the rest of the patch.
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-22 19:13 ` [PATCH 3/6] x86/vtd: Drop struct qi_ctrl Andrew Cooper
@ 2019-02-25 10:02 ` Paul Durrant
2019-02-25 10:59 ` Jan Beulich
2019-02-28 6:02 ` Tian, Kevin
1 sibling, 1 reply; 24+ messages in thread
From: Paul Durrant @ 2019-02-25 10:02 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jan Beulich
> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Paul Durrant
> <Paul.Durrant@citrix.com>; Kevin Tian <kevin.tian@intel.com>
> Subject: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
>
> It is unclear why this abstraction exists, but iommu_qi_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 sole member into struct vtd_iommu, and delete iommu_qi_ctrl().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/drivers/passthrough/vtd/iommu.h | 13 +++--------
> xen/drivers/passthrough/vtd/qinval.c | 42 +++++++++++++++---------------------
> 2 files changed, 20 insertions(+), 35 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
> index 556b3d6..12b531c 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -506,10 +506,6 @@ extern struct list_head acpi_drhd_units;
> extern struct list_head acpi_rmrr_units;
> extern struct list_head acpi_ioapic_units;
>
> -struct qi_ctrl {
> - u64 qinval_maddr; /* queue invalidation page machine address */
> -};
> -
> struct ir_ctrl {
> u64 iremap_maddr; /* interrupt remap table machine address */
> int iremap_num; /* total num of used interrupt remap entry */
> @@ -527,7 +523,6 @@ struct iommu_flush {
> };
>
> struct intel_iommu {
> - struct qi_ctrl qi_ctrl;
> struct ir_ctrl ir_ctrl;
> struct iommu_flush flush;
> struct acpi_drhd_unit *drhd;
> @@ -545,16 +540,14 @@ struct vtd_iommu {
> u64 root_maddr; /* root entry machine address */
> struct msi_desc msi;
> struct intel_iommu *intel;
> +
> + uint64_t qinval_maddr; /* queue invalidation page machine address */
> +
> struct list_head ats_devices;
> unsigned long *domid_bitmap; /* domain id bitmap */
> u16 *domid_map; /* domain id mapping array */
> };
>
> -static inline struct qi_ctrl *iommu_qi_ctrl(struct vtd_iommu *iommu)
> -{
> - return iommu ? &iommu->intel->qi_ctrl : NULL;
> -}
> -
> static inline struct ir_ctrl *iommu_ir_ctrl(struct vtd_iommu *iommu)
> {
> return iommu ? &iommu->intel->ir_ctrl : NULL;
> diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
> index 3ddb9b6..f6fcee5 100644
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -84,7 +84,7 @@ static int __must_check queue_invalidate_context_sync(struct vtd_iommu *iommu,
>
> spin_lock_irqsave(&iommu->register_lock, flags);
> index = qinval_next_index(iommu);
> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
> + entry_base = iommu->qinval_maddr +
> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
^ This calculation looks worthy of a macro or an inline. It is repeated a lot.
Paul
> qinval_entries = map_vtd_domain_page(entry_base);
> qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
> @@ -118,7 +118,7 @@ static int __must_check queue_invalidate_iotlb_sync(struct vtd_iommu *iommu,
>
> spin_lock_irqsave(&iommu->register_lock, flags);
> index = qinval_next_index(iommu);
> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
> + entry_base = iommu->qinval_maddr +
> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
> qinval_entries = map_vtd_domain_page(entry_base);
> qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
> @@ -155,7 +155,7 @@ static int __must_check queue_invalidate_wait(struct vtd_iommu *iommu,
>
> spin_lock_irqsave(&iommu->register_lock, flags);
> index = qinval_next_index(iommu);
> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
> + entry_base = iommu->qinval_maddr +
> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
> qinval_entries = map_vtd_domain_page(entry_base);
> qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
> @@ -201,9 +201,7 @@ static int __must_check queue_invalidate_wait(struct vtd_iommu *iommu,
>
> static int __must_check invalidate_sync(struct vtd_iommu *iommu)
> {
> - struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
> -
> - ASSERT(qi_ctrl->qinval_maddr);
> + ASSERT(iommu->qinval_maddr);
>
> return queue_invalidate_wait(iommu, 0, 1, 1, 0);
> }
> @@ -211,10 +209,9 @@ static int __must_check invalidate_sync(struct vtd_iommu *iommu)
> static int __must_check dev_invalidate_sync(struct vtd_iommu *iommu,
> struct pci_dev *pdev, u16 did)
> {
> - struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
> int rc;
>
> - ASSERT(qi_ctrl->qinval_maddr);
> + ASSERT(iommu->qinval_maddr);
> rc = queue_invalidate_wait(iommu, 0, 1, 1, 1);
> if ( rc == -ETIMEDOUT )
> {
> @@ -248,7 +245,7 @@ int qinval_device_iotlb_sync(struct vtd_iommu *iommu, struct pci_dev *pdev,
> ASSERT(pdev);
> spin_lock_irqsave(&iommu->register_lock, flags);
> index = qinval_next_index(iommu);
> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
> + entry_base = iommu->qinval_maddr +
> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
> qinval_entries = map_vtd_domain_page(entry_base);
> qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
> @@ -282,7 +279,7 @@ static int __must_check queue_invalidate_iec_sync(struct vtd_iommu *iommu,
>
> spin_lock_irqsave(&iommu->register_lock, flags);
> index = qinval_next_index(iommu);
> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
> + entry_base = iommu->qinval_maddr +
> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
> qinval_entries = map_vtd_domain_page(entry_base);
> qinval_entry = &qinval_entries[index % (1 << QINVAL_ENTRY_ORDER)];
> @@ -325,9 +322,8 @@ static int __must_check flush_context_qi(void *_iommu, u16 did,
> bool_t flush_non_present_entry)
> {
> struct vtd_iommu *iommu = _iommu;
> - struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
>
> - ASSERT(qi_ctrl->qinval_maddr);
> + ASSERT(iommu->qinval_maddr);
>
> /*
> * In the non-present entry flush case, if hardware doesn't cache
> @@ -355,9 +351,8 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
> u8 dr = 0, dw = 0;
> int ret = 0, rc;
> struct vtd_iommu *iommu = _iommu;
> - struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
>
> - ASSERT(qi_ctrl->qinval_maddr);
> + ASSERT(iommu->qinval_maddr);
>
> /*
> * In the non-present entry flush case, if hardware doesn't cache
> @@ -397,7 +392,6 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
> int enable_qinval(struct vtd_iommu *iommu)
> {
> struct acpi_drhd_unit *drhd;
> - struct qi_ctrl *qi_ctrl;
> struct iommu_flush *flush;
> u32 sts;
> unsigned long flags;
> @@ -405,24 +399,22 @@ int enable_qinval(struct vtd_iommu *iommu)
> if ( !ecap_queued_inval(iommu->ecap) || !iommu_qinval )
> return -ENOENT;
>
> - qi_ctrl = iommu_qi_ctrl(iommu);
> flush = iommu_get_flush(iommu);
>
> /* Return if already enabled by Xen */
> sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
> - if ( (sts & DMA_GSTS_QIES) && qi_ctrl->qinval_maddr )
> + if ( (sts & DMA_GSTS_QIES) && iommu->qinval_maddr )
> return 0;
>
> - if ( qi_ctrl->qinval_maddr == 0 )
> + if ( iommu->qinval_maddr == 0 )
> {
> drhd = iommu_to_drhd(iommu);
> - qi_ctrl->qinval_maddr = alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR);
> - if ( qi_ctrl->qinval_maddr == 0 )
> - {
> - dprintk(XENLOG_WARNING VTDPREFIX,
> - "Cannot allocate memory for qi_ctrl->qinval_maddr\n");
> +
> + iommu->qinval_maddr =
> + alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR);
> +
> + if ( iommu->qinval_maddr == 0 )
> return -ENOMEM;
> - }
> }
>
> flush->context = flush_context_qi;
> @@ -438,7 +430,7 @@ int enable_qinval(struct vtd_iommu *iommu)
> * to IQA register.
> */
> dmar_writeq(iommu->reg, DMAR_IQA_REG,
> - qi_ctrl->qinval_maddr | QINVAL_PAGE_ORDER);
> + iommu->qinval_maddr | QINVAL_PAGE_ORDER);
>
> dmar_writeq(iommu->reg, DMAR_IQT_REG, 0);
>
> --
> 2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/6] x86/vtd: Drop struct ir_ctrl
2019-02-22 19:13 ` [PATCH 4/6] x86/vtd: Drop struct ir_ctrl Andrew Cooper
@ 2019-02-25 10:09 ` Paul Durrant
2019-02-25 11:00 ` Jan Beulich
0 siblings, 1 reply; 24+ messages in thread
From: Paul Durrant @ 2019-02-25 10:09 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jan Beulich
> -----Original Message-----
[snip]
> 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 */
> +
Elsewhere in the Xen codebase we try to group related fields, so how ahout the following?
struct {
uint64_t maddr; /* interrupt remap table machine address */
unsigned int num; /* total num of used interrupt remap entry */
spinlock_t lock; /* lock for irq remapping table */
} iremap;
You'd than have a fairly mechanical job of replacing '_' with '.' in a few places in your patch but I think it would look better overall.
Paul
> struct list_head ats_devices;
> unsigned long *domid_bitmap; /* domain id bitmap */
> u16 *domid_map; /* domain id mapping array */
> };
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/6] x86/vtd: Drop struct iommu_flush
2019-02-22 19:13 ` [PATCH 5/6] x86/vtd: Drop struct iommu_flush Andrew Cooper
@ 2019-02-25 10:17 ` Paul Durrant
2019-02-28 6:09 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Paul Durrant @ 2019-02-25 10:17 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jan Beulich
> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Paul Durrant
> <Paul.Durrant@citrix.com>; Kevin Tian <kevin.tian@intel.com>
> Subject: [PATCH 5/6] x86/vtd: Drop struct iommu_flush
>
> It is unclear why this abstraction exists, but iommu_get_flush() 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 two function pointers into struct vtd_iommu (using a flush_ prefix),
> and delete iommu_get_flush(). Furthermore, there is no need to pass the IOMMU
> pointer to the callbacks via a void pointer, so change the parameter to be
> correctly typed as struct vtd_iommu. Clean up bool_t to bool in surrounding
> context.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/drivers/passthrough/vtd/iommu.c | 62 ++++++++++++++++--------------------
> xen/drivers/passthrough/vtd/iommu.h | 24 +++++---------
> xen/drivers/passthrough/vtd/qinval.c | 21 +++++-------
> 3 files changed, 44 insertions(+), 63 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 05dc7ff..7fc6fe0 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -334,11 +334,11 @@ static void iommu_flush_write_buffer(struct vtd_iommu *iommu)
> }
>
> /* return value determine if we need a write buffer flush */
> -static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
> - u8 function_mask, u64 type,
> - bool_t flush_non_present_entry)
> +static int __must_check flush_context_reg(struct vtd_iommu *iommu, u16 did,
> + u16 source_id, u8 function_mask,
> + u64 type,
> + bool flush_non_present_entry)
More u8, u16 and u64 cleanup needed.
> {
> - struct vtd_iommu *iommu = _iommu;
> u64 val = 0;
> unsigned long flags;
>
> @@ -387,31 +387,28 @@ static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
> }
>
> static int __must_check iommu_flush_context_global(struct vtd_iommu *iommu,
> - bool_t flush_non_present_entry)
> + bool flush_non_present_entry)
> {
> - struct iommu_flush *flush = iommu_get_flush(iommu);
> - return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
> - flush_non_present_entry);
> + return iommu->flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
> + flush_non_present_entry);
> }
>
> static int __must_check iommu_flush_context_device(struct vtd_iommu *iommu,
> u16 did, u16 source_id,
And here.
> u8 function_mask,
> - bool_t flush_non_present_entry)
> + bool flush_non_present_entry)
> {
> - struct iommu_flush *flush = iommu_get_flush(iommu);
> - return flush->context(iommu, did, source_id, function_mask,
> - DMA_CCMD_DEVICE_INVL,
> - flush_non_present_entry);
> + return iommu->flush_context(iommu, did, source_id, function_mask,
> + DMA_CCMD_DEVICE_INVL, flush_non_present_entry);
> }
>
> /* return value determine if we need a write buffer flush */
> -static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
> +static int __must_check flush_iotlb_reg(struct vtd_iommu *iommu, u16 did,
> + u64 addr,
And here.
> unsigned int size_order, u64 type,
> - bool_t flush_non_present_entry,
> - bool_t flush_dev_iotlb)
> + bool flush_non_present_entry,
> + bool flush_dev_iotlb)
> {
> - struct vtd_iommu *iommu = _iommu;
> int tlb_offset = ecap_iotlb_offset(iommu->ecap);
> u64 val = 0;
> unsigned long flags;
> @@ -474,17 +471,16 @@ static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
> }
>
> static int __must_check iommu_flush_iotlb_global(struct vtd_iommu *iommu,
> - bool_t flush_non_present_entry,
> - bool_t flush_dev_iotlb)
> + bool flush_non_present_entry,
> + bool flush_dev_iotlb)
> {
> - struct iommu_flush *flush = iommu_get_flush(iommu);
> int status;
>
> /* apply platform specific errata workarounds */
> vtd_ops_preamble_quirk(iommu);
>
> - status = flush->iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH,
> - flush_non_present_entry, flush_dev_iotlb);
> + status = iommu->flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH,
> + flush_non_present_entry, flush_dev_iotlb);
>
> /* undo platform specific errata workarounds */
> vtd_ops_postamble_quirk(iommu);
> @@ -496,14 +492,13 @@ static int __must_check iommu_flush_iotlb_dsi(struct vtd_iommu *iommu, u16 did,
And here.
> bool_t flush_non_present_entry,
> bool_t flush_dev_iotlb)
> {
> - struct iommu_flush *flush = iommu_get_flush(iommu);
> int status;
>
> /* apply platform specific errata workarounds */
> vtd_ops_preamble_quirk(iommu);
>
> - status = flush->iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH,
> - flush_non_present_entry, flush_dev_iotlb);
> + status = iommu->flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH,
> + flush_non_present_entry, flush_dev_iotlb);
>
> /* undo platform specific errata workarounds */
> vtd_ops_postamble_quirk(iommu);
> @@ -516,18 +511,19 @@ static int __must_check iommu_flush_iotlb_psi(struct vtd_iommu *iommu, u16 did,
And here.
> bool_t flush_non_present_entry,
> bool_t flush_dev_iotlb)
> {
> - struct iommu_flush *flush = iommu_get_flush(iommu);
> int status;
>
> ASSERT(!(addr & (~PAGE_MASK_4K)));
>
> /* Fallback to domain selective flush if no PSI support */
> if ( !cap_pgsel_inv(iommu->cap) )
> - return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry, flush_dev_iotlb);
> + return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry,
> + flush_dev_iotlb);
>
> /* Fallback to domain selective flush if size is too big */
> if ( order > cap_max_amask_val(iommu->cap) )
> - return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry, flush_dev_iotlb);
> + return iommu_flush_iotlb_dsi(iommu, did, flush_non_present_entry,
> + flush_dev_iotlb);
>
> addr >>= PAGE_SHIFT_4K + order;
> addr <<= PAGE_SHIFT_4K + order;
> @@ -535,8 +531,8 @@ static int __must_check iommu_flush_iotlb_psi(struct vtd_iommu *iommu, u16 did,
And here.
> /* apply platform specific errata workarounds */
> vtd_ops_preamble_quirk(iommu);
>
> - status = flush->iotlb(iommu, did, addr, order, DMA_TLB_PSI_FLUSH,
> - flush_non_present_entry, flush_dev_iotlb);
> + status = iommu->flush_iotlb(iommu, did, addr, order, DMA_TLB_PSI_FLUSH,
> + flush_non_present_entry, flush_dev_iotlb);
>
> /* undo platform specific errata workarounds */
> vtd_ops_postamble_quirk(iommu);
> @@ -2158,7 +2154,6 @@ static int __must_check init_vtd_hw(void)
> {
> struct acpi_drhd_unit *drhd;
> struct vtd_iommu *iommu;
> - struct iommu_flush *flush = NULL;
> int ret;
> unsigned long flags;
> u32 sts;
> @@ -2193,9 +2188,8 @@ static int __must_check init_vtd_hw(void)
> */
> if ( enable_qinval(iommu) != 0 )
> {
> - flush = iommu_get_flush(iommu);
> - flush->context = flush_context_reg;
> - flush->iotlb = flush_iotlb_reg;
> + iommu->flush_context = flush_context_reg;
> + iommu->flush_iotlb = flush_iotlb_reg;
> }
> }
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
> index 97d0e6b..a8cffba 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -506,18 +506,7 @@ extern struct list_head acpi_drhd_units;
> extern struct list_head acpi_rmrr_units;
> extern struct list_head acpi_ioapic_units;
>
> -struct iommu_flush {
> - int __must_check (*context)(void *iommu, u16 did, u16 source_id,
> - u8 function_mask, u64 type,
> - bool_t non_present_entry_flush);
> - int __must_check (*iotlb)(void *iommu, u16 did, u64 addr,
> - unsigned int size_order, u64 type,
> - bool_t flush_non_present_entry,
> - bool_t flush_dev_iotlb);
> -};
> -
> struct intel_iommu {
> - struct iommu_flush flush;
> struct acpi_drhd_unit *drhd;
> };
>
> @@ -540,16 +529,19 @@ struct vtd_iommu {
> unsigned int iremap_num; /* total num of used interrupt remap entry */
> spinlock_t iremap_lock; /* lock for irq remapping table */
>
> + int __must_check (*flush_context)(struct vtd_iommu *iommu, u16 did,
> + u16 source_id, u8 function_mask, u64 type,
> + bool non_present_entry_flush);
Certainly here, since you're moving code.
> + int __must_check (*flush_iotlb)(struct vtd_iommu *iommu, u16 did, u64 addr,
> + unsigned int size_order, u64 type,
> + bool flush_non_present_entry,
> + bool flush_dev_iotlb);
> +
> struct list_head ats_devices;
> unsigned long *domid_bitmap; /* domain id bitmap */
> u16 *domid_map; /* domain id mapping array */
> };
>
> -static inline struct iommu_flush *iommu_get_flush(struct vtd_iommu *iommu)
> -{
> - return iommu ? &iommu->intel->flush : NULL;
> -}
> -
> #define INTEL_IOMMU_DEBUG(fmt, args...) \
> do \
> { \
> diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
> index f6fcee5..99e98e7 100644
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -317,12 +317,10 @@ int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx)
> return queue_invalidate_iec_sync(iommu, IEC_INDEX_INVL, im, iidx);
> }
>
> -static int __must_check flush_context_qi(void *_iommu, u16 did,
> +static int __must_check flush_context_qi(struct vtd_iommu *iommu, u16 did,
> u16 sid, u8 fm, u64 type,
More here.
> - bool_t flush_non_present_entry)
> + bool flush_non_present_entry)
> {
> - struct vtd_iommu *iommu = _iommu;
> -
> ASSERT(iommu->qinval_maddr);
>
> /*
> @@ -343,14 +341,14 @@ static int __must_check flush_context_qi(void *_iommu, u16 did,
> type >> DMA_CCMD_INVL_GRANU_OFFSET);
> }
>
> -static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
> +static int __must_check flush_iotlb_qi(struct vtd_iommu *iommu, u16 did,
> + u64 addr,
And here.
> unsigned int size_order, u64 type,
> - bool_t flush_non_present_entry,
> - bool_t flush_dev_iotlb)
> + bool flush_non_present_entry,
> + bool flush_dev_iotlb)
> {
> u8 dr = 0, dw = 0;
> int ret = 0, rc;
> - struct vtd_iommu *iommu = _iommu;
>
> ASSERT(iommu->qinval_maddr);
>
> @@ -392,15 +390,12 @@ static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
And here.
> int enable_qinval(struct vtd_iommu *iommu)
> {
> struct acpi_drhd_unit *drhd;
> - struct iommu_flush *flush;
> u32 sts;
And here.
Paul
> unsigned long flags;
>
> if ( !ecap_queued_inval(iommu->ecap) || !iommu_qinval )
> return -ENOENT;
>
> - flush = iommu_get_flush(iommu);
> -
> /* Return if already enabled by Xen */
> sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
> if ( (sts & DMA_GSTS_QIES) && iommu->qinval_maddr )
> @@ -417,8 +412,8 @@ int enable_qinval(struct vtd_iommu *iommu)
> return -ENOMEM;
> }
>
> - flush->context = flush_context_qi;
> - flush->iotlb = flush_iotlb_qi;
> + iommu->flush_context = flush_context_qi;
> + iommu->flush_iotlb = flush_iotlb_qi;
>
> spin_lock_irqsave(&iommu->register_lock, flags);
>
> --
> 2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/6] x86/vtd: Drop struct intel_iommu
2019-02-22 19:13 ` [PATCH 6/6] x86/vtd: Drop struct intel_iommu Andrew Cooper
@ 2019-02-25 10:21 ` Paul Durrant
2019-02-28 6:10 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Paul Durrant @ 2019-02-25 10:21 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jan Beulich
> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Paul Durrant
> <Paul.Durrant@citrix.com>; Kevin Tian <kevin.tian@intel.com>
> Subject: [PATCH 6/6] x86/vtd: Drop struct intel_iommu
>
> The sole remaining member of struct intel_iommu is the drhd backpointer. Move
> this into struct vtd_iommu, replacing the the 'intel' pointer.
>
> This removes one dynamic memory allocation per IOMMU on the system.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/drivers/passthrough/vtd/iommu.c | 33 +++++----------------------------
> xen/drivers/passthrough/vtd/iommu.h | 6 +-----
> xen/drivers/passthrough/vtd/quirks.c | 9 +++------
> xen/drivers/passthrough/vtd/utils.c | 2 +-
> 4 files changed, 10 insertions(+), 40 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 7fc6fe0..01e2574 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -139,22 +139,6 @@ static int context_get_domain_id(struct context_entry *context,
> return domid;
> }
>
> -static struct intel_iommu *__init alloc_intel_iommu(void)
> -{
> - struct intel_iommu *intel;
> -
> - intel = xzalloc(struct intel_iommu);
> - if ( intel == NULL )
> - return NULL;
> -
> - return intel;
> -}
> -
> -static void __init free_intel_iommu(struct intel_iommu *intel)
> -{
> - xfree(intel);
> -}
> -
> static int iommus_incoherent;
> static void __iommu_flush_cache(void *addr, unsigned int size)
> {
> @@ -869,7 +853,7 @@ static int iommu_page_fault_do_one(struct vtd_iommu *iommu, int type,
> {
> const char *reason, *kind;
> enum faulttype fault_type;
> - u16 seg = iommu->intel->drhd->segment;
> + u16 seg = iommu->drhd->segment;
>
> reason = iommu_get_fault_reason(fault_reason, &fault_type);
> switch ( fault_type )
> @@ -982,7 +966,7 @@ static void __do_iommu_page_fault(struct vtd_iommu *iommu)
> iommu_page_fault_do_one(iommu, type, fault_reason,
> source_id, guest_addr);
>
> - pci_check_disable_device(iommu->intel->drhd->segment,
> + pci_check_disable_device(iommu->drhd->segment,
> PCI_BUS(source_id), PCI_DEVFN2(source_id));
>
> fault_index++;
> @@ -1180,13 +1164,7 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
> INIT_LIST_HEAD(&iommu->ats_devices);
> spin_lock_init(&iommu->iremap_lock);
>
> - iommu->intel = alloc_intel_iommu();
> - if ( iommu->intel == NULL )
> - {
> - xfree(iommu);
> - return -ENOMEM;
> - }
> - iommu->intel->drhd = drhd;
> + iommu->drhd = drhd;
> drhd->iommu = iommu;
>
> if ( !(iommu->root_maddr = alloc_pgtable_maddr(drhd, 1)) )
> @@ -1279,7 +1257,6 @@ void __init iommu_free(struct acpi_drhd_unit *drhd)
> xfree(iommu->domid_bitmap);
> xfree(iommu->domid_map);
>
> - free_intel_iommu(iommu->intel);
> if ( iommu->msi.irq >= 0 )
> destroy_irq(iommu->msi.irq);
> xfree(iommu);
> @@ -1329,7 +1306,7 @@ int domain_context_mapping_one(
> struct domain_iommu *hd = dom_iommu(domain);
> struct context_entry *context, *context_entries;
> u64 maddr, pgd_maddr;
> - u16 seg = iommu->intel->drhd->segment;
> + u16 seg = iommu->drhd->segment;
u16.
> int agaw, rc, ret;
> bool_t flush_dev_iotlb;
>
> @@ -1618,7 +1595,7 @@ int domain_context_unmap_one(
> spin_unlock(&iommu->lock);
> unmap_vtd_domain_page(context_entries);
>
> - if ( !iommu->intel->drhd->segment && !rc )
> + if ( !iommu->drhd->segment && !rc )
> rc = me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
>
> return rc;
> diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
> index a8cffba..808dfcd 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -506,10 +506,6 @@ extern struct list_head acpi_drhd_units;
> extern struct list_head acpi_rmrr_units;
> extern struct list_head acpi_ioapic_units;
>
> -struct intel_iommu {
> - struct acpi_drhd_unit *drhd;
> -};
> -
> struct vtd_iommu {
> struct list_head list;
> void __iomem *reg; /* Pointer to hardware regs, virtual addr */
> @@ -521,7 +517,7 @@ struct vtd_iommu {
> spinlock_t register_lock; /* protect iommu register handling */
> u64 root_maddr; /* root entry machine address */
Perhaps clean this since it is in context...
> struct msi_desc msi;
> - struct intel_iommu *intel;
> + struct acpi_drhd_unit *drhd;
>
> uint64_t qinval_maddr; /* queue invalidation page machine address */
... so it is consistent with this ^
>
> diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
> index 79209f3..f3a617f 100644
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -139,8 +139,7 @@ static void __init map_igd_reg(void)
> */
> static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
> {
> - struct intel_iommu *intel = iommu->intel;
> - struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> + struct acpi_drhd_unit *drhd = iommu->drhd;
>
> if ( !is_igd_drhd(drhd) || !is_cantiga_b3 )
> return 0;
> @@ -174,8 +173,7 @@ static int cantiga_vtd_ops_preamble(struct vtd_iommu *iommu)
> */
> static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
> {
> - struct intel_iommu *intel = iommu->intel;
> - struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> + struct acpi_drhd_unit *drhd = iommu->drhd;
> s_time_t start_time;
>
> if ( !is_igd_drhd(drhd) || !is_snb_gfx )
> @@ -204,8 +202,7 @@ static void snb_vtd_ops_preamble(struct vtd_iommu *iommu)
>
> static void snb_vtd_ops_postamble(struct vtd_iommu *iommu)
> {
> - struct intel_iommu *intel = iommu->intel;
> - struct acpi_drhd_unit *drhd = intel ? intel->drhd : NULL;
> + struct acpi_drhd_unit *drhd = iommu->drhd;
>
> if ( !is_igd_drhd(drhd) || !is_snb_gfx )
> return;
> diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
> index 72d2235..ee9e6cb 100644
> --- a/xen/drivers/passthrough/vtd/utils.c
> +++ b/xen/drivers/passthrough/vtd/utils.c
> @@ -96,7 +96,7 @@ void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64 gmfn)
u64 (and maybe rename).
Paul
> u32 l_index, level;
>
> printk("print_vtd_entries: iommu #%u dev %04x:%02x:%02x.%u gmfn %"PRI_gfn"\n",
> - iommu->index, iommu->intel->drhd->segment, bus,
> + iommu->index, iommu->drhd->segment, bus,
> PCI_SLOT(devfn), PCI_FUNC(devfn), gmfn);
>
> if ( iommu->root_maddr == 0 )
> --
> 2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-25 10:02 ` Paul Durrant
@ 2019-02-25 10:59 ` Jan Beulich
2019-02-25 11:07 ` Andrew Cooper
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2019-02-25 10:59 UTC (permalink / raw)
To: Andrew Cooper, Paul Durrant; +Cc: Kevin Tian, Xen-devel
>>> On 25.02.19 at 11:02, <Paul.Durrant@citrix.com> wrote:
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: 22 February 2019 19:13
>>
>> --- a/xen/drivers/passthrough/vtd/qinval.c
>> +++ b/xen/drivers/passthrough/vtd/qinval.c
>> @@ -84,7 +84,7 @@ static int __must_check queue_invalidate_context_sync(struct vtd_iommu *iommu,
>>
>> spin_lock_irqsave(&iommu->register_lock, flags);
>> index = qinval_next_index(iommu);
>> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
>> + entry_base = iommu->qinval_maddr +
>> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
>
> ^ This calculation looks worthy of a macro or an inline. It is repeated a
> lot.
And indeed the other day I was surprised that there is
GET_IREMAP_ENTRY(), but no qinval equivalent.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/6] x86/vtd: Drop struct ir_ctrl
2019-02-25 10:09 ` Paul Durrant
@ 2019-02-25 11:00 ` Jan Beulich
2019-02-28 6:07 ` Tian, Kevin
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2019-02-25 11:00 UTC (permalink / raw)
To: Andrew Cooper, Paul Durrant; +Cc: Kevin Tian, Xen-devel
>>> On 25.02.19 at 11:09, <Paul.Durrant@citrix.com> wrote:
>> -----Original Message-----
> [snip]
>> 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 */
>> +
>
> Elsewhere in the Xen codebase we try to group related fields, so how ahout
> the following?
>
> struct {
> uint64_t maddr; /* interrupt remap table machine address */
> unsigned int num; /* total num of used interrupt remap entry */
> spinlock_t lock; /* lock for irq remapping table */
> } iremap;
>
> You'd than have a fairly mechanical job of replacing '_' with '.' in a few
> places in your patch but I think it would look better overall.
+1
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-25 10:59 ` Jan Beulich
@ 2019-02-25 11:07 ` Andrew Cooper
2019-02-25 11:25 ` Jan Beulich
0 siblings, 1 reply; 24+ messages in thread
From: Andrew Cooper @ 2019-02-25 11:07 UTC (permalink / raw)
To: Jan Beulich, Paul Durrant; +Cc: Kevin Tian, Xen-devel
On 25/02/2019 10:59, Jan Beulich wrote:
>>>> On 25.02.19 at 11:02, <Paul.Durrant@citrix.com> wrote:
>>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>>> Sent: 22 February 2019 19:13
>>>
>>> --- a/xen/drivers/passthrough/vtd/qinval.c
>>> +++ b/xen/drivers/passthrough/vtd/qinval.c
>>> @@ -84,7 +84,7 @@ static int __must_check queue_invalidate_context_sync(struct vtd_iommu *iommu,
>>>
>>> spin_lock_irqsave(&iommu->register_lock, flags);
>>> index = qinval_next_index(iommu);
>>> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
>>> + entry_base = iommu->qinval_maddr +
>>> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
>> ^ This calculation looks worthy of a macro or an inline. It is repeated a
>> lot.
> And indeed the other day I was surprised that there is
> GET_IREMAP_ENTRY(), but no qinval equivalent.
I won't be making any changes like that to this patch. There is an
orthogonal piece of work to vmap these structures and replace all of
this logic with a straight array lookup by index. Unfortunately we
don't have memremap() in Xen yet and I don't have time right now to sort
that.
GET_IREMAP_ENTRY() is a particularly bad example of a helper, as results
in several necessary pieces of calculation in certain cases, and hides a
use of map_domain_page() which results in the logic which uses it
looking asymmetric.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-25 11:07 ` Andrew Cooper
@ 2019-02-25 11:25 ` Jan Beulich
0 siblings, 0 replies; 24+ messages in thread
From: Jan Beulich @ 2019-02-25 11:25 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Kevin Tian, Paul Durrant, Xen-devel
>>> On 25.02.19 at 12:07, <andrew.cooper3@citrix.com> wrote:
> On 25/02/2019 10:59, Jan Beulich wrote:
>>>>> On 25.02.19 at 11:02, <Paul.Durrant@citrix.com> wrote:
>>>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>>>> Sent: 22 February 2019 19:13
>>>>
>>>> --- a/xen/drivers/passthrough/vtd/qinval.c
>>>> +++ b/xen/drivers/passthrough/vtd/qinval.c
>>>> @@ -84,7 +84,7 @@ static int __must_check
> queue_invalidate_context_sync(struct vtd_iommu *iommu,
>>>>
>>>> spin_lock_irqsave(&iommu->register_lock, flags);
>>>> index = qinval_next_index(iommu);
>>>> - entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
>>>> + entry_base = iommu->qinval_maddr +
>>>> ((index >> QINVAL_ENTRY_ORDER) << PAGE_SHIFT);
>>> ^ This calculation looks worthy of a macro or an inline. It is repeated a
>>> lot.
>> And indeed the other day I was surprised that there is
>> GET_IREMAP_ENTRY(), but no qinval equivalent.
>
> I won't be making any changes like that to this patch. There is an
> orthogonal piece of work to vmap these structures and replace all of
> this logic with a straight array lookup by index. Unfortunately we
> don't have memremap() in Xen yet and I don't have time right now to sort
> that.
>
> GET_IREMAP_ENTRY() is a particularly bad example of a helper, as results
> in several necessary pieces of calculation in certain cases, and hides a
> use of map_domain_page() which results in the logic which uses it
> looking asymmetric.
Oh, I'm sorry if this came over in an ambiguous way: I didn't
mean to suggest to directly mirror that strange macro. I've
just used its existence to support Paul's request.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
@ 2019-02-25 11:39 ` George Dunlap
2019-02-28 11:28 ` Andrew Cooper
2019-02-28 5:54 ` Tian, Kevin
1 sibling, 1 reply; 24+ messages in thread
From: George Dunlap @ 2019-02-25 11:39 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Kevin Tian, Xen-devel
On Fri, Feb 22, 2019 at 7:15 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> iremap_maddr and qinval_maddr point to the base of a block of contiguous RAM,
> allocated by the driver, holding the Interrupt Remapping table, and the Queued
> Invalidation ring.
>
> Despite their name, they are actually the values of the hardware register,
> including control metadata in the lower 12 bits. While uses of these fields
> do appear to correctly shift out the metadata, this is very subtle behaviour
> and confusing to follow.
Could I suggest that we add an ASSERT() to that macro, that maddr is
page aligned?
The algorithm used certainly assumes that's the case, and would
produce incorrect results if it were ever validly true. Such an
ASSERT() would also have detected the storing-garbage-in-maddr issue
almost immediately.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
2019-02-25 11:39 ` George Dunlap
@ 2019-02-28 5:54 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 5:54 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
>
> iremap_maddr and qinval_maddr point to the base of a block of contiguous
> RAM,
> allocated by the driver, holding the Interrupt Remapping table, and the
> Queued
> Invalidation ring.
>
> Despite their name, they are actually the values of the hardware register,
> including control metadata in the lower 12 bits. While uses of these fields
> do appear to correctly shift out the metadata, this is very subtle behaviour
> and confusing to follow.
>
> Nothing uses the metadata, so make the fields actually point at the base of
> the relevant tables.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Good catch!
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu
2019-02-22 19:13 ` [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu Andrew Cooper
2019-02-25 9:56 ` Paul Durrant
@ 2019-02-28 5:57 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 5:57 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Paul Durrant, Nakajima, Jun, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
>
> VT-d's local struct iommu is an overly-generic name, for a structure which in
> practice maps 1-to-1 with the real IOMMUs in the system.
>
> Additionally, address style issues on impacted lines. This is mostly
> positioning of * for pointers and unnecessay casts with void pointers.
unnecessary
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/6] x86/vtd: Drop struct qi_ctrl
2019-02-22 19:13 ` [PATCH 3/6] x86/vtd: Drop struct qi_ctrl Andrew Cooper
2019-02-25 10:02 ` Paul Durrant
@ 2019-02-28 6:02 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 6:02 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Paul Durrant, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
>
> It is unclear why this abstraction exists, but iommu_qi_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 sole member into struct vtd_iommu, and delete iommu_qi_ctrl().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/6] x86/vtd: Drop struct ir_ctrl
2019-02-25 11:00 ` Jan Beulich
@ 2019-02-28 6:07 ` Tian, Kevin
0 siblings, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 6:07 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper, Paul Durrant; +Cc: Xen-devel
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, February 25, 2019 7:01 PM
>
> >>> On 25.02.19 at 11:09, <Paul.Durrant@citrix.com> wrote:
> >> -----Original Message-----
> > [snip]
> >> 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 */
> >> +
> >
> > Elsewhere in the Xen codebase we try to group related fields, so how ahout
> > the following?
> >
> > struct {
> > uint64_t maddr; /* interrupt remap table machine address */
> > unsigned int num; /* total num of used interrupt remap entry */
> > spinlock_t lock; /* lock for irq remapping table */
> > } iremap;
> >
> > You'd than have a fairly mechanical job of replacing '_' with '.' in a few
> > places in your patch but I think it would look better overall.
>
> +1
>
agree.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/6] x86/vtd: Drop struct iommu_flush
2019-02-22 19:13 ` [PATCH 5/6] x86/vtd: Drop struct iommu_flush Andrew Cooper
2019-02-25 10:17 ` Paul Durrant
@ 2019-02-28 6:09 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 6:09 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Paul Durrant, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
>
> It is unclear why this abstraction exists, but iommu_get_flush() 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 two function pointers into struct vtd_iommu (using a flush_ prefix),
> and delete iommu_get_flush(). Furthermore, there is no need to pass the
> IOMMU
> pointer to the callbacks via a void pointer, so change the parameter to be
> correctly typed as struct vtd_iommu. Clean up bool_t to bool in surrounding
> context.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/6] x86/vtd: Drop struct intel_iommu
2019-02-22 19:13 ` [PATCH 6/6] x86/vtd: Drop struct intel_iommu Andrew Cooper
2019-02-25 10:21 ` Paul Durrant
@ 2019-02-28 6:10 ` Tian, Kevin
1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2019-02-28 6:10 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Paul Durrant, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
>
> The sole remaining member of struct intel_iommu is the drhd backpointer.
> Move
> this into struct vtd_iommu, replacing the the 'intel' pointer.
>
> This removes one dynamic memory allocation per IOMMU on the system.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers
2019-02-25 11:39 ` George Dunlap
@ 2019-02-28 11:28 ` Andrew Cooper
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Cooper @ 2019-02-28 11:28 UTC (permalink / raw)
To: George Dunlap; +Cc: Kevin Tian, Xen-devel
On 25/02/2019 11:39, George Dunlap wrote:
> On Fri, Feb 22, 2019 at 7:15 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> iremap_maddr and qinval_maddr point to the base of a block of contiguous RAM,
>> allocated by the driver, holding the Interrupt Remapping table, and the Queued
>> Invalidation ring.
>>
>> Despite their name, they are actually the values of the hardware register,
>> including control metadata in the lower 12 bits. While uses of these fields
>> do appear to correctly shift out the metadata, this is very subtle behaviour
>> and confusing to follow.
> Could I suggest that we add an ASSERT() to that macro, that maddr is
> page aligned?
>
> The algorithm used certainly assumes that's the case, and would
> produce incorrect results if it were ever validly true. Such an
> ASSERT() would also have detected the storing-garbage-in-maddr issue
> almost immediately.
Where would you suggest putting such an ASSERT() ? This variable is
written exactly once.
Irrespective, I plan to replace the existing scheme with a vmap()'d one
which will delete this variable completely.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-02-28 11:28 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22 19:13 [PATCH 0/6] x86/vtd: Removal of unnecessary abstractions Andrew Cooper
2019-02-22 19:13 ` [PATCH 1/6] x86/vtd: Don't include control register state in the table pointers Andrew Cooper
2019-02-25 11:39 ` George Dunlap
2019-02-28 11:28 ` Andrew Cooper
2019-02-28 5:54 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu Andrew Cooper
2019-02-25 9:56 ` Paul Durrant
2019-02-28 5:57 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 3/6] x86/vtd: Drop struct qi_ctrl Andrew Cooper
2019-02-25 10:02 ` Paul Durrant
2019-02-25 10:59 ` Jan Beulich
2019-02-25 11:07 ` Andrew Cooper
2019-02-25 11:25 ` Jan Beulich
2019-02-28 6:02 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 4/6] x86/vtd: Drop struct ir_ctrl Andrew Cooper
2019-02-25 10:09 ` Paul Durrant
2019-02-25 11:00 ` Jan Beulich
2019-02-28 6:07 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 5/6] x86/vtd: Drop struct iommu_flush Andrew Cooper
2019-02-25 10:17 ` Paul Durrant
2019-02-28 6:09 ` Tian, Kevin
2019-02-22 19:13 ` [PATCH 6/6] x86/vtd: Drop struct intel_iommu Andrew Cooper
2019-02-25 10:21 ` Paul Durrant
2019-02-28 6:10 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.