tpm + tpm_tis : Various fixes

PATCH In review Stefan Berger 2 participants 16 messages Mar 30, 2011 – Jun 13, 2011 TPM device driver

Share & tools

Stefan Berger

author

2011-03-30 16:14 UTC #0 lore raw

v2 cover tpm + tpm_tis : Various fixes

This patchset fixes various issues in the general TPM driver and the TPM TIS driver. Some of the patches were previously posted, and are being re-posted without change, while other patches were refactored. This patchset applies to the tip of the main git tree.

Patches 1-6 are related to using the timeouts that the TPM reports, along with work-arounds for TPMs that don't report the timeouts in microseconds but rather in milliseconds.

Patch 7 fixes the display of the public key of the TPM's Endorsement Key in sysfs.

Patch 8 re-enables the TPM's interrupts after resume from S3.

Patch 9 is useful if the TPM is run in interrupt mode and delays the S3 suspend until the TPM has finished processing a command.

Patch 10 fixes several aspects of the code around the probing for interrupts.

Patch 11 introduces automatic probing for the Intel iTPM flaw. The probing is useful in combination with the 'force' module parameter that enables the interrupt mode on some machines but then ends up circumventing ACPI to determine whether an Intel iTPM is present to activate the work-around. So, in that case we can rely on the probing.

Patch 12 fixes a typo.

I have tested these patches on four different types of machines. They worked fine there even though the TPM could not be used in interrupt mode on all of them (IRQ line not connected?).

V2:
- following Rajiv's feedback. Splitting the durations and timeout patches in 3 parts.
- slightly rearranging the order of the patches

Regards,
Stefan

Stefan Berger

author

2011-03-30 16:17 UTC #1 lore raw

PATCH v2 1/12 tpm: Use durations returned from TPM

The TPM driver currently discards the durations 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.

v4:
- sysfs entry 'durations' is now a patch of its own
- the work-around for TPMs reporting durations in milliseconds is now in a patch of its own

v3:
- sysfs entry now called 'durations' to resemble TPM-speak (previously was called 'timeouts')

v2:
- adjusting all timeouts for TPM devices reporting timeouts in msec rather than usec
- also displaying in sysfs whether the timeouts are 'original' or 'adjusted'

Signed-off-by Stefan Berger <[email protected]>

Tested-by Guillaume Chazarain <[email protected]>

---
 drivers/char/tpm/tpm.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/char/tpm/tpm.c +4 -2

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -575,9 +575,11 @@ 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));

Stefan Berger

author

2011-03-30 16:14 UTC #2 lore raw

PATCH v2 2/12 tpm: Adjust the durations if they are too small

In reply to Stefan Berger

Adjust the durations if they are found to be too small, i.e., if they are returned in milliseconds rather than microseconds as some Infineon TPMs are reported to do.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

drivers/char/tpm/tpm.c +11 -6

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -583,17 +583,22 @@ duration:
 	duration_cap = &tpm_cmd.params.getcap_out.cap.duration;
 	chip->vendor.duration[TPM_SHORT] =
 	    usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_short));
+	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.
+	 * We also scale the TPM_MEDIUM and -_LONG values by 1000.
 	 */
-	if (chip->vendor.duration[TPM_SHORT] < (HZ/100))
+	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));
+		chip->vendor.duration[TPM_MEDIUM] *= 1000;
+		chip->vendor.duration[TPM_LONG] *= 1000;
+		dev_info(chip->dev, "Adjusting TPM timeout parameters.");
+	}
 }
 EXPORT_SYMBOL_GPL(tpm_get_timeouts);

Stefan Berger

author

2011-03-30 16:14 UTC #3 lore raw

PATCH v2 3/12 tpm_tis: Introduce durations sysfs entry

In reply to Stefan Berger

Display the TPM's command timeouts in a 'durations' sysfs entry. Display the entries as having been adjusted when they were scaled due to their values being reported in milliseconds rather than microseconds.

Signed-off-by Stefan Berger <[email protected]>

Tested-by Guillaume Chazarain <[email protected]>

---
 drivers/char/tpm/tpm.c     |   15 +++++++++++++++
 drivers/char/tpm/tpm.h     |    3 +++
 drivers/char/tpm/tpm_tis.c |    4 +++-
 3 files changed, 21 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm.c +15 -0

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -597,6 +597,7 @@ duration:
 		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.");
 	}
 }

@@ -944,6 +945,20 @@ ssize_t tpm_show_caps_1_2(struct device 
 }
 EXPORT_SYMBOL_GPL(tpm_show_caps_1_2);
 
+ssize_t tpm_show_durations(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_durations);
+
 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_durations(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(durations, S_IRUGO, tpm_show_durations, 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_durations.attr, NULL,
 };
 
 static struct attribute_group tis_attr_grp = {

Stefan Berger

author

2011-03-30 16:14 UTC #4 lore raw

PATCH v2 4/12 tpm: Use interface timeouts returned from the TPM

In reply to Stefan Berger

The TPM driver currently discards the interface 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 + 4 interface timeout indicators of type u32.

Signed-off-by Stefan Berger <[email protected]>

drivers/char/tpm/tpm.c +4 -3

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -545,9 +545,10 @@ void tpm_get_timeouts(struct tpm_chip *c
 	if (rc)
 		goto duration;
 
-	if (be32_to_cpu(tpm_cmd.header.out.length)
-	    != 4 * sizeof(u32))
-		goto duration;
+	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) + 4 * sizeof(u32))
+		return;
 
 	timeout_cap = &tpm_cmd.params.getcap_out.cap.timeout;
 	/* Don't overwrite default if value is 0 */

Stefan Berger

author

2011-03-30 16:17 UTC #5 lore raw

PATCH v2 5/12 tpm: Adjust interface timeouts if they are too small

In reply to Stefan Berger

Adjust the interface timeouts if they are found to be too small, i.e., if they are returned in milliseconds rather than microseconds as we heared from Infineon that some (old) Infineon TPMs do.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/char/tpm/tpm.c +9 -4

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -534,6 +534,7 @@ void tpm_get_timeouts(struct tpm_chip *c
 	struct duration_t *duration_cap;
 	ssize_t rc;
 	u32 timeout;
+	unsigned int scale = 1;
 
 	tpm_cmd.header.in = tpm_getcap_header;
 	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;

@@ -553,17 +554,21 @@ void tpm_get_timeouts(struct tpm_chip *c
 	timeout_cap = &tpm_cmd.params.getcap_out.cap.timeout;
 	/* Don't overwrite default if value is 0 */
 	timeout = be32_to_cpu(timeout_cap->a);
+	if (timeout && timeout < 1000) {
+		/* timeouts in msec rather usec */
+		scale = 1000;
+	}
 	if (timeout)
-		chip->vendor.timeout_a = usecs_to_jiffies(timeout);
+		chip->vendor.timeout_a = usecs_to_jiffies(timeout * scale);
 	timeout = be32_to_cpu(timeout_cap->b);
 	if (timeout)
-		chip->vendor.timeout_b = usecs_to_jiffies(timeout);
+		chip->vendor.timeout_b = usecs_to_jiffies(timeout * scale);
 	timeout = be32_to_cpu(timeout_cap->c);
 	if (timeout)
-		chip->vendor.timeout_c = usecs_to_jiffies(timeout);
+		chip->vendor.timeout_c = usecs_to_jiffies(timeout * scale);
 	timeout = be32_to_cpu(timeout_cap->d);
 	if (timeout)
-		chip->vendor.timeout_d = usecs_to_jiffies(timeout);
+		chip->vendor.timeout_d = usecs_to_jiffies(timeout * scale);
 
 duration:
 	tpm_cmd.header.in = tpm_getcap_header;

Stefan Berger

author

2011-03-30 16:17 UTC #6 lore raw

PATCH v2 6/12 tpm_tis: Add timeouts sysfs entry

In reply to Stefan Berger

Display the TPM's interface timeouts in a 'timeouts' sysfs entry. Display the entries as having been adjusted when they were scaled due to their values being reported in milliseconds rather than microseconds.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm.c     |   16 ++++++++++++++++
 drivers/char/tpm/tpm.h     |    3 +++
 drivers/char/tpm/tpm_tis.c |    4 +++-
 3 files changed, 22 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm.h +3 -0

Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
@@ -58,6 +58,8 @@ extern ssize_t tpm_show_temp_deactivated
 					 struct device_attribute *attr, char *);
 extern ssize_t tpm_show_durations(struct device *,
 				  struct device_attribute *attr, char *);
+extern ssize_t tpm_show_timeouts(struct device *,
+				 struct device_attribute *attr, char *);
 
 struct tpm_chip;

@@ -83,6 +85,7 @@ struct tpm_vendor_specific {
 	struct list_head list;
 	int locality;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* jiffies */
+	bool timeout_adjusted;
 	unsigned long duration[3]; /* jiffies */
 	bool duration_adjusted;

drivers/char/tpm/tpm_tis.c +3 -1

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -377,6 +377,7 @@ static DEVICE_ATTR(temp_deactivated, S_I
 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(durations, S_IRUGO, tpm_show_durations, NULL);
+static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
 
 static struct attribute *tis_attrs[] = {
 	&dev_attr_pubek.attr,

@@ -387,7 +388,8 @@ static struct attribute *tis_attrs[] = {
 	&dev_attr_temp_deactivated.attr,
 	&dev_attr_caps.attr,
 	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr, NULL,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr, NULL,
 };
 
 static struct attribute_group tis_attr_grp = {

drivers/char/tpm/tpm.c +16 -0

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -557,6 +557,7 @@ void tpm_get_timeouts(struct tpm_chip *c
 	if (timeout && timeout < 1000) {
 		/* timeouts in msec rather usec */
 		scale = 1000;
+		chip->vendor.timeout_adjusted = true;
 	}
 	if (timeout)
 		chip->vendor.timeout_a = usecs_to_jiffies(timeout * scale);

@@ -965,6 +966,21 @@ ssize_t tpm_show_durations(struct device
 }
 EXPORT_SYMBOL_GPL(tpm_show_durations);
 
+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 %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.timeout_a),
+		       jiffies_to_usecs(chip->vendor.timeout_b),
+		       jiffies_to_usecs(chip->vendor.timeout_c),
+		       jiffies_to_usecs(chip->vendor.timeout_d),
+		       chip->vendor.timeout_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)
 {

Stefan Berger

author

2011-03-30 16:14 UTC #7 lore raw

PATCH v2 7/12 tpm: Fix display of data in pubek sysfs entry

In reply to Stefan Berger

This patch fixes the TPM's pubek sysfs entry that is accessible as long as the TPM doesn't have an owner. It was necessary to shift the access to the data by -10 -- the first byte immediately follows the 10 byte header. The line

data = tpm_cmd.params.readpubek_out_buffer;

sets it at the offset '10' in the packet, so we can read the data array starting at offset '0'.

Before:

Algorithm: 00 0C 00 00
Encscheme: 08 00
Sigscheme: 00 00
Parameters: 00 00 00 00 01 00 AC E2 5E 3C A0 78
Modulus length: -563306801
Modulus:
28 21 08 0F 82 CD F2 B1 E7 49 F7 74 70 BE 59 8C
43 78 B1 24 EA 52 E2 FE 52 5C 3A 12 3B DC 61 71
[...]

After:

Algorithm: 00 00 00 01
Encscheme: 00 03
Sigscheme: 00 01
Parameters: 00 00 08 00 00 00 00 02 00 00 00 00
Modulus length: 256
Modulus:
AC E2 5E 3C A0 78 DE 6C 9E CF 28 21 08 0F 82 CD
F2 B1 E7 49 F7 74 70 BE 59 8C 43 78 B1 24 EA 52
[...]

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm.c |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

drivers/char/tpm/tpm.c +16 ~~-10~~

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -878,18 +878,24 @@ ssize_t tpm_show_pubek(struct device *de
 	data = tpm_cmd.params.readpubek_out_buffer;
 	str +=
 	    sprintf(str,
-		    "Algorithm: %02X %02X %02X %02X\nEncscheme: %02X %02X\n"
-		    "Sigscheme: %02X %02X\nParameters: %02X %02X %02X %02X"
-		    " %02X %02X %02X %02X %02X %02X %02X %02X\n"
-		    "Modulus length: %d\nModulus: \n",
-		    data[10], data[11], data[12], data[13], data[14],
-		    data[15], data[16], data[17], data[22], data[23],
-		    data[24], data[25], data[26], data[27], data[28],
-		    data[29], data[30], data[31], data[32], data[33],
-		    be32_to_cpu(*((__be32 *) (data + 34))));
+		    "Algorithm: %02X %02X %02X %02X\n"
+		    "Encscheme: %02X %02X\n"
+		    "Sigscheme: %02X %02X\n"
+		    "Parameters: %02X %02X %02X %02X "
+		    "%02X %02X %02X %02X "
+		    "%02X %02X %02X %02X\n"
+		    "Modulus length: %d\n"
+		    "Modulus:\n",
+		    data[0], data[1], data[2], data[3],
+		    data[4], data[5],
+		    data[6], data[7],
+		    data[12], data[13], data[14], data[15],
+		    data[16], data[17], data[18], data[19],
+		    data[20], data[21], data[22], data[23],
+		    be32_to_cpu(*((__be32 *) (data + 24))));
 
 	for (i = 0; i < 256; i++) {
-		str += sprintf(str, "%02X ", data[i + 38]);
+		str += sprintf(str, "%02X ", data[i + 28]);
 		if ((i + 1) % 16 == 0)
 			str += sprintf(str, "\n");
 	}

Stefan Berger

author

2011-03-30 16:16 UTC #8 lore raw

PATCH v2 8/12 tpm_tis: Re-enable interrupts upon (S3) resume

In reply to Stefan Berger

This patch makes sure that if the TPM TIS interface is run in interrupt mode (rather than polling mode) that all interrupts are enabled in the TPM's interrupt enable register after a resume from ACPI S3 suspend. The registers may either have been cleared by the TPM loosing its state during device sleep or by the BIOS leaving the TPM in polling mode (after sending a command to the TPM for starting it up again)

You may want to check if your TPM runs with interrupts by doing

cat /proc/interrupts | grep -i tpm

and see whether there is an entry or otherwise for it to use interrupts:

modprobe tpm_tis interrupts=1 [add 'itpm=1' for Intel TPM ]

v2:
- the patch was adapted to work with the pnp and platform driver implementations in tpm_tis.c

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm_tis.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

drivers/char/tpm/tpm_tis.c +30 -0

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -649,11 +649,36 @@ static int tpm_tis_pnp_suspend(struct pn
 	return tpm_pm_suspend(&dev->dev, msg);
 }
 
+static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
+{
+	u32 intmask;
+
+	/* reenable interrupts that device may have lost or
+	   BIOS/firmware may have disabled */
+	iowrite8(chip->vendor.irq, chip->vendor.iobase +
+		 TPM_INT_VECTOR(chip->vendor.locality));
+
+	intmask =
+	    ioread32(chip->vendor.iobase +
+		     TPM_INT_ENABLE(chip->vendor.locality));
+
+	intmask |= TPM_INTF_CMD_READY_INT
+	    | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
+	    | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
+
+	iowrite32(intmask,
+		  chip->vendor.iobase + TPM_INT_ENABLE(chip->vendor.locality));
+}
+
+
 static int tpm_tis_pnp_resume(struct pnp_dev *dev)
 {
 	struct tpm_chip *chip = pnp_get_drvdata(dev);
 	int ret;
 
+	if (chip->vendor.irq)
+		tpm_tis_reenable_interrupts(chip);
+
 	ret = tpm_pm_resume(&dev->dev);
 	if (!ret)
 		tpm_continue_selftest(chip);

@@ -706,6 +731,11 @@ static int tpm_tis_suspend(struct platfo
 
 static int tpm_tis_resume(struct platform_device *dev)
 {
+	struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
+
+	if (chip->vendor.irq)
+		tpm_tis_reenable_interrupts(chip);
+
 	return tpm_pm_resume(&dev->dev);
 }
 static struct platform_driver tis_drv = {

Stefan Berger

author

2011-03-30 16:16 UTC #9 lore raw

PATCH v2 9/12 tpm_tis: Delay ACPI S3 suspend while the TPM is busy

In reply to Stefan Berger

This patch delays the (ACPI S3) suspend while the TPM is busy processing a command and the TPM TIS driver is run in interrupt mode. This is the same behavior as we already have it for the TPM TIS driver in polling mode.

Reasoning: Some of the TPM's commands advance the internal state of the TPM. An example would be the extending of one of its PCR registers. Upper layers, such as IMA or TSS (TrouSerS), would certainly want to be sure that the command succeeded rather than getting an error code (-62 = -ETIME) that may not give a conclusive answer as for what reason the command failed. Reissuing such a command would put the TPM into the wrong state, so waiting for it to finish is really the only option.

The downside is that some commands (key creation) can take a long time and actually prevent the machine from entering S3 at all before the 20 second timeout of the power management subsystem arrives.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm_tis.c |   28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

drivers/char/tpm/tpm_tis.c +23 -5

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -26,6 +26,7 @@
 #include <linux/interrupt.h>
 #include <linux/wait.h>
 #include <linux/acpi.h>
+#include <linux/freezer.h>
 #include "tpm.h"
 
 #define TPM_HEADER_SIZE 10

@@ -120,7 +121,7 @@ static void release_locality(struct tpm_
 
 static int request_locality(struct tpm_chip *chip, int l)
 {
-	unsigned long stop;
+	unsigned long stop, timeout;
 	long rc;
 
 	if (check_locality(chip, l) >= 0)

@@ -129,17 +130,25 @@ static int request_locality(struct tpm_c
 	iowrite8(TPM_ACCESS_REQUEST_USE,
 		 chip->vendor.iobase + TPM_ACCESS(l));
 
+	stop = jiffies + chip->vendor.timeout_a;
+
 	if (chip->vendor.irq) {
+again:
+		timeout = stop - jiffies;
+		if ((long)timeout <= 0)
+			return -1;
 		rc = wait_event_interruptible_timeout(chip->vendor.int_queue,
 						      (check_locality
 						       (chip, l) >= 0),
-						      chip->vendor.timeout_a);
+						      timeout);
 		if (rc > 0)
 			return l;
-
+		if (rc == -ERESTARTSYS && freezing(current)) {
+			clear_thread_flag(TIF_SIGPENDING);
+			goto again;
+		}
 	} else {
 		/* wait for burstcount */
-		stop = jiffies + chip->vendor.timeout_a;
 		do {
 			if (check_locality(chip, l) >= 0)
 				return l;

@@ -196,15 +205,24 @@ static int wait_for_stat(struct tpm_chip
 	if ((status & mask) == mask)
 		return 0;
 
+	stop = jiffies + timeout;
+
 	if (chip->vendor.irq) {
+again:
+		timeout = stop - jiffies;
+		if ((long)timeout <= 0)
+			return -ETIME;
 		rc = wait_event_interruptible_timeout(*queue,
 						      ((tpm_tis_status
 							(chip) & mask) ==
 						       mask), timeout);
 		if (rc > 0)
 			return 0;
+		if (rc == -ERESTARTSYS && freezing(current)) {
+			clear_thread_flag(TIF_SIGPENDING);
+			goto again;
+		}
 	} else {
-		stop = jiffies + timeout;
 		do {
 			msleep(TPM_TIMEOUT);
 			status = tpm_tis_status(chip);

Stefan Berger

author

2011-03-30 16:15 UTC #10 lore raw

PATCH v2 10/12 tpm_tis: Fix the probing for interrupts

In reply to Stefan Berger

This patch fixes several aspects of the probing for interrupts.

This patch reads the TPM's timeouts before probing for the interrupts. The tpm_get_timeouts() function is invoked in polling mode and gets the proper timeouts from the TPM so that we don't need to fall back to 2 minutes timeouts for short duration commands while the interrupt probing is happening.

This patch introduces a variable probed_irq into the vendor structure that gets the irq number if an interrupt is received while the the tpm_gen_interrupt() function is run in polling mode during interrupt probing. Previously some parts of tpm_gen_interrupt() were run in polling mode, then the irq variable was set in the interrupt handler when an interrupt was received and execution of tpm_gen_interrupt() ended up switching over to interrupt mode. tpm_gen_interrupt() execution ended up on an event queue where it eventually timed out since the probing handler doesn't wake any queues.

Before calling into free_irq() clear all interrupt flags that may have been set by the TPM. The reason is that free_irq() will call into the probing interrupt handler and may otherwise fool us into thinking that a real interrupt happened (because we see the flags as being set) while the TPM's interrupt line is not even connected to anything on the motherboard. This solves a problem on one machine I did testing on (Thinkpad T60).

If a TPM claims to use a specifc interrupt, the probing is done as well to verify that the interrupt is actually working. If a TPM indicates that it does not use a specific interrupt (returns '0'), probe all interrupts from 3 to 15.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm.h     |    1 +
 drivers/char/tpm/tpm_tis.c |   33 +++++++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 6 deletions(-)

drivers/char/tpm/tpm_tis.c +27 -6

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -438,7 +438,7 @@ static irqreturn_t tis_int_probe(int irq
 	if (interrupt == 0)
 		return IRQ_NONE;
 
-	chip->vendor.irq = irq;
+	chip->vendor.probed_irq = irq;
 
 	/* Clear interrupts handled with TPM_EOI */
 	iowrite32(interrupt,

@@ -486,7 +486,7 @@ static int tpm_tis_init(struct device *d
 			resource_size_t len, unsigned int irq)
 {
 	u32 vendor, intfcaps, intmask;
-	int rc, i;
+	int rc, i, irq_s, irq_e;
 	struct tpm_chip *chip;
 
 	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))

@@ -544,6 +544,9 @@ static int tpm_tis_init(struct device *d
 	if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
 		dev_dbg(dev, "\tData Avail Int Support\n");
 
+	/* get the timeouts before testing for irqs */
+	tpm_get_timeouts(chip);
+
 	/* INTERRUPT Setup */
 	init_waitqueue_head(&chip->vendor.read_queue);
 	init_waitqueue_head(&chip->vendor.int_queue);

@@ -562,13 +565,19 @@ static int tpm_tis_init(struct device *d
 	if (interrupts)
 		chip->vendor.irq = irq;
 	if (interrupts && !chip->vendor.irq) {
-		chip->vendor.irq =
+		irq_s =
 		    ioread8(chip->vendor.iobase +
 			    TPM_INT_VECTOR(chip->vendor.locality));
+		if (irq_s) {
+			irq_e = irq_s;
+		} else {
+			irq_s = 3;
+			irq_e = 15;
+		}
 
-		for (i = 3; i < 16 && chip->vendor.irq == 0; i++) {
+		for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) {
 			iowrite8(i, chip->vendor.iobase +
-				    TPM_INT_VECTOR(chip->vendor.locality));
+				 TPM_INT_VECTOR(chip->vendor.locality));
 			if (request_irq
 			    (i, tis_int_probe, IRQF_SHARED,
 			     chip->vendor.miscdev.name, chip) != 0) {

@@ -590,9 +599,22 @@ static int tpm_tis_init(struct device *d
 				  chip->vendor.iobase +
 				  TPM_INT_ENABLE(chip->vendor.locality));
 
+			chip->vendor.probed_irq = 0;
+
 			/* Generate Interrupts */
 			tpm_gen_interrupt(chip);
 
+			chip->vendor.irq = chip->vendor.probed_irq;
+
+			/* free_irq will call into tis_int_probe;
+			   clear all irqs we haven't seen while doing
+			   tpm_gen_interrupt */
+			iowrite32(ioread32
+				  (chip->vendor.iobase +
+				   TPM_INT_STATUS(chip->vendor.locality)),
+				  chip->vendor.iobase +
+				  TPM_INT_STATUS(chip->vendor.locality));
+
 			/* Turn off */
 			iowrite32(intmask,
 				  chip->vendor.iobase +

@@ -631,7 +653,6 @@ static int tpm_tis_init(struct device *d
 	list_add(&chip->vendor.list, &tis_chips);
 	spin_unlock(&tis_lock);
 
-	tpm_get_timeouts(chip);
 	tpm_continue_selftest(chip);
 
 	return 0;

drivers/char/tpm/tpm.h +1 -0

Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
@@ -71,6 +71,7 @@ struct tpm_vendor_specific {
 	unsigned long base;		/* TPM base address */
 
 	int irq;
+	int probed_irq;
 
 	int region_size;
 	int have_region;

Stefan Berger

author

2011-03-30 16:14 UTC #11 lore raw

PATCH v2 11/12 tpm_tis: Probing function for Intel iTPM bug

In reply to Stefan Berger

This patch introduces a function for automatic probing for the Intel iTPM STS_DATA_EXPECT flaw.

The patch splits the current tpm_tis_send function into 2 parts where the 1st part is now called tpm_tis_send_data() and merely sends the data to the TPM. This function is then used for probing. The new tpm_tis_send function now first calls tpm_tis_send_data and if that succeeds has the TPM process the command and waits until the response is there.

The probing for the Intel iTPM is only invoked if the user has not passed itpm=1 as parameter for the module *or* if such a TPM was detected via ACPI. Previously it was necessary to pass itpm=1 when also passing force=1 to the module when doing a 'modprobe'. This function is more general than the ACPI test function and the function relying on ACPI could probably be removed.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm_tis.c |   77 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 4 deletions(-)

drivers/char/tpm/tpm_tis.c +73 -4

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
@@ -306,11 +306,10 @@ MODULE_PARM_DESC(itpm, "Force iTPM worka
  * tpm.c can skip polling for the data to be available as the interrupt is
  * waited for here
  */
-static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
+static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	int rc, status, burstcnt;
 	size_t count = 0;
-	u32 ordinal;
 
 	if (request_locality(chip, 0) < 0)
 		return -EBUSY;

@@ -345,8 +344,7 @@ static int tpm_tis_send(struct tpm_chip 
 
 	/* write last byte */
 	iowrite8(buf[count],
-		 chip->vendor.iobase +
-		 TPM_DATA_FIFO(chip->vendor.locality));
+		 chip->vendor.iobase + TPM_DATA_FIFO(chip->vendor.locality));
 	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c,
 		      &chip->vendor.int_queue);
 	status = tpm_tis_status(chip);

@@ -355,6 +353,28 @@ static int tpm_tis_send(struct tpm_chip 
 		goto out_err;
 	}
 
+	return 0;
+
+out_err:
+	tpm_tis_ready(chip);
+	release_locality(chip, chip->vendor.locality, 0);
+	return rc;
+}
+
+/*
+ * If interrupts are used (signaled by an irq set in the vendor structure)
+ * tpm.c can skip polling for the data to be available as the interrupt is
+ * waited for here
+ */
+static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	int rc;
+	u32 ordinal;
+
+	rc = tpm_tis_send_data(chip, buf, len);
+	if (rc < 0)
+		return rc;
+
 	/* go and do it */
 	iowrite8(TPM_STS_GO,
 		 chip->vendor.iobase + TPM_STS(chip->vendor.locality));

@@ -376,6 +396,47 @@ out_err:
 	return rc;
 }
 
+/*
+ * Early probing for iTPM with STS_DATA_EXPECT flaw.
+ * Try sending command without itpm flag set and if that
+ * fails, repeat with itpm flag set.
+ */
+static int probe_itpm(struct tpm_chip *chip)
+{
+	int rc = 0;
+	u8 cmd_getticks[] = {
+		0x00, 0xc1, 0x00, 0x00, 0x00, 0x0a,
+		0x00, 0x00, 0x00, 0xf1
+	};
+	size_t len = sizeof(cmd_getticks);
+	int rem_itpm = itpm;
+
+	itpm = 0;
+
+	rc = tpm_tis_send_data(chip, cmd_getticks, len);
+	if (rc == 0)
+		goto out;
+
+	tpm_tis_ready(chip);
+	release_locality(chip, chip->vendor.locality, 0);
+
+	itpm = 1;
+
+	rc = tpm_tis_send_data(chip, cmd_getticks, len);
+	if (rc == 0) {
+		dev_info(chip->dev, "Detected an iTPM.\n");
+		rc = 1;
+	} else
+		rc = -EFAULT;
+
+out:
+	itpm = rem_itpm;
+	tpm_tis_ready(chip);
+	release_locality(chip, chip->vendor.locality, 0);
+
+	return rc;
+}
+
 static const struct file_operations tis_ops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,

@@ -515,6 +576,14 @@ static int tpm_tis_init(struct device *d
 		 "1.2 TPM (device-id 0x%X, rev-id %d)\n",
 		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));
 
+	if (!itpm) {
+		itpm = probe_itpm(chip);
+		if (itpm < 0) {
+			rc = -ENODEV;
+			goto out_err;
+		}
+	}
+
 	if (itpm)
 		dev_info(dev, "Intel iTPM workaround enabled\n");

Stefan Berger

author

2011-03-30 16:16 UTC #12 lore raw

PATCH v2 12/12 tpm: Fix a typo

In reply to Stefan Berger

This patch fixes a typo.

Signed-off-by Stefan Berger <[email protected]>

drivers/char/tpm/tpm.c +1 -1

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
@@ -615,7 +615,7 @@ void tpm_continue_selftest(struct tpm_ch
 	u8 data[] = {
 		0, 193,			/* TPM_TAG_RQU_COMMAND */
 		0, 0, 0, 10,		/* length */
-		0, 0, 0, 83,		/* TPM_ORD_GetCapability */
+		0, 0, 0, 83,		/* TPM_ORD_ContinueSelfTest */
 	};
 
 	tpm_transmit(chip, data, sizeof(data));

Stefan Berger

author

2011-05-27 20:56 UTC #13 lore raw

In reply to Stefan Berger

Rajiv,

are you planning to use these patches?

Regards,
Stefan

Quoted 43 lines · Stefan Berger

This patchset fixes various issues in the general TPM driver and the TPM TIS driver. Some of the patches were previously posted, and are being re-posted without change, while other patches were refactored. This patchset applies to the tip of the main git tree.
Patches 1-6 are related to using the timeouts that the TPM reports, along with work-arounds for TPMs that don't report the timeouts in microseconds but rather in milliseconds.
Patch 7 fixes the display of the public key of the TPM's Endorsement Key in sysfs.
Patch 8 re-enables the TPM's interrupts after resume from S3.
Patch 9 is useful if the TPM is run in interrupt mode and delays the S3 suspend until the TPM has finished processing a command.
Patch 10 fixes several aspects of the code around the probing for interrupts.
Patch 11 introduces automatic probing for the Intel iTPM flaw. The probing is useful in combination with the 'force' module parameter that enables the interrupt mode on some machines but then ends up circumventing ACPI to determine whether an Intel iTPM is present to activate the work-around. So, in that case we can rely on the probing.
Patch 12 fixes a typo.
I have tested these patches on four different types of machines. They worked fine there even though the TPM could not be used in interrupt mode on all of them (IRQ line not connected?).
V2:
- following Rajiv's feedback. Splitting the durations and timeout patches in 3 parts.
- slightly rearranging the order of the patches
Regards,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Rajiv Andrade

2011-05-31 13:24 UTC #14 lore raw

Hi Stefan,

Yes, I'll have them reviewed again and in a git pull request real soon.

Thanks, Rajiv Andrade
Security Development
IBM Linux Technology Center

Quoted 53 lines · Stefan Berger

Rajiv,
are you planning to use these patches?
Regards,
Stefan
Quoted 43 lines · Stefan Berger
This patchset fixes various issues in the general TPM driver and the TPM TIS driver. Some of the patches were previously posted, and are being re-posted without change, while other patches were refactored. This patchset applies to the tip of the main git tree.
Patches 1-6 are related to using the timeouts that the TPM reports, along with work-arounds for TPMs that don't report the timeouts in microseconds but rather in milliseconds.
Patch 7 fixes the display of the public key of the TPM's Endorsement Key in sysfs.
Patch 8 re-enables the TPM's interrupts after resume from S3.
Patch 9 is useful if the TPM is run in interrupt mode and delays the S3 suspend until the TPM has finished processing a command.
Patch 10 fixes several aspects of the code around the probing for interrupts.
Patch 11 introduces automatic probing for the Intel iTPM flaw. The probing is useful in combination with the 'force' module parameter that enables the interrupt mode on some machines but then ends up circumventing ACPI to determine whether an Intel iTPM is present to activate the work-around. So, in that case we can rely on the probing.
Patch 12 fixes a typo.
I have tested these patches on four different types of machines. They worked fine there even though the TPM could not be used in interrupt mode on all of them (IRQ line not connected?).
V2:
- following Rajiv's feedback. Splitting the durations and timeout patches in 3 parts.
- slightly rearranging the order of the patches
Regards,
Stefan
Signature · 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Stefan Berger

author

2011-06-13 15:34 UTC #15 lore raw

Quoted 3 lines · Rajiv Andrade

Hi Stefan,
Yes, I'll have them reviewed again and in a git pull request real soon.

Has the review-oracle spoken in the meantime ? What did it say?

Stefan

Quoted 62 lines

Thanks, Rajiv Andrade
Security Development
IBM Linux Technology Center
Quoted 52 lines · Stefan Berger
Rajiv,
are you planning to use these patches?
Regards,
Stefan
Quoted 43 lines · Stefan Berger
This patchset fixes various issues in the general TPM driver and the TPM TIS driver. Some of the patches were previously posted, and are being re-posted without change, while other patches were refactored. This patchset applies to the tip of the main git tree.
Patches 1-6 are related to using the timeouts that the TPM reports, along with work-arounds for TPMs that don't report the timeouts in microseconds but rather in milliseconds.
Patch 7 fixes the display of the public key of the TPM's Endorsement Key in sysfs.
Patch 8 re-enables the TPM's interrupts after resume from S3.
Patch 9 is useful if the TPM is run in interrupt mode and delays the S3 suspend until the TPM has finished processing a command.
Patch 10 fixes several aspects of the code around the probing for interrupts.
Patch 11 introduces automatic probing for the Intel iTPM flaw. The probing is useful in combination with the 'force' module parameter that enables the interrupt mode on some machines but then ends up circumventing ACPI to determine whether an Intel iTPM is present to activate the work-around. So, in that case we can rely on the probing.
Patch 12 fixes a typo.
I have tested these patches on four different types of machines. They worked fine there even though the TPM could not be used in interrupt mode on all of them (IRQ line not connected?).
V2:
- following Rajiv's feedback. Splitting the durations and timeout patches in 3 parts.
- slightly rearranging the order of the patches
Regards,
Stefan
Signature · 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/