[PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Mark Cilissen]

On this board the ACPI RSDP structure points to both a RSDT and an XSDT,
but the XSDT points to a truncated FADT. This causes all sorts of trouble
and usually a complete failure to boot after the following error occurs:

  ACPI Error: Unsupported address space: 0x20 (*/hwregs-*)
  ACPI Error: AE_SUPPORT, Unable to initialize fixed events (*/evevent-*)
  ACPI: Unable to start ACPI Interpreter

This leaves the ACPI implementation in such a broken state that subsequent
kernel subsystem initialisations go wrong, resulting in among others
mismapped PCI memory, SATA and USB enumeration failures, and freezes.

As this is an older embedded platform that will likely never see any BIOS
updates to address this issue and its default shipping OS only complies to
ACPI 1.0, work around this by forcing `acpi=rsdt`. This patch, applied on
top of Linux 5.10.102, was confirmed on real hardware to fix the issue.

Signed-off-by: Mark Cilissen <mark@yotsuba.nl>
Cc: stable@vger.kernel.org
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
v2:
- Reduce DMI match count to 4 to not overflow dmi_system_id structure
Reported-by: kernel test robot <lkp@intel.com>
- Change board ident to correct name
- Fix small style issue
- Fix up subject as per Rafael's changes

As this patch is CC'd to stable, it seemed wiser to submit a V2 rather
than an additional fixup patch to process.
---
 arch/x86/kernel/acpi/boot.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 5b6d1a95776f..b47338cd579d 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1328,6 +1328,17 @@ static int __init disable_acpi_pci(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int __init disable_acpi_xsdt(const struct dmi_system_id *d)
+{
+	if (!acpi_force) {
+		pr_notice("%s detected: force use of acpi=rsdt\n", d->ident);
+		acpi_gbl_do_not_use_xsdt = TRUE;
+	} else {
+		pr_notice("Warning: DMI blacklist says broken, but acpi XSDT forced\n");
+	}
+	return 0;
+}
+
 static int __init dmi_disable_acpi(const struct dmi_system_id *d)
 {
 	if (!acpi_force) {
@@ -1451,6 +1462,19 @@ static const struct dmi_system_id acpi_dmi_table[] __initconst = {
 		     DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 360"),
 		     },
 	 },
+	/*
+	 * Boxes that need ACPI XSDT use disabled due to corrupted tables
+	 */
+	{
+	 .callback = disable_acpi_xsdt,
+	 .ident = "Advantech DAC-BJ01",
+	 .matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "NEC"),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "Bearlake CRB Board"),
+		     DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"),
+		     DMI_MATCH(DMI_BIOS_VERSION, "V1.12"),
+		     },
+	 },
 	{}
 };
 

base-commit: 038101e6b2cd5c55f888f85db42ea2ad3aecb4b6
-- 
2.28.0

Re: [PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Hans de Goede]

Hi,

On 3/2/22 05:08, Mark Cilissen wrote:
> On this board the ACPI RSDP structure points to both a RSDT and an XSDT,
> but the XSDT points to a truncated FADT. This causes all sorts of trouble
> and usually a complete failure to boot after the following error occurs:
> 
>   ACPI Error: Unsupported address space: 0x20 (*/hwregs-*)
>   ACPI Error: AE_SUPPORT, Unable to initialize fixed events (*/evevent-*)
>   ACPI: Unable to start ACPI Interpreter
> 
> This leaves the ACPI implementation in such a broken state that subsequent
> kernel subsystem initialisations go wrong, resulting in among others
> mismapped PCI memory, SATA and USB enumeration failures, and freezes.
> 
> As this is an older embedded platform that will likely never see any BIOS
> updates to address this issue and its default shipping OS only complies to
> ACPI 1.0, work around this by forcing `acpi=rsdt`. This patch, applied on
> top of Linux 5.10.102, was confirmed on real hardware to fix the issue.
> 
> Signed-off-by: Mark Cilissen <mark@yotsuba.nl>
> Cc: stable@vger.kernel.org
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
> v2:
> - Reduce DMI match count to 4 to not overflow dmi_system_id structure
> Reported-by: kernel test robot <lkp@intel.com>
> - Change board ident to correct name
> - Fix small style issue
> - Fix up subject as per Rafael's changes
> 
> As this patch is CC'd to stable, it seemed wiser to submit a V2 rather
> than an additional fixup patch to process.
> ---
>  arch/x86/kernel/acpi/boot.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 5b6d1a95776f..b47338cd579d 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -1328,6 +1328,17 @@ static int __init disable_acpi_pci(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> +static int __init disable_acpi_xsdt(const struct dmi_system_id *d)
> +{
> +	if (!acpi_force) {
> +		pr_notice("%s detected: force use of acpi=rsdt\n", d->ident);
> +		acpi_gbl_do_not_use_xsdt = TRUE;
> +	} else {
> +		pr_notice("Warning: DMI blacklist says broken, but acpi XSDT forced\n");
> +	}
> +	return 0;
> +}
> +
>  static int __init dmi_disable_acpi(const struct dmi_system_id *d)
>  {
>  	if (!acpi_force) {
> @@ -1451,6 +1462,19 @@ static const struct dmi_system_id acpi_dmi_table[] __initconst = {
>  		     DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 360"),
>  		     },
>  	 },
> +	/*
> +	 * Boxes that need ACPI XSDT use disabled due to corrupted tables
> +	 */
> +	{
> +	 .callback = disable_acpi_xsdt,
> +	 .ident = "Advantech DAC-BJ01",
> +	 .matches = {
> +		     DMI_MATCH(DMI_SYS_VENDOR, "NEC"),
> +		     DMI_MATCH(DMI_PRODUCT_NAME, "Bearlake CRB Board"),
> +		     DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"),
> +		     DMI_MATCH(DMI_BIOS_VERSION, "V1.12"),
> +		     },
> +	 },

Heh, I should have noticed this new version before replying. I see that
you've dropped the BIOS-date match. But that actually is often more useful
then the BIOS_VERSION, sometimes vendors don't bump the version when
doing a new BIOS build.

If you only want to match the exact BIOS you tested against I would
drop the BIOS_VENDOR check instead.

Regards,

Hans



>  	{}
>  };
>  
> 
> base-commit: 038101e6b2cd5c55f888f85db42ea2ad3aecb4b6

Re: [PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Mark Cilissen]

> On 2 Mar 4 Reiwa, at 10:02, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> Hi,

Hi Hans,

> 
>> […]
> 
> Heh, I should have noticed this new version before replying. I see that
> you've dropped the BIOS-date match. But that actually is often more useful
> then the BIOS_VERSION, sometimes vendors don't bump the version when
> doing a new BIOS build.
> 
> If you only want to match the exact BIOS you tested against I would
> drop the BIOS_VENDOR check instead.

I am admittedly bit wary of dropping the BIOS_VENDOR check. As the cause of
this issue seems to be specifically a BIOS compilation error, it feels 
incomplete to leave this match out.

Since “CRB” in the DMI product name indicates the board design is derivative
of a generic Intel reference design (“Customer Reference Board”),
maybe it’s better to drop the SYS_VENDOR match instead?
It seems to bear little relation to the actual vendor (Advantech)
encountered in my testing hardware, anyway.

Let me know; if you still feel it’s better to drop the BIOS_VENDOR match,
I will do that instead.

> Regards,
> 
> Hans

Thanks and regards,

Mark

Re: [PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Hans de Goede]

Hi Mark,

On 3/2/22 21:20, Mark Cilissen wrote:
>> On 2 Mar 4 Reiwa, at 10:02, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
> 
> Hi Hans,
> 
>>
>>> […]
>>
>> Heh, I should have noticed this new version before replying. I see that
>> you've dropped the BIOS-date match. But that actually is often more useful
>> then the BIOS_VERSION, sometimes vendors don't bump the version when
>> doing a new BIOS build.
>>
>> If you only want to match the exact BIOS you tested against I would
>> drop the BIOS_VENDOR check instead.
> 
> I am admittedly bit wary of dropping the BIOS_VENDOR check. As the cause of
> this issue seems to be specifically a BIOS compilation error, it feels 
> incomplete to leave this match out.
> 
> Since “CRB” in the DMI product name indicates the board design is derivative
> of a generic Intel reference design (“Customer Reference Board”),
> maybe it’s better to drop the SYS_VENDOR match instead?
> It seems to bear little relation to the actual vendor (Advantech)
> encountered in my testing hardware, anyway.
> 
> Let me know; if you still feel it’s better to drop the BIOS_VENDOR match,
> I will do that instead.

I think that there are a lot more boards that will have
DMI_BIOS_VENDOR == "Phoenix Technologies LTD"
then that there are boards that will have
DMI_PRODUCT_NAME == "Bearlake CRB Board"

So if you want to make the DMI match as specific as possible then
IMHO dropping the bios-vendor match is best.

Regards,

Hans

Re: [PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Mark Cilissen]

> On 3 Mar 4 Reiwa, at 14:38, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> Hi Mark,

Hi Hans,

> On 3/2/22 21:20, Mark Cilissen wrote:
>>> 
> 
> I think that there are a lot more boards that will have
> DMI_BIOS_VENDOR == "Phoenix Technologies LTD"
> then that there are boards that will have
> DMI_PRODUCT_NAME == "Bearlake CRB Board"
> 
> So if you want to make the DMI match as specific as possible then
> IMHO dropping the bios-vendor match is best.

Of course, but just to clarify -- my proposal above is to drop

> DMI_MATCH(DMI_SYS_VENDOR, "NEC”),

not

> DMI_MATCH(DMI_PRODUCT_NAME, "Bearlake CRB Board”),

. :-)

Thanks and regards,
Mark

Re: [PATCH v2] ACPI / x86: Work around broken XSDT on Advantech DAC-BJ01 board

[Hans de Goede]

Hi,

On 3/3/22 14:42, Mark Cilissen wrote:
> 
>> On 3 Mar 4 Reiwa, at 14:38, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Mark,
> 
> Hi Hans,
> 
>> On 3/2/22 21:20, Mark Cilissen wrote:
>>>>
>>
>> I think that there are a lot more boards that will have
>> DMI_BIOS_VENDOR == "Phoenix Technologies LTD"
>> then that there are boards that will have
>> DMI_PRODUCT_NAME == "Bearlake CRB Board"
>>
>> So if you want to make the DMI match as specific as possible then
>> IMHO dropping the bios-vendor match is best.
> 
> Of course, but just to clarify -- my proposal above is to drop
> 
>> DMI_MATCH(DMI_SYS_VENDOR, "NEC”),
> 
> not
> 
>> DMI_MATCH(DMI_PRODUCT_NAME, "Bearlake CRB Board”),
> 
> . :-)

Ah I see, I did indeed misunderstand that. Still NEC is not that
often seen as sys-vendor, so I still believe dropping the bios-vendor
match is best.

Regards,

Hans