From: Jan Beulich Subject: AMD/IOMMU: ensure suitable ordering of DTE modifications DMA and interrupt translation should be enabled only after other applicable DTE fields have been written. Similarly when disabling translation or when moving a device between domains, translation should first be disabled, before other entry fields get modified. Note however that the "moving" aspect doesn't apply to the interrupt remapping side, as domain specifics are maintained in the IRTEs here, not the DTE. We also never disable interrupt remapping once it got enabled for a device (the respective argument passed is always the immutable iommu_intremap). This is part of XSA-347. Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant --- a/xen/drivers/passthrough/amd/iommu_map.c +++ b/xen/drivers/passthrough/amd/iommu_map.c @@ -162,7 +162,22 @@ void amd_iommu_set_root_page_table(uint3 uint16_t domain_id, uint8_t paging_mode, uint8_t valid) { - uint32_t addr_hi, addr_lo, entry; + uint32_t addr_hi, addr_lo, entry, dte0 = dte[0]; + + if ( valid || + get_field_from_reg_u32(dte0, IOMMU_DEV_TABLE_VALID_MASK, + IOMMU_DEV_TABLE_VALID_SHIFT) ) + { + set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, dte0, + IOMMU_DEV_TABLE_TRANSLATION_VALID_MASK, + IOMMU_DEV_TABLE_TRANSLATION_VALID_SHIFT, &dte0); + set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, dte0, + IOMMU_DEV_TABLE_VALID_MASK, + IOMMU_DEV_TABLE_VALID_SHIFT, &dte0); + dte[0] = dte0; + smp_wmb(); + } + set_field_in_reg_u32(domain_id, 0, IOMMU_DEV_TABLE_DOMAIN_ID_MASK, IOMMU_DEV_TABLE_DOMAIN_ID_SHIFT, &entry); @@ -181,8 +196,9 @@ void amd_iommu_set_root_page_table(uint3 IOMMU_DEV_TABLE_IO_READ_PERMISSION_MASK, IOMMU_DEV_TABLE_IO_READ_PERMISSION_SHIFT, &entry); dte[1] = entry; + smp_wmb(); - set_field_in_reg_u32(addr_lo >> PAGE_SHIFT, 0, + set_field_in_reg_u32(addr_lo >> PAGE_SHIFT, dte0, IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK, IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT, &entry); set_field_in_reg_u32(paging_mode, entry, @@ -195,7 +211,7 @@ void amd_iommu_set_root_page_table(uint3 IOMMU_CONTROL_DISABLED, entry, IOMMU_DEV_TABLE_VALID_MASK, IOMMU_DEV_TABLE_VALID_SHIFT, &entry); - dte[0] = entry; + write_atomic(&dte[0], entry); } void iommu_dte_set_iotlb(uint32_t *dte, uint8_t i) @@ -226,6 +242,7 @@ void __init amd_iommu_set_intremap_table IOMMU_DEV_TABLE_INT_CONTROL_MASK, IOMMU_DEV_TABLE_INT_CONTROL_SHIFT, &entry); dte[5] = entry; + smp_wmb(); set_field_in_reg_u32(addr_lo >> 6, 0, IOMMU_DEV_TABLE_INT_TABLE_PTR_LOW_MASK, @@ -243,7 +260,7 @@ void __init amd_iommu_set_intremap_table IOMMU_CONTROL_DISABLED, entry, IOMMU_DEV_TABLE_INT_VALID_MASK, IOMMU_DEV_TABLE_INT_VALID_SHIFT, &entry); - dte[4] = entry; + write_atomic(&dte[4], entry); } void __init iommu_dte_add_device_entry(uint32_t *dte,