Skip to contentSkip to conversation

tpm_tis: Use timeouts returned from TPM

PATCH In review Stefan Berger 2 participants 5 messages Feb 22, 2011 – Feb 26, 2011 TPM device driver

lore
Share & tools
lore

Stefan Berger

author
#0 lore raw

PATCH v2 1/1 tpm_tis: Use timeouts returned from TPM

Below patch applies to the tip of the git tree.

v2:
- adjusting all timeouts for TPM devices reporting too low timeout
numbers
- also displaying in sysfs whether the timeouts are 'original' or
'adjusted'

The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.

Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.

I am also adding a sysfs entry 'timeouts' showing the timeouts that are being used.

Signed-off-by Stefan Berger <[email protected]>
---
 drivers/char/tpm/tpm.c     |   35 ++++++++++++++++++++++++++---------
 drivers/char/tpm/tpm.h     |    2 ++
 drivers/char/tpm/tpm_tis.c |    4 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)
drivers/char/tpm/tpm.c +31 -9
Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -577,23 +577,30 @@ duration:
 	if (rc)
 		return;
 
-	if (be32_to_cpu(tpm_cmd.header.out.return_code)
-	    != 3 * sizeof(u32))
+	if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
+	    be32_to_cpu(tpm_cmd.header.out.length)
+	    != sizeof(tpm_cmd.header.out) + sizeof(u32) + 3 * sizeof(u32))
 		return;
+
 	duration_cap = &tpm_cmd.params.getcap_out.cap.duration;
 	chip->vendor.duration[TPM_SHORT] =
 	    usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_short));
-	/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
-	 * value wrong and apparently reports msecs rather than usecs. So we
-	 * fix up the resulting too-small TPM_SHORT value to make things work.
-	 */
-	if (chip->vendor.duration[TPM_SHORT] < (HZ/100))
-		chip->vendor.duration[TPM_SHORT] = HZ;
-
 	chip->vendor.duration[TPM_MEDIUM] =
 	    usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_medium));
 	chip->vendor.duration[TPM_LONG] =
 	    usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_long));
+
+	/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
+	 * value wrong and apparently reports msecs rather than usecs. So we
+	 * fix up the resulting too-small TPM_SHORT value to make things work.
+	 */
+	if (chip->vendor.duration[TPM_SHORT] < (HZ/100)) {
+		chip->vendor.duration[TPM_SHORT]   = HZ;
+		chip->vendor.duration[TPM_MEDIUM] *= 1000;
+		chip->vendor.duration[TPM_LONG]   *= 1000;
+		chip->vendor.duration_adjusted = true;
+		dev_info(chip->dev, "Adjusting TPM timeout parameters.");
+	}
 }
 EXPORT_SYMBOL_GPL(tpm_get_timeouts);
 

@@ -939,6 +946,21 @@ ssize_t tpm_show_caps_1_2(struct device 
 }
 EXPORT_SYMBOL_GPL(tpm_show_caps_1_2);
 
+ssize_t tpm_show_timeouts(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d %d %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
+		       chip->vendor.duration_adjusted
+			 ? "adjusted"
+			 : "original");
+}
+EXPORT_SYMBOL_GPL(tpm_show_timeouts);
+
 ssize_t tpm_store_cancel(struct device *dev, struct device_attribute *attr,
 			const char *buf, size_t count)
 {
drivers/char/tpm/tpm.h +3 -0
Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
@@ -56,6 +56,8 @@ extern ssize_t tpm_show_owned(struct dev
 				char *);
 extern ssize_t tpm_show_temp_deactivated(struct device *,
 					 struct device_attribute *attr, char *);
+extern ssize_t tpm_show_timeouts(struct device *,
+				 struct device_attribute *attr, char *);
 
 struct tpm_chip;
 

@@ -82,6 +84,7 @@ struct tpm_vendor_specific {
 	int locality;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* jiffies */
 	unsigned long duration[3]; /* jiffies */
+	bool duration_adjusted;
 
 	wait_queue_head_t read_queue;
 	wait_queue_head_t int_queue;
drivers/char/tpm/tpm_tis.c +3 -1
Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -376,6 +376,7 @@ static DEVICE_ATTR(temp_deactivated, S_I
 		   NULL);
 static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps_1_2, NULL);
 static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
+static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
 
 static struct attribute *tis_attrs[] = {
 	&dev_attr_pubek.attr,

@@ -385,7 +386,8 @@ static struct attribute *tis_attrs[] = {
 	&dev_attr_owned.attr,
 	&dev_attr_temp_deactivated.attr,
 	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr, NULL,
+	&dev_attr_cancel.attr,
+	&dev_attr_timeouts.attr, NULL,
 };
 
 static struct attribute_group tis_attr_grp = {
#1 lore raw
Quoted 14 lines · Stefan Berger

Below patch applies to the tip of the git tree.

v2:
- adjusting all timeouts for TPM devices reporting too low timeout
numbers
- also displaying in sysfs whether the timeouts are 'original' or
'adjusted'

The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.

Quoted 6 lines

Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.

I am also adding a sysfs entry 'timeouts' showing the timeouts that are being used.

It works:

# dmesg
[15318.813905] tpm_tis 00:0c: 1.2 TPM (device-id 0xB, rev-id 16)
[15318.879154] tpm_tis 00:0c: Adjusting TPM timeout parameters.
# cat /sys/devices/pnp0/00\:0*/timeouts
1000000 2000000 150000000 [adjusted]

thanks,

Signature · 1 line
js
author
#2 lore raw
Quoted 15 lines · Jiri Slaby
Quoted 14 lines · Stefan Berger

Below patch applies to the tip of the git tree.

v2:
- adjusting all timeouts for TPM devices reporting too low timeout
numbers
- also displaying in sysfs whether the timeouts are 'original' or
'adjusted'

The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.

Quoted 7 lines
Quoted 6 lines

Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.

I am also adding a sysfs entry 'timeouts' showing the timeouts that are being used.

It works:

'It' means also 'suspend' works?

Quoted 6 lines
# dmesg
[15318.813905] tpm_tis 00:0c: 1.2 TPM (device-id 0xB, rev-id 16)
[15318.879154] tpm_tis 00:0c: Adjusting TPM timeout parameters.
# cat /sys/devices/pnp0/00\:0*/timeouts
1000000 2000000 150000000 [adjusted]

Looks more reasonable.

Stefan

Quoted 1 line

thanks,

#3 lore raw
Quoted 16 lines · Stefan Berger
Quoted 15 lines · Jiri Slaby
Quoted 14 lines · Stefan Berger

Below patch applies to the tip of the git tree.

v2:
- adjusting all timeouts for TPM devices reporting too low timeout
numbers
- also displaying in sysfs whether the timeouts are 'original' or
'adjusted'

The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.

Quoted 8 lines
Quoted 7 lines
Quoted 6 lines

Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.

I am also adding a sysfs entry 'timeouts' showing the timeouts that are being used.

It works:

'It' means also 'suspend' works?

Yes.

regards,

Signature · 1 line
js
author
#4 lore raw
Quoted 21 lines · Jiri Slaby
Quoted 20 lines · Stefan Berger
Quoted 19 lines · Jiri Slaby
Quoted 18 lines · Stefan Berger

Below patch applies to the tip of the git tree.

v2:
- adjusting all timeouts for TPM devices reporting too low timeout
numbers
- also displaying in sysfs whether the timeouts are 'original' or
'adjusted'

The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.

Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.

Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines

I am also adding a sysfs entry 'timeouts' showing the timeouts that are being used.

It works:

'It' means also 'suspend' works?

Yes.

That's great! Thanks for testing the patch!

Stefan

Quoted 1 line

regards,