linux-kernel - Re: [PATCH 5/6] tpm_tis: convert from pnp_driver to acpi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 10 Sep 2009 16:08:34 -0300
From:	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
To:	Andy Isaacson <adi@...apodia.org>
Cc:	len.brown@...el.com, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Andy Isaacson <adi@...are.com>, linux-kernel@...r.kernel.org,
	tpmdd-devel@...ts.sourceforge.net, dds@...gle.com,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Shahbaz Khan <shaz.linux@...il.com>, seiji.munetoh@...il.com
Subject: Re: [PATCH 5/6] tpm_tis: convert from pnp_driver to acpi_driver

On Mon, 2009-07-20 at 11:27 -0700, Andy Isaacson wrote:
> On Wed, Jul 01, 2009 at 10:45:19AM -0300, Rajiv Andrade wrote:
> > On Wed, 2009-07-01 at 11:01 +0100, Alan Cox wrote:
> > > On Tue, 30 Jun 2009 18:04:14 -0700
> > > Andy Isaacson <adi@...are.com> wrote:
> > > 
> > > > Not all TIS-compatible TPM chips have a _HID method in their ACPI entry,
> > > > and the TPM spec says that the _CID method should be used to enumerate
> > > > the TPM chip.
> > > 
> > > There are a number of systems with TPMs (older laptops) that don't work
> > > very well if you enable ACPI.
> > > 
> > > This is therefore a regression - NAK
> > > 
> > > Probably the best thing to do is to provide both ACPI and PnP
> > > registration according to what is configured into the kernel. (And I
> > > guess spot duplicates although the resource should be busy anyway)
> > > --
> > David sent this earlier when I said that PNP didn't work with this chip:
> >         
> > <quote>
> > The problem here is acpi pnp but the fix is really simple. The current
> > pnpacpi/core.c routine that looks for isapnp devices enumerated in acpi
> > enforces that the acpi hid be a valid isapnp id (the formats are
> > slightly different). But that's broken: it shoudl be enforcing that
> > either the acpi hid or any acpi cids be valid isapnp ids. It's a
> > one-line change to do this, see patch 2. 
> > 
> > commit 7a553b4e7439ad0733da7da8663d32aa4865aa9e
> > Author: David Smith <dds@...gle.com>
> > Date:   Tue Apr 28 18:52:02 2009 +0900
> > 
> >     Update ACPI PNP to support devices with EISA PNP CIDs but non-PNP HIDs
> >     
> >     Signed-off-by: David Smith <dds@...gle.com>
> > 
> > diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> > index 9496494..8bfddfb 100644
> > --- a/drivers/pnp/pnpacpi/core.c
> > +++ b/drivers/pnp/pnpacpi/core.c
> > @@ -159,8 +159,8 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
> >  	 * driver should not be loaded.
> >  	 */
> >  	status = acpi_get_handle(device->handle, "_CRS", &temp);
> > -	if (ACPI_FAILURE(status) || !ispnpidacpi(acpi_device_hid(device)) ||
> > -	    is_exclusive_device(device) || (!device->status.present))
> > +	if (ACPI_FAILURE(status) || is_exclusive_device(device) ||
> > +            (!device->status.present))
> >  		return 0;
> >  
> >  	dev = pnp_alloc_dev(&pnpacpi_protocol, num, acpi_device_hid(device));
> > 
> > </quote>
> 
> Len,
> 
> Is this an acceptable change to pnpacpi?  It resolves an issue with
> tpm_tis but I'm concerned that it might have far-reaching impact.
> 
> I've pasted in the problematic DSDT (manually fixing up whitespace to
> make it more readable), and then a normal TPM simply has a _HID which
> is matched by a pnp_device_id table in the driver
> (drivers/char/tpm/tpm_tis.c).
> 
> T400:
>         Device (TPM)
>         {
>             Method (_HID, 0, NotSerialized)
>             {
>                 TPHY (0x00)
>                 If (LEqual (TPMV, 0x01)) { Return (0x0201D824) } 
>                 If (LEqual (TPMV, 0x02)) { Return (0x0435CF4D) } 
>                 If (LEqual (TPMV, 0x03)) { Return (0x02016D08) } 
>                 If (LEqual (TPMV, 0x04)) { Return (0x01016D08) } 
>                 If (LOr (LEqual (TPMV, 0x05), LEqual (TPMV, 0x06))) {
> 			Return (0x0010A35C)
> 		} 
>                 If (LEqual (TPMV, 0x08)) { Return (0x00128D06) } 
>                 If (LEqual (TPMV, 0x09)) { Return ("INTC0102") } 
>                 Return (0x310CD041)
>             }
> 
>             Name (_CID, EisaId ("PNP0C31"))
> 
> standard TPM:
>         Device (TPM)
>         {
>             Name (_HID, EisaId ("BCM0102"))
>             Name (_CID, EisaId ("PNP0C31"))
> 
> The full thread is at
> http://lkml.org/lkml/2009/7/1/265
> 
> Thanks for any insight.
> 

We've already waited too much on this, is it acceptable to make the
workaround depend on (and only on) the module parameter you've set in
patch 6/6? Therefore no need to check the vendor ID.

<snip>
+MODULE_PARM_DESC(itpm, "Force iTPM workarounds (found on some Lenovo laptops)"); 
</snip>

It already mentions _Force_, which in many cases maps to "it's all your
responsibility"...

And yes, still without PNP, but at least, working.

Rajiv



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/