[Linaro-acpi] [PATCH v5 0/5] Provide better MADT subtable sanity checks

[Hanjun Guo]

On 10/12/2015 05:44 PM, Sudeep Holla wrote:
>
>
> On 12/10/15 08:04, Hanjun Guo wrote:
>> On 10/12/2015 11:58 AM, Pat Erley wrote:
>>> On 10/11/2015 08:49 PM, Hanjun Guo wrote:
>>>> On 10/12/2015 11:08 AM, Pat Erley wrote:
>>>>> On 10/05/2015 10:12 AM, Al Stone wrote:
>>>>>> On 10/05/2015 07:39 AM, Rafael J. Wysocki wrote:
>>>>>>> On Wednesday, September 30, 2015 10:10:16 AM Al Stone wrote:
>>>>>>>> On 09/30/2015 03:00 AM, Hanjun Guo wrote:
>>>>>>>>> On 2015/9/30 7:45, Al Stone wrote:
>>>>>>>>>> NB: this patch set is for use against the linux-pm bleeding edge
>>>>>>>>>> branch.
>>>>>>>>>>
>>>>>>>>
>>>>>>>> [snip...]
>>>>>>>>
>>>>>>>>>
>>>>>>>>> For this patch set,
>>>>>>>>>
>>>>>>>>> Reviewed-by: Hanjun Guo <hanjun.guo at linaro.org>
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Hanjun
>>>>>>>>
>>>>>>>> Thanks, Hanjun!
>>>>>>>
>>>>>>> Series applied, thanks!
>>>>>>>
>>>>>>> Rafael
>>>>>>>
>>>>>>
>>>>>> Thanks, Rafael!
>>>>>>
>>>>>
>>>>> Just decided to test out linux-next (to see the new nouveau cleanups).
>>>>> This change set prevents my Lenovo W510 from booting properly.
>>>>>
>>>>> Reverting: 7494b0 "ACPI: add in a bad_madt_entry() function to
>>>>> eventually replace the macro"
>>>>>
>>>>> Gets the system booting again.  I'm attaching my dmesg from the failed
>>>>> boot, who wants the acpidump?
>>>>
>>>> [    0.000000] ACPI: undefined version for either FADT 4.0 or MADT 1
>>>> [    0.000000] ACPI: Error parsing LAPIC address override entry
>>>> [    0.000000] ACPI: Invalid BIOS MADT, disabling ACPI
>>>>
>>>> Seems the MADT revision is not right, could you dump the ACPI MADT
>>>> (APIC) table and send it out? I will take a look :)
>>>>
>>>> Thanks
>>>> Hanjun
>>>
>>> Here ya go, enjoy.  Feel free to CC me on any patches that might fix it.
>>
>> Thanks! I think I had the right guess, the MADT revision is not right
>> for ACPI 4.0:
>>
>> [000h 0000   4]                    Signature : "APIC"    [Multiple APIC
>> Description Table (MADT)]
>> [004h 0004   4]                 Table Length : 000000BC
>> [008h 0008   1]                     *Revision : 01*
>>
>> I encountered such problem before because the table was just copied from
>> previous version, and without the update for table revision.
>>
>> I think we may need to ignore the table revision for x86, but restrict
>> it for ARM64, I'd like Al and Rafael's suggestion before I send out a
>> patch.
>>
>
> Instead of just removing the check completely on x86, IMO restrict it to
> some newer/later version of ACPI so you can still force vendors to fix
> their ACPI tables at-least in future.

I agree.

>
> It would be good to get such sanity check in the tools used to build
> those tables, but yes since such static tables can be built in many
> ways, its difficult to deal it in all those tools.

At least we can check that in the FWTS. :)

Thanks
Hanjun