From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 1/2] AMD IOMMU: also spot missing IO-APIC entries in IVRS table Date: Mon, 11 Feb 2013 10:49:34 +0000 Message-ID: <1360579774.29432.102.camel@zakaz.uk.xensource.com> References: <511262E302000078000BC738@nat28.tlf.novell.com> <511264B402000078000BC754@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <511264B402000078000BC754@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Boris Ostrovsky , Sherry Hurwitz , xen-devel List-Id: xen-devel@lists.xenproject.org On Wed, 2013-02-06 at 13:12 +0000, Jan Beulich wrote: > Apart from dealing duplicate conflicting entries, we also have to > handle firmware omitting IO-APIC entries in IVRS altogether. Not doing > so has resulted in c/s 26517:601139e2b0db to crash such systems during > boot (whereas with the change here the IOMMU gets disabled just as is > being done in the other cases, i.e. unless global tables are being > used). > > Debugging this issue has also pointed out that the debug log output is > pretty ugly to look at - consolidate the output, and add one extra > item for the IVHD special entries, so that future issues are easier > to analyze. > > Signed-off-by: Jan Beulich > Tested-by: Sander Eikelenboom Not really my area but the change looks reasonably straight forward and fixes a real error observed in the field, Acked-by: Ian Campbell > > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > @@ -354,9 +354,8 @@ static int __init parse_ivmd_block(const > base = start_addr & PAGE_MASK; > limit = (start_addr + mem_length - 1) & PAGE_MASK; > > - AMD_IOMMU_DEBUG("IVMD Block: Type %#x\n",ivmd_block->header.type); > - AMD_IOMMU_DEBUG(" Start_Addr_Phys %#lx\n", start_addr); > - AMD_IOMMU_DEBUG(" Mem_Length %#lx\n", mem_length); > + AMD_IOMMU_DEBUG("IVMD Block: type %#x phys %#lx len %#lx\n", > + ivmd_block->header.type, start_addr, mem_length); > > if ( ivmd_block->header.flags & ACPI_IVMD_EXCLUSION_RANGE ) > iw = ir = IOMMU_CONTROL_ENABLED; > @@ -551,8 +550,8 @@ static u16 __init parse_ivhd_device_alia > return 0; > } > > - AMD_IOMMU_DEBUG(" Dev_Id Range: %#x -> %#x\n", first_bdf, last_bdf); > - AMD_IOMMU_DEBUG(" Dev_Id Alias: %#x\n", alias_id); > + AMD_IOMMU_DEBUG(" Dev_Id Range: %#x -> %#x alias %#x\n", > + first_bdf, last_bdf, alias_id); > > for ( bdf = first_bdf; bdf <= last_bdf; bdf++ ) > add_ivrs_mapping_entry(bdf, alias_id, range->alias.header.data_setting, > @@ -654,6 +653,9 @@ static u16 __init parse_ivhd_device_spec > return 0; > } > > + AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle %#x\n", > + seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf), > + special->variety, special->handle); > add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu); > > switch ( special->variety ) > @@ -758,10 +760,9 @@ static int __init parse_ivhd_block(const > { > ivhd_device = (const void *)((const u8 *)ivhd_block + block_length); > > - AMD_IOMMU_DEBUG( "IVHD Device Entry:\n"); > - AMD_IOMMU_DEBUG( " Type %#x\n", ivhd_device->header.type); > - AMD_IOMMU_DEBUG( " Dev_Id %#x\n", ivhd_device->header.id); > - AMD_IOMMU_DEBUG( " Flags %#x\n", ivhd_device->header.data_setting); > + AMD_IOMMU_DEBUG("IVHD Device Entry: type %#x id %#x flags %#x\n", > + ivhd_device->header.type, ivhd_device->header.id, > + ivhd_device->header.data_setting); > > switch ( ivhd_device->header.type ) > { > @@ -890,6 +891,7 @@ static int __init parse_ivrs_table(struc > { > const struct acpi_ivrs_header *ivrs_block; > unsigned long length; > + unsigned int apic; > int error = 0; > > BUG_ON(!table); > @@ -903,11 +905,9 @@ static int __init parse_ivrs_table(struc > { > ivrs_block = (struct acpi_ivrs_header *)((u8 *)table + length); > > - AMD_IOMMU_DEBUG("IVRS Block:\n"); > - AMD_IOMMU_DEBUG(" Type %#x\n", ivrs_block->type); > - AMD_IOMMU_DEBUG(" Flags %#x\n", ivrs_block->flags); > - AMD_IOMMU_DEBUG(" Length %#x\n", ivrs_block->length); > - AMD_IOMMU_DEBUG(" Dev_Id %#x\n", ivrs_block->device_id); > + AMD_IOMMU_DEBUG("IVRS Block: type %#x flags %#x len %#x id %#x\n", > + ivrs_block->type, ivrs_block->flags, > + ivrs_block->length, ivrs_block->device_id); > > if ( table->length < (length + ivrs_block->length) ) > { > @@ -922,6 +922,29 @@ static int __init parse_ivrs_table(struc > length += ivrs_block->length; > } > > + /* Each IO-APIC must have been mentioned in the table. */ > + for ( apic = 0; !error && apic < nr_ioapics; ++apic ) > + { > + if ( !nr_ioapic_entries[apic] || > + ioapic_sbdf[IO_APIC_ID(apic)].pin_setup ) > + continue; > + > + printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n", > + IO_APIC_ID(apic)); > + if ( amd_iommu_perdev_intremap ) > + error = -ENXIO; > + else > + { > + ioapic_sbdf[IO_APIC_ID(apic)].pin_setup = xzalloc_array( > + unsigned long, BITS_TO_LONGS(nr_ioapic_entries[apic])); > + if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_setup ) > + { > + printk(XENLOG_ERR "IVHD Error: Out of memory\n"); > + error = -ENOMEM; > + } > + } > + } > + > return error; > } > > >