Fwd: [tpmdd-devel] TPM drivers support and Linux Integrity Module for 2.6.30

[dds (â)]

Hi Rajiv. I'm resending this since it is not in the archives (my bad).


---------- Forwarded message ----------
From: dds (â) <dds@xxxxxxxxxx>
Date: Sun, Jun 14, 2009 at 4:15 PM
Subject: Re: [tpmdd-devel] TPM drivers support and Linux Integrity
Module for 2.6.30
To: Rajiv Andrade <srajiv@xxxxxxxxxxxxxxxxxx>
Cc: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>,
tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx, linux-kernel
<linux-kernel@xxxxxxxxxxxxxxx>, Shahbaz Khan <shaz.linux@xxxxxxxxx>


Hello, I'd been meaning to write about this.


On Sun, Jun 14, 2009 at 12:55 PM, Rajiv Andrade
<srajiv@xxxxxxxxxxxxxxxxxx> wrote:
>
> Hi Mimi, thanks for copying us.
>
> Shaz,
>
> If this is the same chip we find in the GM45 boards, iTPM, the upstream
> driver won't work properly with it.
> Mainly because this iTPM returns the wrong status code when the driver
> didn't finish sending all bytes required for a specific command.
> As suggested by Seiji Munetoh in the tpmdd-devel sf mailing list, you
> can modify line 263 of tpm_tis.c as below:
>
> - Â Â Â Â Â Â Â if ((status & TPM_STS_DATA_EXPECT) == 0) {
> + Â Â Â Â Â Â Â if ((status & TPM_STS_VALID) == 0) {
>

This isn't unreasonable. In the block that should be executing there,
it's proper to check both, since VALID is an override for DATA_EXPECT.
See first patch.

>
> Then, after compiling it, since it also seems to not support PNP, load
> it with force option on:
>
> modprobe tpm_tis force=1

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.


>
> If modprobe fails the first time, try the second and then it will work.

This is fixed by changing the order in the code of setting default
timeouts and requesting locality. See patch 3.

>
> I'm going to submit a patch to make the upstream driver work with it,
> making this line depend on a module param too..
>
> Thanks,
> Rajiv
>
> Mimi Zohar wrote:
> > On Fri, 2009-06-12 at 11:59 +0600, Shahbaz Khan wrote:
> >
> >> Hi,
> >>
> >> I am using Intel Q45 Express chipset with TPM version 1.2 specs of
> >> TCG. The kernel version is 2.6.30. Problem is that the TPM drivers
> >> cannot provide functionality to the TCG TSS giving error message:
> >>
> >> "TCSD TDDL ERROR: Could not find a device to open!"
> >>
> >> The device node in /dev is also not being created which should be
> >> "/dev/tpm". If created manually then still it does not work.
> >>
> >> What should be done?
> >>
> >> Thanks.
> >>
> >> --
> >> Shaz
> >>
> >
> > This is a device driver issue. Copying the TPM maintainers and the
> > forum.
> >
> > Mimi
> >
> >
>
>
> ------------------------------------------------------------------------------
> Crystal Reports - New Free Runtime and 30 Day Trial
> Check out the new simplified licensing option that enables unlimited
> royalty-free distribution of the report engine for externally facing
> server and web deployment.
> http://p.sf.net/sfu/businessobjects
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
commit cca56d7b550bac0a00d6322b225f4d0a8d3e6b88
Author: David Smith <dds@xxxxxxxxxx>
Date:   Tue Apr 28 18:56:39 2009 +0900

    Fix tpm_tis driver to support either DATA_EXPECT or VALID status when uploading command data.
    
    The TCG spec says that a VALID status implies that a DATA_EXPECT
    status. This occurs in the real world with the iTPM in Intel's Mobile 4
    platform which never sets DATA_EXPECT, but sets VALID when expecting more
    data.
    
    Signed-off-by: David Smith <dds@xxxxxxxxxx>

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index aec1931..be112ef 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -293,7 +293,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 		wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c,
 			      &chip->vendor.int_queue);
 		status = tpm_tis_status(chip);
-		if ((status & TPM_STS_DATA_EXPECT) == 0) {
+		if ((status & TPM_STS_DATA_EXPECT) == 0 &&
+                    (status & TPM_STS_VALID) == 0) {
 			rc = -EIO;
 			goto out_err;
 		}
@@ -306,7 +307,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c,
 		      &chip->vendor.int_queue);
 	status = tpm_tis_status(chip);
-	if ((status & TPM_STS_DATA_EXPECT) != 0) {
+	if ((status & TPM_STS_DATA_EXPECT) != 0 &&
+            (status & TPM_STS_VALID) == 1) {
 		rc = -EIO;
 		goto out_err;
 	}
commit 7a553b4e7439ad0733da7da8663d32aa4865aa9e
Author: David Smith <dds@xxxxxxxxxx>
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@xxxxxxxxxx>

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));
commit 2117a060d04b1063f26bae6450bdd21be400b799
Author: David Smith <dds@xxxxxxxxxx>
Date:   Thu Jun 11 08:34:16 2009 +0900

    Reorder setting chip timeouts to before locality is requested.
    This stops a failure to load roughly half the time of the module.
    
    Signed-off-by: David Smith <dds@xxxxxxxxxx>

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index be112ef..eea5d4c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -452,6 +452,12 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 		goto out_err;
 	}
 
+	/* Default timeouts */
+	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+	chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
+	chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+	chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+
 	if (request_locality(chip, 0) != 0) {
 		rc = -ENODEV;
 		goto out_err;
@@ -459,12 +465,6 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 
 	vendor = ioread32(chip->vendor.iobase + TPM_DID_VID(0));
 
-	/* Default timeouts */
-	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
-	chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-
 	dev_info(dev,
 		 "1.2 TPM (device-id 0x%X, rev-id %d)\n",
 		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));