Regression in module wmi since 2.6.32 (bisected to commit 1caab3c)

Since 2.6.32, the module wmi fails to load on my x86_64 system. The problem has been bisected to the following commit:

commit 1caab3c1a90be3aa4ec3599409d8fe044b077478

Author: Matthew Garrett <[email protected]>
Date:   Wed Nov 4 14:17:53 2009 -0500

wmi: Add support for module autoloading

WMI provides interface-specific GUIDs that are exported from modules as modalises, but the core currently generates no events to trigger module loading. This patch adds support for registering devices for each WMI GUID and generating the appropriate uevent.

Based heavily on a patch by Carlos Corbacho (<[email protected]>).

This bug has an unfortunate side effect. If the module is allowed to autoload, loading of firmware is affected and symptoms similar to those of Bugzilla No. 14844. I'm still waiting for the OP to report if this is his problem, or if he has a different issue. If I blacklist wmi, the system will boot.

If I modprobe wmi after the system is booted, the specific messages are as follows:

------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:477 sysfs_add_one+0xd6/0xee()
Hardware name: System Product Name
sysfs: cannot create duplicate filename
'/devices/virtual/wmi/05901221-D566-11D1-B2F0-00A0C9062910'
Modules linked in: wmi(+) binfmt_misc snd_pcm_oss snd_mixer_oss snd_seq

snd_seq_device nfs nfsd lockd nfs_acl auth_rpcgss sunrpc cpufreq_conservative cpufreq_userspace cpufreq_powersave powernow_k8 fuse xfs exportfs loop dm_mod ide_cd_mod cdrom ata_generic pata_amd snd_hda_codec_nvhdmi snd_hda_codec_via ide_pci_generic tuner_simple tuner_types tda9887 tda8290 arc4 snd_hda_intel ecb snd_hda_codec tuner tvp5150 b43 saa7115 msp3400 mac80211 ivtv i2c_algo_bit ir_kbd_i2c snd_hwdep snd_pcm cfg80211 cx2341x snd_timer em28xx v4l2_common videobuf_vmalloc videodev led_class v4l1_compat snd amd74xx usbhid videobuf_core v4l2_compat_ioctl32 usblp video rtc_cmos soundcore hid tveeprom output ide_core serio_raw shpchp ssb button k8temp pcspkr rtc_core snd_page_alloc floppy forcedeth sg i2c_core rtc_lib pci_hotplug ohci_hcd ehci_hcd sd_mod crc_t10dif usbcore edd ahci libata scsi_mod ext3 mbcache jbd fan thermal processor thermal_sys hwmon
Pid: 3928, comm: modprobe Not tainted 2.6.33-rc2 #54

Call Trace:
 [<ffffffff81136757>] ? sysfs_add_one+0xd6/0xee
 [<ffffffff810405a1>] warn_slowpath_common+0x77/0xa4
 [<ffffffff8104061b>] warn_slowpath_fmt+0x3c/0x3e
 [<ffffffff81136757>] sysfs_add_one+0xd6/0xee
 [<ffffffff81136d1f>] create_dir+0x58/0x93
 [<ffffffff81136d92>] sysfs_create_dir+0x38/0x4f
 [<ffffffff811855db>] ? kobject_get+0x1a/0x22
 [<ffffffff81185722>] kobject_add_internal+0xd6/0x196
 [<ffffffff811858ba>] kobject_add_varg+0x41/0x4e
 [<ffffffff8118d85d>] ? kvasprintf+0x45/0x6e
 [<ffffffff81185982>] kobject_add+0x64/0x66
 [<ffffffff81213593>] ? get_device_parent+0xac/0x14b
 [<ffffffff81214586>] device_add+0xc9/0x539
 [<ffffffff811853ab>] ? kobject_init+0x43/0x83
 [<ffffffff81214a0f>] device_register+0x19/0x1d
 [<ffffffffa02042ca>] acpi_wmi_init+0x12b/0x19b [wmi]
 [<ffffffff8105d3ff>] ? up_read+0x9/0xb
 [<ffffffffa020419f>] ? acpi_wmi_init+0x0/0x19b [wmi]
 [<ffffffff810001f0>] do_one_initcall+0x5a/0x14a
 [<ffffffff810704c9>] sys_init_module+0xd0/0x229
 [<ffffffff8100296b>] system_call_fastpath+0x16/0x1b
---[ end trace 8700152a35cf1eac ]---
kobject_add_internal failed for 05901221-D566-11D1-B2F0-00A0C9062910 with
-EEXIST, don't try to register things with the same name in the same directory.
Pid: 3928, comm: modprobe Tainted: G        W  2.6.33-rc2 #54
Call Trace:
 [<ffffffff811857c9>] kobject_add_internal+0x17d/0x196
 [<ffffffff811858ba>] kobject_add_varg+0x41/0x4e
 [<ffffffff8118d85d>] ? kvasprintf+0x45/0x6e
 [<ffffffff81185982>] kobject_add+0x64/0x66
 [<ffffffff81213593>] ? get_device_parent+0xac/0x14b
 [<ffffffff81214586>] device_add+0xc9/0x539
 [<ffffffff811853ab>] ? kobject_init+0x43/0x83
 [<ffffffff81214a0f>] device_register+0x19/0x1d
 [<ffffffffa02042ca>] acpi_wmi_init+0x12b/0x19b [wmi]
 [<ffffffff8105d3ff>] ? up_read+0x9/0xb
 [<ffffffffa020419f>] ? acpi_wmi_init+0x0/0x19b [wmi]
 [<ffffffff810001f0>] do_one_initcall+0x5a/0x14a
 [<ffffffff810704c9>] sys_init_module+0xd0/0x229
 [<ffffffff8100296b>] system_call_fastpath+0x16/0x1b
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
IP: [<ffffffff81219e37>] device_pm_remove+0x34/0x52
PGD 11c47e067 PUD 114eca067 PMD 0
Oops: 0002 [#1] SMP

last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map CPU 1
Pid: 3928, comm: modprobe Tainted: G W 2.6.33-rc2 #54 M3N78-VM/System
Product Name

RIP: 0010:[<ffffffff81219e37>]  [<ffffffff81219e37>] device_pm_remove+0x34/0x52
RSP: 0018:ffff88011c071e28  EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff880114e4f800 RCX: ffff880114e4f8a0
RDX: 0000000000000000 RSI: ffffffffa01fbb8c RDI: ffffffff8178f490
RBP: ffff88011c071e38 R08: ffff88011c071e18 R09: ffff88011c071d88
R10: ffff88011c071e68 R11: ffff88011c071e88 R12: ffff88011c342640
R13: 0000000000000000 R14: 00000000ffffffef R15: 0000000000000200
FS:  00007f279c8c16f0(0000) GS:ffff880028280000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000008 CR3: 00000001166ab000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process modprobe (pid: 3928, threadinfo ffff88011c070000, task ffff88011adf0340)
Stack:
 ffff880114e4fa10 ffff880114e4f800 ffff88011c071e68 ffffffff812142b2
<0> ffff88011c071e68 ffff880114e4f800 ffff88011c342640 ffff88011c342640
<0> ffff88011c071e88 ffffffff8121442e ffff88011c071e88 ffff880114e4f800
Call Trace:
 [<ffffffff812142b2>] device_del+0x3c/0x1a7
 [<ffffffff8121442e>] device_unregister+0x11/0x1e
 [<ffffffffa01fb3ec>] wmi_class_exit+0x2c/0x51 [wmi]
 [<ffffffffa020430c>] acpi_wmi_init+0x16d/0x19b [wmi]
 [<ffffffff8105d3ff>] ? up_read+0x9/0xb
 [<ffffffffa020419f>] ? acpi_wmi_init+0x0/0x19b [wmi]
 [<ffffffff810001f0>] do_one_initcall+0x5a/0x14a
 [<ffffffff810704c9>] sys_init_module+0xd0/0x229
 [<ffffffff8100296b>] system_call_fastpath+0x16/0x1b
Code: c7 c7 90 f4 78 81 48 83 ec 08 e8 a2 f1 0a 00 48 8b 83 a8 00 00 00 48 8b 93

a0 00 00 00 48 8d 8b a0 00 00 00 48 c7 c7 90 f4 78 81 <48> 89 42 08 48 89 10 48 89 8b a8 00 00 00 48 89 8b a0 00 00 00
RIP [<ffffffff81219e37>] device_pm_remove+0x34/0x52
RSP <ffff88011c071e28>
CR2: 0000000000000008

---[ end trace 8700152a35cf1ead ]---

Larry

Interesting - what hardware is this? And can you post the DSDT?

-Carlos

E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

The hardware is home-built with an ASUS M3N78-VM motherboard and an Athlon X2 6000 3.1G CPU running an x86_64 OS.

I ran the following commands per the instructions at
http://www.lesswatts.org/projects/acpi/overridingDSDT.php with the following results:

=====================
desktop:~ # cp /proc/acpi/dsdt DSDT
desktop:~ # acpidump > acpidump.out
Wrong checksum for OEMB!
desktop:~ # acpixtract DSDT acpidump > DSDT.dat
desktop:~ # iasl -d DSDT.dat

Intel ACPI Component Architecture
AML Disassembler version 20081031 [Dec 3 2008]
Copyright (C) 2000 - 2008 Intel Corporation
Supports ACPI Specification Revision 3.0a

Loading Acpi table from file DSDT.dat
TableHeader length [0x62617420] greater than the input file size [0x35] Could not get table from the file
==================

Do the messages indicate a BIOS problem? I have not updated for some time, and would rather not do so unless necessary.

Should I attach the files DSDT.dat, acpidump.out, or DSDT?

Larry

DSDT from /proc/acpi/dsdt will do fine for my purposes.

-Carlos

E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Attached.

Thanks,

Larry

This is the same as bugzilla #14846 - in both cases, the GUID 05901221-D566-11D1-B2F0-00A0C9062910 (a data block query) is duplicated in two different devices, although both look like they are nVidia related hooks.

Since we don't currently have any in kernel drivers that use that stuff, and the query in question just returns a binary MOF, the simplest solution (for now) seems to be to just ignore these duplicates - if and when someone wants to support these hooks, they can clean the mess up properly when they figure out how WMI is supposed to cope with conflicting GUID's.

Matthew, you looked at NVIF for your sins, can you see a better solution? We could just blacklist the offending GUID, rather than trying to catch all duplicates? Although just ignoring all duplicates strikes me as a better solution to catch the next time that J Random BIOS Developer decides to duplicate GUIDs in WMI.

Initial patch below which takes the "ignore all duplicate GUIDs" approach.

-Carlos

E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
---
ACPI: WMI: Handle duplicate GUIDs

From: Carlos Corbacho <[email protected]>

It would appear that in BIOS's with nVidia hooks, the GUID
05901221-D566-11D1-B2F0-00A0C9062910 is duplicated. For now, the simplest
solution is to just ignore any duplicate GUIDs. These particular hooks are not
currently supported/ used in the kernel, so whoever does that can figure out
what the 'right' solution should be (if there's a better one).
---
 drivers/platform/x86/wmi.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

index 9f93d6c..483827f 100644
@@ -716,6 +716,22 @@ static int wmi_class_init(void)
 	return ret;
 }
 
+static bool guid_already_parsed(const char *guid_string) {
+	struct guid_block *gblock;
+	struct wmi_block *wblock;
+	struct list_head *p;
+
+	list_for_each(p, &wmi_blocks.list) {
+		wblock = list_entry(p, struct wmi_block, list);
+		gblock = &wblock->gblock;
+
+		if (strncmp(gblock->guid, guid_string, 16) != 0) {
+			return true;
+		}
+	}
+	return false;
+}
+
 /*
  * Parse the _WDG method for the GUID data blocks
  */

@@ -747,6 +763,16 @@ static __init acpi_status parse_wdg(acpi_handle handle)
 	memcpy(gblock, obj->buffer.pointer, obj->buffer.length);
 
 	for (i = 0; i < total; i++) {
+		/*
+		  Some WMI devices, like those for nVidia hooks, have a
+		  duplicate GUID. It's not clear what we should do in this
+		  case yet, so for now, we'll just ignore the duplicate.
+		  Anyone who wants to add support for that device can come
+		  up with a better workaround for the mess then.
+		*/
+		if (guid_already_parsed(gblock[i].guid) == true) {
+			continue;
+		}
 		wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
 		if (!wblock)
 			return AE_NO_MEMORY;

Hi Carlos,

Dmitry

The patch fixes my machine.

Thanks,

Larry

Can you test this instead? It should actually do the right thing.

-Carlos

E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
---
ACPI: WMI: Handle duplicate GUIDs

From: Carlos Corbacho <[email protected]>

It would appear that in BIOS's with nVidia hooks, the GUID
05901221-D566-11D1-B2F0-00A0C9062910 is duplicated. For now, the simplest
solution is to just ignore any duplicate GUIDs. These particular hooks are not
currently supported/ used in the kernel, so whoever does that can figure out
what the 'right' solution should be (if there's a better one).
---
 drivers/platform/x86/wmi.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

index 9f93d6c..845aaf6 100644
@@ -716,6 +716,22 @@ static int wmi_class_init(void)
 	return ret;
 }
 
+static bool guid_already_parsed(const char *guid_string) {
+	struct guid_block *gblock;
+	struct wmi_block *wblock;
+	struct list_head *p;
+
+	list_for_each(p, &wmi_blocks.list) {
+		wblock = list_entry(p, struct wmi_block, list);
+		gblock = &wblock->gblock;
+
+		if (strncmp(gblock->guid, guid_string, 16) == 0) {
+			return true;
+		}
+	}
+	return false;
+}
+
 /*
  * Parse the _WDG method for the GUID data blocks
  */

@@ -725,6 +741,7 @@ static __init acpi_status parse_wdg(acpi_handle handle)
 	union acpi_object *obj;
 	struct guid_block *gblock;
 	struct wmi_block *wblock;
+	char guid_string[37];
 	acpi_status status;
 	u32 i, total;
 

@@ -747,6 +764,19 @@ static __init acpi_status parse_wdg(acpi_handle handle)
 	memcpy(gblock, obj->buffer.pointer, obj->buffer.length);
 
 	for (i = 0; i < total; i++) {
+		/*
+		  Some WMI devices, like those for nVidia hooks, have a
+		  duplicate GUID. It's not clear what we should do in this
+		  case yet, so for now, we'll just ignore the duplicate.
+		  Anyone who wants to add support for that device can come
+		  up with a better workaround for the mess then.
+		*/
+		if (guid_already_parsed(gblock[i].guid) == true) {
+			wmi_gtoa(gblock[i].guid, guid_string);
+			printk(KERN_INFO PREFIX "Skipping duplicate GUID %s\n",
+				guid_string);
+			continue;
+		}
 		wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
 		if (!wblock)
 			return AE_NO_MEMORY;

This one worked, and I got the message that there was a duplicate in the logs.

Larry

It would appear that in BIOS's with nVidia hooks, the GUID 05901221-D566-11D1-B2F0-00A0C9062910 is duplicated. For now, the simplest solution is to just ignore any duplicate GUIDs. These particular hooks are not currently supported/ used in the kernel, so whoever does that can figure out what the 'right' solution should be (if there's a better one).

Fixes kernel bugzilla #14846

---
Len,

Can you get this out ASAP, as it fixes a regression with the WMI autoloading code.

 drivers/platform/x86/wmi.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

index 9f93d6c..845aaf6 100644
@@ -716,6 +716,22 @@ static int wmi_class_init(void)
 	return ret;
 }
 
+static bool guid_already_parsed(const char *guid_string) {
+	struct guid_block *gblock;
+	struct wmi_block *wblock;
+	struct list_head *p;
+
+	list_for_each(p, &wmi_blocks.list) {
+		wblock = list_entry(p, struct wmi_block, list);
+		gblock = &wblock->gblock;
+
+		if (strncmp(gblock->guid, guid_string, 16) == 0) {
+			return true;
+		}
+	}
+	return false;
+}
+
 /*
  * Parse the _WDG method for the GUID data blocks
  */

@@ -725,6 +741,7 @@ static __init acpi_status parse_wdg(acpi_handle handle)
 	union acpi_object *obj;
 	struct guid_block *gblock;
 	struct wmi_block *wblock;
+	char guid_string[37];
 	acpi_status status;
 	u32 i, total;
 

@@ -747,6 +764,19 @@ static __init acpi_status parse_wdg(acpi_handle handle)
 	memcpy(gblock, obj->buffer.pointer, obj->buffer.length);
 
 	for (i = 0; i < total; i++) {
+		/*
+		  Some WMI devices, like those for nVidia hooks, have a
+		  duplicate GUID. It's not clear what we should do in this
+		  case yet, so for now, we'll just ignore the duplicate.
+		  Anyone who wants to add support for that device can come
+		  up with a better workaround for the mess then.
+		*/
+		if (guid_already_parsed(gblock[i].guid) == true) {
+			wmi_gtoa(gblock[i].guid, guid_string);
+			printk(KERN_INFO PREFIX "Skipping duplicate GUID %s\n",
+				guid_string);
+			continue;
+		}
 		wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
 		if (!wblock)
 			return AE_NO_MEMORY;

--
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

applied to acpi-test, after satisfying checkpatch's brace-foo below.

thanks,
Len Brown, Intel Open Source Technology Center

ERROR: open brace '{' following function declarations go on the next line #34: FILE: drivers/platform/x86/wmi.c:719:
+static bool guid_already_parsed(const char *guid_string) {

WARNING: braces {} are not necessary for single statement blocks #43: FILE: drivers/platform/x86/wmi.c:728:
+ if (strncmp(gblock->guid, guid_string, 16) == 0) {
+ return true;
+ }

total: 1 errors, 1 warnings, 48 lines checked

Hm. I think this is an artifact of the way we're treating WMI right now. The assumption is that the interfaces are defined in namespace by their GUID, whereas in fact they're defined by their ACPI namespace. Can we avoid this by just setting the parent to the ACPI device's corresponding Linux device (if present) and including the WMI device's name in the dev_set_name call?

Matthew Garrett | [email protected]