Re: Xen 4.0.0-rc7 problem/hang with vt-d DMAR parsing

[Weidong Han <weidong.han@intel.com>]

Jan Beulich wrote:
>>>> "Cui, Dexuan" <dexuan.cui@intel.com> 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
>
>