From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH 1/2] AMD IOMMU: also spot missing IO-APIC entries in IVRS table Date: Wed, 06 Feb 2013 15:12:23 +0000 Message-ID: <511280E702000078000BC84A@nat28.tlf.novell.com> References: <511262E302000078000BC738@nat28.tlf.novell.com> <511264B402000078000BC754@nat28.tlf.novell.com> <51126BA3.9020307@oracle.com> <51127C2102000078000BC819@nat28.tlf.novell.com> <511270CA.6090903@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <511270CA.6090903@oracle.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: xen-devel , Sherry Hurwitz List-Id: xen-devel@lists.xenproject.org >>> On 06.02.13 at 16:03, Boris Ostrovsky wrote: > On 2/6/2013 9:52 AM, Jan Beulich wrote: >>>>> On 06.02.13 at 15:41, Boris Ostrovsky wrote: >>> On 2/6/2013 8:12 AM, Jan Beulich wrote: >>> >>>> >>>> + /* 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; >>>> } >>>> >>> >>> Don't we end up with ioapic_sbdf[IO_APIC_ID(apic)].bdf/seg being >>> uninitialized? They are usually set in parse_ivhd_device_special(), at >>> the same time pin_setup is allocated, but with IVRS broken in this way >>> we'll never get there, will we? >> >> Correct. .bdf/.seg being uninitialized is no much of a problem >> when using global intremap tables though. > > Since this patch has been tested it clearly must have worked somehow but > I don't understand where you'd get bdf and seg in > amd_iommu_ioapic_update_ire() and then manage to find the right IOMMU. The "right" IOMMU simply is the only one, and that is equally well found with sbdf being zero. Jan