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

["Pasi Kärkkäinen" <pasik@iki.fi>]

On Wed, Mar 24, 2010 at 05:02:10PM +0800, Weidong Han wrote:
> 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.
>

Thanks, I'll try it on friday, I'm away from the system until that.

-- Pasi

> 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
>>
>>   
>