Re: char/tpm: tpm_infineon no longer loaded for HP 2510p laptop

Previous thread: Re: [Bug 11219] KVM modules break emergency reboot by Avi Kivity on Monday, August 18, 2008 - 6:39 am. (2 messages)

Next thread: Re: [PATCH] binfmt_misc.c: avoid potential kernel stack overflow by Pavel Emelyanov on Monday, August 18, 2008 - 7:09 am. (5 messages)
From: Frans Pop
Date: Monday, August 18, 2008 - 6:40 am

While comparing the loaded modules for 2.6.26 and 2.6.27-rc3 for my HP 
2510p, I noticed that the tpm_infineon module and related modules no 
longer get loaded automatically.

The difference seems to be that 2.6.26 listed:
/lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0102* tpm_infineon
/lib/modules/2.6.26.2/modules.alias:alias pnp:dIFX0101* tpm_infineon

while 2.6.27 has:
/lib/modules/2.6.27-rc3/modules.alias:alias acpi*:IFX0101:* tpm_infineon
/lib/modules/2.6.27-rc3/modules.alias:alias pnp:dIFX0101* tpm_infineon

My system has:
$ grep IFX /sys/bus/pnp/devices/*/id
/sys/bus/pnp/devices/00:02/id:IFX0102

And if I modprobe the module manually the device really does seem to be 
supported:
pnp: the driver 'tpm_inf_pnp' has been registered
tpm_inf_pnp 00:02: Found C284 with ID IFX0102
tpm_inf_pnp 00:02: TPM found: config base 0x560, data base 0x570, chip 
version 0x000b, vendor id 0x15d1 (Infineon), product id 0x000b (SLB 9635 
TT 1.2)
tpm_inf_pnp 00:02: driver attached

Relevant bit of the kernel config:
CONFIG_TCG_TPM=m
CONFIG_TCG_TIS=m
# CONFIG_TCG_NSC is not set
# CONFIG_TCG_ATMEL is not set
CONFIG_TCG_INFINEON=m

I've taken a quick look at changes from 2.6.26 in drivers/char/tpm/ but 
did not see really anything there that could explain this difference in 
behavior.

Cheers,
FJP
--

From: Bjorn Helgaas
Date: Wednesday, August 20, 2008 - 8:56 am

drivers/char/tpm/tpm_infineon.c hasn't changed since v2.6.26.  I think
the problem is more likely related to commit 22454cb99fc39f2629a:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=22454c...

I don't know enough about modaliases to understand the point of this
patch, but I am suspicious of this part of it:

-               do_table(symval, sym->st_size,
-                        sizeof(struct pnp_device_id), "pnp",
-                        do_pnp_entry, mod);
+               do_pnp_device_entry(symval, sym->st_size, mod);

That suggests to me that where we used to generate an alias for every
PNP ID in the table:

    static const struct pnp_device_id tpm_pnp_tbl[] = {
        {"IFX0101", 0},
        {"IFX0102", 0},
        {"", 0}
    };

possibly the new code only does it for the first entry (IFX0101).

Apart from problem Frans points out, I'd like to understand the reason
for generating the "acpi*" aliases for PNP drivers.  I'd like to move
away from ACPI being visible to userland, and this file2alias.c change
makes it *more* visible.

Bjorn
--

From: Rafael J. Wysocki
Date: Thursday, August 21, 2008 - 5:40 am

Frans, could you verify if reverting commit 22454cb99fc39f2629a fixes the
problem for you?

Thanks,
Rafael
--

From: Kay Sievers
Date: Thursday, August 21, 2008 - 6:28 am

Yeah, we miss to loop over the list. This should fix it. Thanks!


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: pnp: fix "add acpi:* modalias entries"

With 22454cb99fc39f2629ad06a7eccb3df312f8830e we added only the
first entry of the device table. We need to loop over the whole
device list.

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---

 file2alias.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -344,14 +344,20 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
 				struct module *mod)
 {
 	const unsigned long id_size = sizeof(struct pnp_device_id);
-	const struct pnp_device_id *id = symval;
+	const unsigned int count = (size / id_size)-1;
+	const struct pnp_device_id *devs = symval;
+	unsigned int i;
 
 	device_id_check(mod->name, "pnp", size, id_size, symval);
 
-	buf_printf(&mod->dev_table_buf,
-		   "MODULE_ALIAS(\"pnp:d%s*\");\n", id->id);
-	buf_printf(&mod->dev_table_buf,
-		   "MODULE_ALIAS(\"acpi*:%s:*\");\n", id->id);
+	for (i = 0; i < count; i++) {
+		const char *id = (char *)devs[i].id;
+
+		buf_printf(&mod->dev_table_buf,
+			   "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
+		buf_printf(&mod->dev_table_buf,
+			   "MODULE_ALIAS(\"acpi*:%s:*\");\n", id);
+	}
 }
 
 /* looks like: "pnp:dD" for every device of the card */


--

From: Bjorn Helgaas
Date: Thursday, August 21, 2008 - 8:14 am

Thanks for the patch.

Can somebody elaborate on why we need to add "acpi*" aliases for all
PNP devices?  That broadens the kernel/user interface, so I'd like
to understand why we need it.

Bjorn

--

From: Kay Sievers
Date: Thursday, August 21, 2008 - 8:38 am

We already do ACPI module autoloading by MODALIAS for other things than
pnp. ACPI exports the pnp devices with modalias, but the modules do not
have a matching alias, this add them.

PNP has no MODALIAS support at all, and the current pnp-aliases would
not work for the standard modalias method, they would need to change
their format.

The plan is to replace the current pnp modprobe shell script hack in
udev, when ACPI devices load the right modules without any special
userspace mangling.

Thanks,
Kay

--

From: Bjorn Helgaas
Date: Thursday, August 21, 2008 - 9:31 am

Sorry, I should have prefaced my question with "I'm completely ignorant
of this modalias stuff."  Is there a "complete idiot's guide to modules
and udev"?  There's precious little in Documentation/ other than a bunch


pnp_bus_type has no .uevent method.  What if I added one?  Would that
help this situation?

It seems wrong for file2alias.c to take every PNP device (even if it's

Is this the shell hack you mean (from etc/udev/rules.d/80-drivers.rules
in udev-117)?

  SUBSYSTEM=="pnp", DRIVER!="?*", ENV{MODALIAS}!="?*", \
    RUN{ignore_error}+="/bin/sh -c '/sbin/modprobe -a $$(while read id; do echo pnp:d$$id; done < /sys$devpath/id)'"

I agree that's gross.  Could I fix this by implementing pnp_device_uevent()?

Bjorn
--

From: Frans Pop
Date: Thursday, August 21, 2008 - 1:30 pm

This patch works for me (tested on top of -rc4).

$ grep IFX /lib/modules/2.6.27-rc4/modules.alias
alias acpi*:IFX0102:* tpm_infineon
alias pnp:dIFX0102* tpm_infineon
alias acpi*:IFX0101:* tpm_infineon
alias pnp:dIFX0101* tpm_infineon

And the tpm modules are automatically loaded again.

Cheers,
FJP
--

Previous thread: Re: [Bug 11219] KVM modules break emergency reboot by Avi Kivity on Monday, August 18, 2008 - 6:39 am. (2 messages)

Next thread: Re: [PATCH] binfmt_misc.c: avoid potential kernel stack overflow by Pavel Emelyanov on Monday, August 18, 2008 - 7:09 am. (5 messages)