[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] AMD IOMMU: Support IOAPIC IDs larger than 128



>>> On 02.12.16 at 04:48, <Suravee.Suthikulpanit@xxxxxxx> wrote:
> On 12/01/2016 06:58 PM, Jan Beulich wrote:
>>>>> On 01.12.16 at 12:04, <suravee.suthikulpanit@xxxxxxx> wrote:
>>> Currently, the driver uses the APIC ID to index into the ioapic_sbdf array.
>>> The current MAX_IO_APICS is 128, which causes the driver initialization
>>> to fail on the system with IOAPIC ID >= 128.
>>>
>>> Instead, this patch adds APIC ID in the struct ioapic_sbdf,
>>> which is used to match the entry when searching through the array.
>>
>> Wouldn't it have been a lot simpler to just bump the array size to
>> 256? I'll comment on the rest of the patch anyway ...
> 
> Yes, it would, and that's what I originally tried. However, I was 
> thinking that it would be unnecessarily waste of space since that would 
> affect serveral structures i.e.
>      * mp_ioapic_routing[MAX_IO_APICS]
>      * mp_ioapics[MAX_IO_APICS]
>      * nr_ioapic_entries[MAX_IO_APICS]
>      * vector_map[MAX_IO_APICS]

I don't understand - these arrays aren't indexed by IO-APIC ID,
so why would they need to grow? Note that I specifically did not
suggest to bump MAX_IO_APICS, but only to ...

>      * ioapic_sbdf[MAX_IO_APICS]

... grow this one array (or alternatively index it the same way
the others are being indexed).

> If you think this is a reasonable change, I can simplify the patch. The 
> number 256 should be reasonable for x1APIC since it only supports 8-bit 
> APIC ID. However, this might be an issue later on with 32-bit APIC ID w/ 
> x2APIC.

How would such a wider ID be communicated? I don't see any
MADT sub-table structure other than type 1, which has an 8-bit
ID field.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.