From mboxrd@z Thu Jan 1 00:00:00 1970 From: Weidong Han Subject: Re: Xen 4.0.0-rc7 problem/hang with vt-d DMAR parsing Date: Wed, 24 Mar 2010 17:02:10 +0800 Message-ID: <4BA9D512.9090902@intel.com> References: <20100323193748.GW1878@reaktio.net> <20100323200515.GZ1878@reaktio.net> <4BA9DA400200007800036ABB@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4BA9DA400200007800036ABB@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" , Keir Fraser , "Cui, Dexuan" List-Id: xen-devel@lists.xenproject.org Jan Beulich wrote: >>>> "Cui, Dexuan" 24.03.10 02:52 >>> >>>> >> Pasi K?rkk?inen wrote: >> >>> Hmm.. wondering if the patch Jan just sent will help with that. >>> Sounds like it might help :) >>> >> I guess Jan's patch helps here in a very interesting way: >> > > I think reference was to a patch I sent yesterday, which I don't think > would help here (as the box would have to crash for it to help). > > >> I suspect your BIOS doesn't construct the DMAR properly, e.g., in acpi_parse_dmar(), entry_header->length is always 0, so xen'll hang in the while loop and continue printing the "dmaru->address = 0" message when iommu=verbose. >> > > Surely entry_header->length == 0 (or really > entry_header->length < sizeof(struct acpi_table_XXX)) should be > considered invalid, and hence get checked for? Linux at least has > a check against zero here... > yes, we need the check to solve Pasi's issue. I ported the patch from Linux, post below. Pasi, pls test the patch on your machine. it cannot check entry_header->length < sizeof(struct acpi_table_XXX), which is not the actual size in acpi table. diff -r a4eac162dcb9 xen/drivers/passthrough/vtd/dmar.c --- a/xen/drivers/passthrough/vtd/dmar.c Thu Mar 25 01:05:03 2010 +0800 +++ b/xen/drivers/passthrough/vtd/dmar.c Thu Mar 25 01:54:31 2010 +0800 @@ -659,6 +659,15 @@ static int __init acpi_parse_dmar(struct while ( ((unsigned long)entry_header) < (((unsigned long)dmar) + table->length) ) { + /* Avoid looping forever on bad ACPI tables */ + if ( entry_header->length == 0 ) + { + dprintk(XENLOG_WARNING VTDPREFIX, + "Invalid 0-length entry_header\n"); + ret = -EINVAL; + break; + } + switch ( entry_header->type ) { case ACPI_DMAR_DRHD: > >> Without verbose message outputing, the loop runs even faster and in acpi_parse_one_drhd(), xmalloc(struct acpi_drhd_unit) would NULL in a short periof of time and hence VT-d is got disabled... :-) >> > > Why would you expect xmalloc() to fail soon? This is only to be expected > on a 32-bit system (which I doubt this one is). > > Jan > >