Re: [Xen-devel] [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

Tuesday, August 27, 2013, 6:30:32 PM, you wrote:


> Tuesday, August 27, 2013, 6:21:02 PM, you wrote:

>>>>> On 27.08.13 at 18:14, Sander Eikelenboom <linux@xxxxxxxxxxxxxx> wrote:
>>> Tuesday, August 27, 2013, 5:59:28 PM, you wrote:
>>> 
>>>>>>> On 27.08.13 at 16:15, Sander Eikelenboom <linux@xxxxxxxxxxxxxx> wrote:
>>>>> Where as your implementation seems to only walk the list present from 
>>>>> bios 
>>>>> and override a specific entry when it's mapping them in 
>>>>> parse_ivhd_special ?
>>> 
>>>> No, the code change to parse_ivrs_table() should be setting up
>>>> necessary information.
>>> 
>>> Well i just checked, and in parse_ivhd_device_special() doing a:
>>> 
>>> printk(XENLOG_ERR "is apic 0x6 override parsed ? %d ",  test_bit(0x6, 
>>> ioapic_cmdline ));
>>> 
>>> returns 1 ... so the parsing part seems to be OK.
>>> 
>>> but parse_ivhd_device_special() isn't called for ioapic 0x6, because there 
>>> is no entry for it in the bios provided tables in my case.
>>> and since that seems to be what is iterated ..

>> As said, the handling of missing entries should be done by the tail
>> of parse_ivrs_table().

> Ah whoops, it does the setup as well .. of course my printk's in 
> parse_ivhd_device_special() don't show anything then ...




>     /* Each IO-APIC must have been mentioned in the table. */
>     for ( apic = 0; !error && iommu_intremap && apic < nr_ioapics; ++apic )
>     {
>         if ( !nr_ioapic_entries[apic] ||
>              ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
>             continue;

>         if ( !test_bit(IO_APIC_ID(apic), ioapic_cmdline) )
>         {
>             printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n",
>                    IO_APIC_ID(apic));
>             if ( amd_iommu_perdev_intremap )
>                 return -ENXIO;
>         }

> wouldn't an extra printk mentioning the addition of an entry from cmdline be 
> nice here ?


>         ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
>             u16, nr_ioapic_entries[apic]);
>         if ( ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
>             memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
>                    nr_ioapic_entries[apic] * sizeof(*ioapic_sbdf->pin_2_idx));
>         else
>         {
>             printk(XENLOG_ERR "IVHD Error: Out of memory\n");
>             error = -ENOMEM;
>         }
>     }


>> Jan

Hi Jan,

I just added a printk there and all seems to work fine .. sorry for the noise

so apart from perhaps adding some verbosity when actually applying the vodoo ...

at least for the IOAPIC part:

Tested-By: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>


--
Sander






_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel