All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Stone <al.stone@linaro.org>
To: lkp@lists.01.org
Subject: Re: [ACPI] 7494b07eba: Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 0
Date: Thu, 08 Oct 2015 10:36:40 -0600	[thread overview]
Message-ID: <56169B98.3070105@linaro.org> (raw)
In-Reply-To: <56165701.90402@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

On 10/08/2015 05:44 AM, Hanjun Guo wrote:
> On 10/08/2015 11:21 AM, kernel test robot wrote:
>> FYI, we noticed the below changes on
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> commit 7494b07ebaae2117629024369365f7be7adc16c3 ("ACPI: add in a
>> bad_madt_entry() function to eventually replace the macro")
>>
>> [    0.000000] ACPI: undefined MADT subtable type for FADT 4.0: 127 (length 12)
> 
> Seems that the MADT table contains reserved subtable type (0x7F),
> so this is traded as a wrong type in our patch.
> 
>> [    0.000000] ACPI: Error parsing LAPIC address override entry
> 
> This was called by early_acpi_parse_madt_lapic_addr_ovr() in
> arch/x86/kernel/acpi/boot.c, which is scanning MADT for the first
> time when booting, so it will fail the boot process when finding
> the reserved MADT subtable type.
> 
>> [    0.000000] ACPI: Invalid BIOS MADT, disabling ACPI
> 
> As the spec said in Table 5-46 (ACPI 6.0):
> 
> 0x10-0x7F Reserved. OSPM skips structures of the reserved type.
> 
> Should we just ignore those reserved type when scanning the MADT
> table? In the patch "ACPI: add in a bad_madt_entry() function to
> eventually replace the macro", we just trade it as wrong, that's
> why we failed to boot the system.
> 
> Thanks
> Hanjun

Arrgh.  This is why people get frustrated with ACPI.  The spec is
saying that those sub-table types are reserved -- implying they can
and probably will be used for something else in the future -- but
then vendors are shipping firmware that uses those reserved values,
and an OS *expects* them to be used, and there is *no* documentation
of it other than a kernel workaround.

So yet again, technically this MADT subtable *is* wrong, and someone
should slap the vendor for doing this.  But, the practical side of
this is that we now have to workaround what is now a known violation
of the spec.

The more ACPI allows this kind of nonsense, the less usable it will
become.

At a minimum, whoever is responsible for this firmware needs to make
sure the spec reflects what they are doing.  In the meantime, the
only option is what Hanjun suggests -- make this a warning and not a
failure.  I'll prepare a patch and attach it to a reply here in a few
minutes...


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone(a)linaro.org
-----------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: Al Stone <al.stone@linaro.org>
To: Hanjun Guo <hanjun.guo@linaro.org>,
	kernel test robot <ying.huang@linux.intel.com>
Cc: lkp@01.org, LKML <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: [lkp] [ACPI] 7494b07eba: Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 0
Date: Thu, 8 Oct 2015 10:36:40 -0600	[thread overview]
Message-ID: <56169B98.3070105@linaro.org> (raw)
In-Reply-To: <56165701.90402@linaro.org>

On 10/08/2015 05:44 AM, Hanjun Guo wrote:
> On 10/08/2015 11:21 AM, kernel test robot wrote:
>> FYI, we noticed the below changes on
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> commit 7494b07ebaae2117629024369365f7be7adc16c3 ("ACPI: add in a
>> bad_madt_entry() function to eventually replace the macro")
>>
>> [    0.000000] ACPI: undefined MADT subtable type for FADT 4.0: 127 (length 12)
> 
> Seems that the MADT table contains reserved subtable type (0x7F),
> so this is traded as a wrong type in our patch.
> 
>> [    0.000000] ACPI: Error parsing LAPIC address override entry
> 
> This was called by early_acpi_parse_madt_lapic_addr_ovr() in
> arch/x86/kernel/acpi/boot.c, which is scanning MADT for the first
> time when booting, so it will fail the boot process when finding
> the reserved MADT subtable type.
> 
>> [    0.000000] ACPI: Invalid BIOS MADT, disabling ACPI
> 
> As the spec said in Table 5-46 (ACPI 6.0):
> 
> 0x10-0x7F Reserved. OSPM skips structures of the reserved type.
> 
> Should we just ignore those reserved type when scanning the MADT
> table? In the patch "ACPI: add in a bad_madt_entry() function to
> eventually replace the macro", we just trade it as wrong, that's
> why we failed to boot the system.
> 
> Thanks
> Hanjun

Arrgh.  This is why people get frustrated with ACPI.  The spec is
saying that those sub-table types are reserved -- implying they can
and probably will be used for something else in the future -- but
then vendors are shipping firmware that uses those reserved values,
and an OS *expects* them to be used, and there is *no* documentation
of it other than a kernel workaround.

So yet again, technically this MADT subtable *is* wrong, and someone
should slap the vendor for doing this.  But, the practical side of
this is that we now have to workaround what is now a known violation
of the spec.

The more ACPI allows this kind of nonsense, the less usable it will
become.

At a minimum, whoever is responsible for this firmware needs to make
sure the spec reflects what they are doing.  In the meantime, the
only option is what Hanjun suggests -- make this a warning and not a
failure.  I'll prepare a patch and attach it to a reply here in a few
minutes...


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

  reply	other threads:[~2015-10-08 16:36 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-08  3:21 [ACPI] 7494b07eba: Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 0 kernel test robot
2015-10-08 11:44 ` Hanjun Guo
2015-10-08 11:44   ` [lkp] " Hanjun Guo
2015-10-08 16:36   ` Al Stone [this message]
2015-10-08 16:36     ` Al Stone
2015-10-08 20:37     ` Rafael J. Wysocki
2015-10-08 20:37       ` [lkp] " Rafael J. Wysocki
2015-10-08 20:41       ` Rafael J. Wysocki
2015-10-08 20:41         ` [lkp] " Rafael J. Wysocki
2015-10-08 20:32         ` Al Stone
2015-10-08 20:32           ` [lkp] " Al Stone
2015-10-08 22:50           ` Rafael J. Wysocki
2015-10-08 22:50             ` [lkp] " Rafael J. Wysocki
2015-10-08 23:05             ` Al Stone
2015-10-08 23:05               ` [lkp] " Al Stone
2015-10-09 21:02               ` Rafael J. Wysocki
2015-10-09 21:02                 ` [lkp] " Rafael J. Wysocki
2015-10-09 22:52                 ` Al Stone
2015-10-09 22:52                   ` [lkp] " Al Stone
2015-10-09 23:44                   ` Rafael J. Wysocki
2015-10-09 23:44                     ` [lkp] " Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56169B98.3070105@linaro.org \
    --to=al.stone@linaro.org \
    --cc=lkp@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.