[PATCH] acpi: madt: Fix ITS subtable length check

[Al Stone]

On 07/14/2016 09:29 AM, Jeffrey Hugo wrote:
> On 7/14/2016 12:50 AM, Alex Hung wrote:
>> On 2016-07-14 02:41 AM, Jeffrey Hugo wrote:
>>> The ACPI spec defines the MADT GIC ITS subtable length as 20 bytes.  The
>>> test is checking to see if the subtable is 16 bytes in length which
>>> causes
>>> an invalid test failure for MADT tables with GIC ITS subtables that
>>> follow
>>> the spec.
>>>
>>> Signed-off-by: Jeffrey Hugo <jhugo at codeaurora.org>
>>> ---
>>>   src/acpi/madt/madt.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
>>> index b65b89e..65d3962 100644
>>> --- a/src/acpi/madt/madt.c
>>> +++ b/src/acpi/madt/madt.c
>>> @@ -183,7 +183,7 @@ static struct acpi_madt_subtable_lengths
>>> spec_info[] = {
>>>           .madt_version = 3,
>>>           .num_types = 16,
>>>           .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE,
>>> -                 16, 16, 12, 80, 24, 24, 16, 16 }
>>> +                 16, 16, 12, 80, 24, 24, 16, 20 }
>>>       },
>>
>> Hi Jeffrey,
>>
>> Thanks a lot for pointing this out.
>>
>> I noticed the ITS's length is 16 in ACPI 6.0 and it is updated to 20 in
>> ACPI 6.1 (See revision history "1487 The Length of GIC ITS Structure is
>> wrong"). However, the madt's revision also changed from 3 to 4
> 
> Drat, I must have missed that.  Thanks for pointing it out.
> 
>>
>> 1. talk about to this change in this patch
>> 2. keep the original structure and add a new structure for revision 4, ex.
>> { /* for ACPI 6.0 */
>> .major_version = 6,
>> .minor_version = 0,
>> .madt_version = 3,
>> .num_types = 16,
>> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE,
>> 16, 16, 12, 80, 24, 24, 16, 16 }
>> },
>> { /* for ACPI 6.1 */
>> .major_version = 6,
>> .minor_version = 0,
>> .madt_version = 4,
>> .num_types = 16,
>> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE,
>> 16, 16, 12, 80, 24, 24, 16, 20 }
> 
> Off the top of my head, this appears to make sense, but I'll play around with it
> while we wait for Al to commment.
> 
>>
>> Hi Al,
>>
>> Will you be able to give some suggestions?
>>
>>
>>>       { /* terminator */
>>>           .major_version = 0,
>>>
>>
>>
> 
> 

Oh, ick.  So, if we look at the ACPI 6.0 spec, it does very clearly
say the ITS subtable has a length of 16.  But, if you add up the lengths
of all the fields, the length should be 20.  I would recommend we just
accept that 6.0 has a typo (which is part of why 6.0a came out, btw)
and incorporate this patch as is.  If we leave it at 16, it's going to
really mess up any traversals of the subtables.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------