ACPI: acpi_table_parse() should return handler's error code Currently, the error code returned by acpi_table_parse()'s handler is ignored. This patch will propagate handler's return value to acpi_table_parse()'s caller. AMD,IOMMU: Clean up old entries in remapping tables when creating new interrupt mapping. When changing the affinity of an IRQ associated with a passed through PCI device, clear previous mapping. In addition, because some BIOSes may incorrectly program IVRS entries for IOAPIC try to check for entry's consistency. Specifically, if conflicting entries are found disable IOMMU if per-device remapping table is used. If entries refer to bogus IOAPIC IDs disable IOMMU unconditionally AMD,IOMMU: Disable IOMMU if SATA Combined mode is on AMD's SP5100 chipset can be placed into SATA Combined mode that may cause prevent dom0 from booting when IOMMU is enabled and per-device interrupt remapping table is used. While SP5100 erratum 28 requires BIOSes to disable this mode, some may still use it. This patch checks whether this mode is on and, if per-device table is in use, disables IOMMU. AMD,IOMMU: Make per-device interrupt remapping table default Using global interrupt remapping table may be insecure, as described by XSA-36. This patch makes per-device mode default. This is XSA-36 / CVE-2013-0153. Signed-off-by: Jan Beulich Signed-off-by: Boris Ostrovsky --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -1942,9 +1942,6 @@ int map_domain_pirq( spin_lock_irqsave(&desc->lock, flags); set_domain_irq_pirq(d, irq, info); spin_unlock_irqrestore(&desc->lock, flags); - - if ( opt_irq_vector_map == OPT_IRQ_VECTOR_MAP_PERDEV ) - printk(XENLOG_INFO "Per-device vector maps for GSIs not implemented yet.\n"); } done: --- a/xen/drivers/acpi/tables.c +++ b/xen/drivers/acpi/tables.c @@ -267,7 +267,7 @@ acpi_table_parse_madt(enum acpi_madt_typ * @handler: handler to run * * Scan the ACPI System Descriptor Table (STD) for a table matching @id, - * run @handler on it. Return 0 if table found, return on if not. + * run @handler on it. */ int __init acpi_table_parse(char *id, acpi_table_handler handler) { @@ -282,8 +282,7 @@ int __init acpi_table_parse(char *id, ac acpi_get_table(id, 0, &table); if (table) { - handler(table); - return 0; + return handler(table); } else return 1; } --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -635,6 +636,7 @@ static u16 __init parse_ivhd_device_spec u16 header_length, u16 block_length, struct amd_iommu *iommu) { u16 dev_length, bdf; + int apic; dev_length = sizeof(*special); if ( header_length < (block_length + dev_length) ) @@ -651,10 +653,59 @@ static u16 __init parse_ivhd_device_spec } add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu); - /* set device id of ioapic */ - ioapic_sbdf[special->handle].bdf = bdf; - ioapic_sbdf[special->handle].seg = seg; - return dev_length; + + if ( special->variety != ACPI_IVHD_IOAPIC ) + { + if ( special->variety != ACPI_IVHD_HPET ) + printk(XENLOG_ERR "Unrecognized IVHD special variety %#x\n", + special->variety); + return dev_length; + } + + /* + * Some BIOSes have IOAPIC broken entries so we check for IVRS + * consistency here --- whether entry's IOAPIC ID is valid and + * whether there are conflicting/duplicated entries. + */ + for ( apic = 0; apic < nr_ioapics; apic++ ) + { + if ( IO_APIC_ID(apic) != special->handle ) + continue; + + if ( ioapic_sbdf[special->handle].pin_setup ) + { + if ( ioapic_sbdf[special->handle].bdf == bdf && + ioapic_sbdf[special->handle].seg == seg ) + AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC %#x entries\n", + special->handle); + else + { + printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC %#x entries\n", + special->handle); + if ( amd_iommu_perdev_intremap ) + return 0; + } + } + else + { + /* set device id of ioapic */ + ioapic_sbdf[special->handle].bdf = bdf; + ioapic_sbdf[special->handle].seg = seg; + + ioapic_sbdf[special->handle].pin_setup = xzalloc_array( + unsigned long, BITS_TO_LONGS(nr_ioapic_entries[apic])); + if ( nr_ioapic_entries[apic] && + !ioapic_sbdf[IO_APIC_ID(apic)].pin_setup ) + { + printk(XENLOG_ERR "IVHD Error: Out of memory\n"); + return 0; + } + } + return dev_length; + } + + printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n", special->handle); + return 0; } static int __init parse_ivhd_block(const struct acpi_ivrs_hardware *ivhd_block) --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -1126,12 +1126,45 @@ static int __init amd_iommu_setup_device return 0; } +/* Check whether SP5100 SATA Combined mode is on */ +static bool_t __init amd_sp5100_erratum28(void) +{ + u32 bus, id; + u16 vendor_id, dev_id; + u8 byte; + + for (bus = 0; bus < 256; bus++) + { + id = pci_conf_read32(0, bus, 0x14, 0, PCI_VENDOR_ID); + + vendor_id = id & 0xffff; + dev_id = (id >> 16) & 0xffff; + + /* SP5100 SMBus module sets Combined mode on */ + if (vendor_id != 0x1002 || dev_id != 0x4385) + continue; + + byte = pci_conf_read8(0, bus, 0x14, 0, 0xad); + if ( (byte >> 3) & 1 ) + { + printk(XENLOG_WARNING "AMD-Vi: SP5100 erratum 28 detected, disabling IOMMU.\n" + "If possible, disable SATA Combined mode in BIOS or contact your vendor for BIOS update.\n"); + return 1; + } + } + + return 0; +} + int __init amd_iommu_init(void) { struct amd_iommu *iommu; BUG_ON( !iommu_found() ); + if ( amd_iommu_perdev_intremap && amd_sp5100_erratum28() ) + goto error_out; + ivrs_bdf_entries = amd_iommu_get_ivrs_dev_entries(); if ( !ivrs_bdf_entries ) --- a/xen/drivers/passthrough/amd/iommu_intr.c +++ b/xen/drivers/passthrough/amd/iommu_intr.c @@ -99,12 +99,12 @@ static void update_intremap_entry(u32* e static void update_intremap_entry_from_ioapic( int bdf, struct amd_iommu *iommu, - struct IO_APIC_route_entry *ioapic_rte) + const struct IO_APIC_route_entry *rte, + const struct IO_APIC_route_entry *old_rte) { unsigned long flags; u32* entry; u8 delivery_mode, dest, vector, dest_mode; - struct IO_APIC_route_entry *rte = ioapic_rte; int req_id; spinlock_t *lock; int offset; @@ -120,6 +120,14 @@ static void update_intremap_entry_from_i spin_lock_irqsave(lock, flags); offset = get_intremap_offset(vector, delivery_mode); + if ( old_rte ) + { + int old_offset = get_intremap_offset(old_rte->vector, + old_rte->delivery_mode); + + if ( offset != old_offset ) + free_intremap_entry(iommu->seg, bdf, old_offset); + } entry = (u32*)get_intremap_entry(iommu->seg, req_id, offset); update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest); @@ -188,6 +196,7 @@ int __init amd_iommu_setup_ioapic_remapp amd_iommu_flush_intremap(iommu, req_id); spin_unlock_irqrestore(&iommu->lock, flags); } + set_bit(pin, ioapic_sbdf[IO_APIC_ID(apic)].pin_setup); } } return 0; @@ -199,6 +208,7 @@ void amd_iommu_ioapic_update_ire( struct IO_APIC_route_entry old_rte = { 0 }; struct IO_APIC_route_entry new_rte = { 0 }; unsigned int rte_lo = (reg & 1) ? reg - 1 : reg; + unsigned int pin = (reg - 0x10) / 2; int saved_mask, seg, bdf; struct amd_iommu *iommu; @@ -236,6 +246,14 @@ void amd_iommu_ioapic_update_ire( *(((u32 *)&new_rte) + 1) = value; } + if ( new_rte.mask && + !test_bit(pin, ioapic_sbdf[IO_APIC_ID(apic)].pin_setup) ) + { + ASSERT(saved_mask); + __io_apic_write(apic, reg, value); + return; + } + /* mask the interrupt while we change the intremap table */ if ( !saved_mask ) { @@ -244,7 +262,11 @@ void amd_iommu_ioapic_update_ire( } /* Update interrupt remapping entry */ - update_intremap_entry_from_ioapic(bdf, iommu, &new_rte); + update_intremap_entry_from_ioapic( + bdf, iommu, &new_rte, + test_and_set_bit(pin, + ioapic_sbdf[IO_APIC_ID(apic)].pin_setup) ? &old_rte + : NULL); /* Forward write access to IO-APIC RTE */ __io_apic_write(apic, reg, value); @@ -354,6 +376,12 @@ void amd_iommu_msi_msg_update_ire( return; } + if ( msi_desc->remap_index >= 0 ) + update_intremap_entry_from_msi_msg(iommu, pdev, msi_desc, NULL); + + if ( !msg ) + return; + update_intremap_entry_from_msi_msg(iommu, pdev, msi_desc, msg); } --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -205,6 +205,8 @@ int __init amd_iov_detect(void) { printk("AMD-Vi: Not overriding irq_vector_map setting\n"); } + if ( !amd_iommu_perdev_intremap ) + printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not recommended (see XSA-36)!\n"); return scan_pci_devices(); } --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -52,7 +52,7 @@ bool_t __read_mostly iommu_qinval = 1; bool_t __read_mostly iommu_intremap = 1; bool_t __read_mostly iommu_hap_pt_share = 1; bool_t __read_mostly iommu_debug; -bool_t __read_mostly amd_iommu_perdev_intremap; +bool_t __read_mostly amd_iommu_perdev_intremap = 1; DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb); --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h @@ -100,6 +100,7 @@ void amd_iommu_read_msi_from_ire( extern struct ioapic_sbdf { u16 bdf, seg; + unsigned long *pin_setup; } ioapic_sbdf[MAX_IO_APICS]; extern void *shared_intremap_table;