acpi: fix early DSDT dmi check warnings on ia64

PATCH Applied Lin Ming 4 participants 10 messages Apr 29, 2010 – May 20, 2010 ACPI

lore

Share & tools

lore

Lin Ming

author

2010-04-29 01:41 UTC #0 lore raw

PATCH 1/1 acpi: fix early DSDT dmi check warnings on ia64

From: Lin Ming <[email protected]>
Subject: [PATCH] acpi: fix early DSDT dmi check warnings on ia64

Tony Luck saw a lot of warning messages on ia64:

WARNING: at drivers/firmware/dmi_scan.c:423 dmi_matches+0x70/0x160()

dmi check: not initialized yet.

This is caused by commit aa2110c(ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT). DMI is not initialized yet in acpi_early_init.
This patch checks the availability of DMI to avoid the warnings.

Tested-by Tony Luck <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/bus.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

drivers/acpi/bus.c +3 -1

index 49af19b..047de07 100644
@@ -847,8 +847,10 @@ void __init acpi_early_init(void)
 	/*
 	 * If the machine falls into the DMI check table,
 	 * DSDT will be copied to memory
+	 * Only check x86, it's too early to check dmi for ia64
 	 */
-	dmi_check_system(dsdt_dmi_table);
+	if (dmi_available)
+		dmi_check_system(dsdt_dmi_table);
 
 	status = acpi_reallocate_root_table();
 	if (ACPI_FAILURE(status)) {

Bjorn Helgaas

2010-04-30 17:12 UTC #1 lore raw

Quoted 22 lines · Lin Ming

From: Lin Ming <[email protected]>
Subject: [PATCH] acpi: fix early DSDT dmi check warnings on ia64
Tony Luck saw a lot of warning messages on ia64:
WARNING: at drivers/firmware/dmi_scan.c:423 dmi_matches+0x70/0x160()
dmi check: not initialized yet.
This is caused by commit aa2110c(ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT). DMI is not initialized yet in acpi_early_init.
This patch checks the availability of DMI to avoid the warnings.
Tested-by Tony Luck <[email protected]>
Signed-off-by Lin Ming <[email protected]>
---
 drivers/acpi/bus.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/acpi/bus.c +0 -0
index 49af19b..047de07 100644

drivers/acpi/bus.c void __init acpi_early_init(void) · line 850

Quoted 4 lines

drivers/acpi/bus.c +1 -0

@@ -847,8 +847,10 @@ void __init acpi_early_init(void)
 	/*
 	 * If the machine falls into the DMI check table,
 	 * DSDT will be copied to memory
+	 * Only check x86, it's too early to check dmi for ia64

I can't remember why this is different between x86 and ia64. If it's reasonable to do, I think it'd be nicer to make DMI available earlier on ia64 so we don't have to add checks like this. Did you investigate that?

Bjorn

Quoted 14 lines

drivers/acpi/bus.c +2 -2
 	 */
-	dmi_check_system(dsdt_dmi_table);
+	if (dmi_available)
+		dmi_check_system(dsdt_dmi_table);
 
 	status = acpi_reallocate_root_table();
 	if (ACPI_FAILURE(status)) {
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Luck, Tony

2010-04-30 16:59 UTC #2 lore raw

Quoted 4 lines

I can't remember why this is different between x86 and ia64. If it's reasonable to do, I think it'd be nicer to make DMI available earlier on ia64 so we don't have to add checks like this. Did you investigate that?

There is not clue in the commit that added this to ia64 as to why dmi_scan_machine() was added as an initcall. I just tried to move it into setup_arch() - immediately after the efi_init() call as in x86 - and my system hung without printing anything in boot.

Agreed this would be a better fix. I'll see if I can figure out what this call depends on, and so whether I can move is a bit later, but still early enough to be available for Ming's code to work.

-Tony

Luck, Tony

2010-04-30 17:21 UTC #3 lore raw

In reply to Bjorn Helgaas

Quoted 8 lines

There is not clue in the commit that added this to ia64 as to why dmi_scan_machine() was added as an initcall. I just tried to move it into setup_arch() - immediately after the efi_init() call as in x86 - and my system hung without printing anything in boot.
Agreed this would be a better fix. I'll see if I can figure out what this call depends on, and so whether I can move is a bit later, but still early enough to be available for Ming's code to work.

At a random mid-point in setup_arch() I got a lot of errors from slub about not being able to allocate memory. At the very end of setup_arch() (after paging_init()) it still failed - with a deref NULL inside kmem_alloc().

So it seems to be non-trivial to just move dmi_scan_machine() earlier on ia64 :-(

-Tony

Lin Ming

author

2010-05-06 01:58 UTC #4 lore raw

Quoted 9 lines · Tony

Quoted 8 lines
There is not clue in the commit that added this to ia64 as to why dmi_scan_machine() was added as an initcall. I just tried to move it into setup_arch() - immediately after the efi_init() call as in x86 - and my system hung without printing anything in boot.
Agreed this would be a better fix. I'll see if I can figure out what this call depends on, and so whether I can move is a bit later, but still early enough to be available for Ming's code to work.

Quoted 7 lines

At a random mid-point in setup_arch() I got a lot of errors from slub about not being able to allocate memory. At the very end of setup_arch() (after paging_init()) it still failed - with a deref NULL inside kmem_alloc().
So it seems to be non-trivial to just move dmi_scan_machine() earlier on ia64 :-(

Sorry for late response, I'm just back from holiday.

Len,
Can you apply the patch since dmi_available check is needed for ia64?

Thanks,
Lin Ming

Lin Ming

author

2010-05-11 02:10 UTC #5 lore raw

Quoted 15 lines · Lin Ming

Quoted 14 lines · Tony
Quoted 8 lines
There is not clue in the commit that added this to ia64 as to why dmi_scan_machine() was added as an initcall. I just tried to move it into setup_arch() - immediately after the efi_init() call as in x86 - and my system hung without printing anything in boot.
Agreed this would be a better fix. I'll see if I can figure out what this call depends on, and so whether I can move is a bit later, but still early enough to be available for Ming's code to work.
At a random mid-point in setup_arch() I got a lot of errors from slub about not being able to allocate memory. At the very end of setup_arch() (after paging_init()) it still failed - with a deref NULL inside kmem_alloc().

Quoted 7 lines

Quoted 2 lines
So it seems to be non-trivial to just move dmi_scan_machine() earlier on ia64 :-(
Sorry for late response, I'm just back from holiday.
Len,
Can you apply the patch since dmi_available check is needed for ia64?

Hi, all

What do you think of below new fix?

Subject: [PATCH] acpi: fix early DSDT dmi check warnings on ia64

Tony Luck saw a lot of warning messages on ia64:

WARNING: at drivers/firmware/dmi_scan.c:423 dmi_matches+0x70/0x160()

dmi check: not initialized yet.

This is caused by commit aa2110c(ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT). DMI is not initialized yet in acpi_early_init on ia64.

The DSDT dmi check table is x86 specific, so make it empty on other archs. And this fixes the warnings on ia64.

Reported-by Tony Luck <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/bus.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

drivers/acpi/bus.c +8 -1

index c0113e6..6ff33a8 100644
@@ -77,6 +77,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
 	return 0;
 }
 
+#ifdef CONFIG_X86
 static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 	/*
 	 * Insyde BIOS on some TOSHIBA machines corrupt the DSDT.

@@ -97,8 +98,14 @@ static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Satellite L505D"),
 		},
-	}
+	},
+	{}
 };
+#else
+static struct dmi_system_id dsdt_dmi_table[] __initdata = {
+	{}
+};
+#endif
 
 /* --------------------------------------------------------------------------
                                 Device Management

Luck, Tony

Tested-by

2010-05-11 23:01 UTC #6 lore raw

+#ifdef CONFIG_X86
static struct dmi_system_id dsdt_dmi_table[] __initdata = {

This #ifdef needs to be a few lines earlier to avoid getting a warning:

drivers/acpi/bus.c:73: 'set_copy_dsdt' defined but not used

But otherwise this patch does work and fixes the problem.

Tested-by Tony Luck <[email protected]>

Insyde are now writing BIOS for ia64 systems (I have two in my lab). I hope they don't copy this bug from their laptop BIOS to my systems. If they do, we'd have to revisit this :-)

-Tony

Lin Ming

author

2010-05-12 01:27 UTC #7 lore raw

Quoted 7 lines · Tony

+#ifdef CONFIG_X86
static struct dmi_system_id dsdt_dmi_table[] __initdata = {
This #ifdef needs to be a few lines earlier to avoid getting a warning:
drivers/acpi/bus.c:73: 'set_copy_dsdt' defined but not used

Fixed it now. Thanks.

Here is new one,

Subject: [PATCH] acpi: fix early DSDT dmi check warnings on ia64

Tony Luck saw a lot of warning messages on ia64:

WARNING: at drivers/firmware/dmi_scan.c:423 dmi_matches+0x70/0x160()

dmi check: not initialized yet.

This is caused by commit aa2110c(ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT). DMI is not initialized yet in acpi_early_init on ia64.

The DSDT dmi check table is x86 specific, so make it empty on other archs. And this fixes the warnings on ia64.

Reported-and-tested-by Tony Luck <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/bus.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

drivers/acpi/bus.c +8 -1

index c0113e6..9042a85 100644
@@ -69,6 +69,7 @@ static struct dmi_system_id __cpuinitdata power_nocheck_dmi_table[] = {
 };
 
 
+#ifdef CONFIG_X86
 static int set_copy_dsdt(const struct dmi_system_id *id)
 {
 	printk(KERN_NOTICE "%s detected - "

@@ -97,8 +98,14 @@ static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Satellite L505D"),
 		},
-	}
+	},
+	{}
 };
+#else
+static struct dmi_system_id dsdt_dmi_table[] __initdata = {
+	{}
+};
+#endif
 
 /* --------------------------------------------------------------------------
                                 Device Management

Quoted 14 lines

But otherwise this patch does work and fixes the problem.
Tested-by Tony Luck <[email protected]>
Insyde are now writing BIOS for ia64 systems (I have two in my lab). I hope they don't copy this bug from their laptop BIOS to my systems. If they do, we'd have to revisit this :-)
-Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Len Brown

reviewer Applied

2010-05-20 03:56 UTC #8 lore raw

applied

thanks,
Len Brown, Intel Open Source Technology Center

Len Brown

reviewer

2010-05-01 01:44 UTC #9 lore raw

In reply to Lin Ming

x86 calls dmi_scan_machine() to initialize DMI
from setup_arch().

ia64 calls dmi_scan_machine() from core_initcall()

Maybe they should both call it from the same place?

Also, ia64 should not parse entries which are known at compile-time to be x86 specific

thanks,
Len Brown, Intel Open Source Technology Center