@@ -649,6 +649,12 @@ bool __init cf_check iov_supports_xt(void)
if ( !iommu_enable || !iommu_intremap )
return false;
+ if ( unlikely(!cpu_has_cx16) )
+ {
+ AMD_IOMMU_WARN("CPU doesn't support CMPXCHG16B, disable interrupt remapping\n");
+ return false;
+ }
+
if ( amd_iommu_prepare(true) )
return false;
@@ -150,6 +150,13 @@ bool __init cf_check intel_iommu_supports_eim(void)
if ( !iommu_qinval || !iommu_intremap || list_empty(&acpi_drhd_units) )
return false;
+ if ( unlikely(!cpu_has_cx16) )
+ {
+ printk(XENLOG_WARNING VTDPREFIX
+ "CPU doesn't support CMPXCHG16B, disable interrupt remapping\n");
+ return false;
+ }
+
/* We MUST have a DRHD unit for each IOAPIC. */
for ( apic = 0; apic < nr_ioapics; apic++ )
if ( !ioapic_to_drhd(IO_APIC_ID(apic)) )
@@ -173,47 +180,26 @@ bool __init cf_check intel_iommu_supports_eim(void)
* Assume iremap_lock has been acquired. It is to make sure software will not
* change the same IRTE behind us. With this assumption, if only high qword or
* low qword in IRTE is to be updated, this function's atomic variant can
- * present an atomic update to VT-d hardware even when cmpxchg16b
- * instruction is not supported.
+ * present an atomic update to VT-d hardware.
*/
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->intremap.lock));
+ __uint128_t ret;
+ struct iremap_entry old_ire;
- if ( cpu_has_cx16 )
- {
- __uint128_t ret;
- struct iremap_entry old_ire;
+ ASSERT(spin_is_locked(&iommu->intremap.lock));
- old_ire = *entry;
- ret = cmpxchg16b(entry, &old_ire, new_ire);
+ old_ire = *entry;
+ ret = cmpxchg16b(entry, &old_ire, new_ire);
- /*
- * In the above, we use cmpxchg16 to atomically update the 128-bit
- * IRTE, and the hardware cannot update the IRTE behind us, so
- * the return value of cmpxchg16 should be the same as old_ire.
- * This ASSERT validate it.
- */
- ASSERT(ret == old_ire.val);
- }
- else
- {
- /*
- * VT-d hardware doesn't update IRTEs behind us, nor the software
- * since we hold iremap_lock. If the caller wants VT-d hardware to
- * always see a consistent entry, but we can't meet it, a bug will
- * be raised.
- */
- if ( entry->lo == new_ire->lo )
- write_atomic(&entry->hi, new_ire->hi);
- else if ( entry->hi == new_ire->hi )
- write_atomic(&entry->lo, new_ire->lo);
- else if ( !atomic )
- *entry = *new_ire;
- else
- BUG();
- }
+ /*
+ * In the above, we use cmpxchg16 to atomically update the 128-bit
+ * IRTE, and the hardware cannot update the IRTE behind us, so
+ * the return value of cmpxchg16 should be the same as old_ire.
+ * This ASSERT validate it.
+ */
+ ASSERT(ret == old_ire.val);
}
/* Mark specified intr remap entry as free */
@@ -395,7 +381,6 @@ static int ioapic_rte_to_remap_entry(struct vtd_iommu *iommu,
/* Indicate remap format. */
remap_rte->format = 1;
- /* If cmpxchg16b is not available the caller must mask the IO-APIC pin. */
update_irte(iommu, iremap_entry, &new_ire, !init && !masked);
iommu_sync_cache(iremap_entry, sizeof(*iremap_entry));
iommu_flush_iec_index(iommu, 0, index);
@@ -437,21 +422,6 @@ void cf_check io_apic_write_remap_rte(
bool masked = true;
int rc;
- if ( !cpu_has_cx16 )
- {
- /*
- * Cannot atomically update the IRTE entry: mask the IO-APIC pin to
- * avoid interrupts seeing an inconsistent IRTE entry.
- */
- old_rte = __ioapic_read_entry(apic, pin, true);
- if ( !old_rte.mask )
- {
- masked = false;
- old_rte.mask = 1;
- __ioapic_write_entry(apic, pin, true, old_rte);
- }
- }
-
/* Not the initializer, for old gcc to cope. */
new_rte.raw = rte;
@@ -2663,12 +2663,7 @@ static int __init cf_check vtd_setup(void)
iommu_intremap = iommu_intremap_off;
#ifndef iommu_intpost
- /*
- * We cannot use posted interrupt if X86_FEATURE_CX16 is
- * not supported, since we count on this feature to
- * atomically update 16-byte IRTE in posted format.
- */
- if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 )
+ if ( !cap_intr_post(iommu->cap) || !iommu_intremap )
iommu_intpost = false;
#endif
All hardware with VT-d/AMD-Vi has CMPXCHG16B support. Check this at initialisation time, and remove the effectively-dead logic for the non-cx16 case. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> --- xen/drivers/passthrough/amd/iommu_intr.c | 6 ++ xen/drivers/passthrough/vtd/intremap.c | 70 +++++++----------------- xen/drivers/passthrough/vtd/iommu.c | 7 +-- 3 files changed, 27 insertions(+), 56 deletions(-)