[patch 0/8] tpm + tpm_tis : Various fixes

[patch 0/8] tpm + tpm_tis : Various fixes

[Stefan Berger]

The following is a series of patches that fix various issues in the general
TPM driver and in the TPM TIS driver. I had posted some of the patches
previously, but I am now reposting all of them again as part of this series.
Some previously posted patches have seen further changes (also to conform
to the coding style).

All patches apply to the current tip of the main git tree.

Some of the changes are related to using timeouts that the TPM reports, others
are related to fixing the interrupt mode in which the TPM TIS driver can be
run.

Another patch introduces the 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.

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?).

The patches apply in the order they appear.

  Regards,
         Stefan

[patch 1/8] tpm_tis: Use timeouts returned from TPM

[Stefan Berger]

[-- Attachment #1: tpm_driver_fix_timeout_usage.patch --]
[-- Type: text/plain, Size: 5157 bytes --]

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'

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 'durations' showing the timeouts that are
being used.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> 

---
 drivers/char/tpm/tpm.c     |   38 ++++++++++++++++++++++++++++++--------
 drivers/char/tpm/tpm.h     |    3 +++
 drivers/char/tpm/tpm_tis.c |    4 +++-
 3 files changed, 36 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.c
+++ linux-2.6/drivers/char/tpm/tpm.c
@@ -575,23 +575,31 @@ 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));
+	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;
+		chip->vendor.duration_adjusted = true;
+		dev_info(chip->dev, "Adjusting TPM timeout parameters.");
+	}
 }
 EXPORT_SYMBOL_GPL(tpm_get_timeouts);
 
@@ -937,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)
 {
Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.h
+++ 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;
Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ 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 = {

[patch 2/8] tpm_tis: Re-enable interrupts upon (S3) resume

[Stefan Berger]

[-- Attachment #1: tpm_irq_fix.patch --]
[-- Type: text/plain, Size: 2408 bytes --]

v2:
  - the patch was adapted to also work with a machine with a Intel iTPM

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 ]

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

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

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ linux-2.6/drivers/char/tpm/tpm_tis.c
@@ -647,11 +647,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);
@@ -704,6 +729,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 = {

[patch 3/8] tpm: Fix display of data in pubek sysfs entry

[Stefan Berger]

[-- Attachment #1: tpm_driver_fix_pubek.patch --]
[-- Type: text/plain, Size: 2586 bytes --]

This patch fixes the TPM's pubek sysfs entry that is accessible as long
as the TPM doesn't have an owner. I shifted 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 <stefanb@linux.vnet.ibm.com>

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

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

[patch 4/8] tpm_tis: Delay ACPI S3 suspend while TPM is busy

[Stefan Berger]

[-- Attachment #1: tpm_irq_refuse_susp_while_execute.patch --]
[-- Type: text/plain, Size: 3087 bytes --]

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 <stefanb@linux.vnet.ibm.com>

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

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ 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);

[patch 5/8] tpm_tis: Fix the probing for interrupts

[Stefan Berger]

[-- Attachment #1: tpm_fix_irq_probe.v4.patch --]
[-- Type: text/plain, Size: 4817 bytes --]

This patch fixes several aspects of the probing for interrupts.

I am reading 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.

I am introducing 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() I am clearing 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
one one machine I did testing.

Further, if a TPM claims to use a specifc interrupt, also make sure that that
interrupt is working by running the probing on it as well. 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 <stefanb@linux.vnet.ibm.com>

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

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ linux-2.6/drivers/char/tpm/tpm_tis.c
@@ -436,7 +436,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,
@@ -484,7 +484,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)))
@@ -542,6 +542,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);
@@ -560,13 +563,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) {
@@ -588,9 +597,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 +
@@ -629,7 +651,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;
Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.h
+++ linux-2.6/drivers/char/tpm/tpm.h
@@ -69,6 +69,7 @@ struct tpm_vendor_specific {
 	unsigned long base;		/* TPM base address */
 
 	int irq;
+	int probed_irq;
 
 	int region_size;
 	int have_region;

[patch 6/8] tpm + tpm_tis: Use interface timeouts returned from TPM

[Stefan Berger]

[-- Attachment #1: tpm_fix_iftimeouts.patch --]
[-- Type: text/plain, Size: 5267 bytes --]

This patch fixes the check for whether the 4 interface  timeouts (a,b,c,d)
reported by the TPM have been properly returned.

The current TPM TIS driver in git 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 timeout indicators of type u32.

I anticipate that some TPMs will also report these timeouts in msec rather than
usec and therefore scale them by a factor of 1000 if the timeout_a value is
found to be below 1000 (usecs).

I am also add a sysfs entry 'timeouts' that shows the 4 timeouts and reports
them either as being either 'original' or 'adjusted' if they were scaled.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

---
 drivers/char/tpm/tpm.c     |   36 +++++++++++++++++++++++++++++-------
 drivers/char/tpm/tpm.h     |    3 +++
 drivers/char/tpm/tpm_tis.c |    4 +++-
 3 files changed, 35 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.c
+++ 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;
@@ -545,24 +546,30 @@ 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 */
 	timeout = be32_to_cpu(timeout_cap->a);
+	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);
+		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;
@@ -965,6 +972,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)
 {
Index: linux-2.6/drivers/char/tpm/tpm.h
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.h
+++ 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;
 
@@ -84,6 +86,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;
 
Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ linux-2.6/drivers/char/tpm/tpm_tis.c
@@ -395,6 +395,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,
@@ -405,7 +406,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 = {

[patch 7/8] tpm_tis: Probing function for Intel iTPM bug

[Stefan Berger]

[-- Attachment #1: tis_probe_itpm.v2.patch --]
[-- Type: text/plain, Size: 4000 bytes --]

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

I split 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. I am using
that function 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 <stefanb@linux.vnet.ibm.com>

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

Index: linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
+++ 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");
 

[patch 8/8] tpm: Fix a typo

[Stefan Berger]

[-- Attachment #1: tpm_fix_typo.diff --]
[-- Type: text/plain, Size: 561 bytes --]

This patch fixes a typo.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Index: linux-2.6/drivers/char/tpm/tpm.c
===================================================================
--- linux-2.6.orig/drivers/char/tpm/tpm.c
+++ 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));

Re: [patch 1/8] tpm_tis: Use timeouts returned from TPM

[Rajiv Andrade]

Hi Stefan,

Some comments:

On 03/15/2011 08:13 AM, Stefan Berger wrote:
> 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

This is a bugfix that's different than the one you originally sent, can you
submit this as a separated patch? This is mainly for easier debug in the
future, so each feature/bugfix/commit can be tested separately.

> - also displaying in sysfs whether the timeouts are 'original' or 'adjusted'
> 

Minor comment, it's easier to read the log when the changelog comes after the main 
patch description.

> 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 'durations' showing the timeouts that are
> being used.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> 
> 
> ---
>  drivers/char/tpm/tpm.c     |   38 ++++++++++++++++++++++++++++++--------
>  drivers/char/tpm/tpm.h     |    3 +++
>  drivers/char/tpm/tpm_tis.c |    4 +++-
>  3 files changed, 36 insertions(+), 9 deletions(-)
> 
> Index: linux-2.6/drivers/char/tpm/tpm.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/tpm/tpm.c
> +++ linux-2.6/drivers/char/tpm/tpm.c
> @@ -575,23 +575,31 @@ 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));
> +	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;
> +		chip->vendor.duration_adjusted = true;
> +		dev_info(chip->dev, "Adjusting TPM timeout parameters.");
> +	}
>  }
>  EXPORT_SYMBOL_GPL(tpm_get_timeouts);
> 
> @@ -937,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)
>  {
> Index: linux-2.6/drivers/char/tpm/tpm.h
> ===================================================================
> --- linux-2.6.orig/drivers/char/tpm/tpm.h
> +++ 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;
> Index: linux-2.6/drivers/char/tpm/tpm_tis.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/tpm/tpm_tis.c
> +++ 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 = {
> 

Re: [patch 2/8] tpm_tis: Re-enable interrupts upon (S3) resume

[Rajiv Andrade]

On 03/15/2011 08:13 AM, Stefan Berger wrote:
> v2:
>   - the patch was adapted to also work with a machine with a Intel iTPM
> 

This is also a separated fix, which this time I can't also identify 
in the patch, can you submit it as another patch, that applies 
on top of the original 2/8?

Rajiv

Re: [patch 2/8] tpm_tis: Re-enable interrupts upon (S3) resume

[Stefan Berger]

On 03/29/2011 10:37 AM, Rajiv Andrade wrote:
> On 03/15/2011 08:13 AM, Stefan Berger wrote:
>> v2:
>>    - the patch was adapted to also work with a machine with a Intel iTPM
>>
> This is also a separated fix, which this time I can't also identify
> in the patch, can you submit it as another patch, that applies
> on top of the original 2/8?
There are two types of drivers in the tpm_tis driver, the pnp one and 
the 'platform' driver. The initial patch did not provide support for 
both of them, so maybe the comment above is misleading. I'll fix the 
comment, but don't think it makes sense tearing the patch into two pieces.

    Stefan

> Rajiv

Re: [patch 1/8] tpm_tis: Use timeouts returned from TPM

[Stefan Berger]

On 03/29/2011 10:34 AM, Rajiv Andrade wrote:
> Hi Stefan,
>
> Some comments:
>
> On 03/15/2011 08:13 AM, Stefan Berger wrote:
>> 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
> This is a bugfix that's different than the one you originally sent, can you
> submit this as a separated patch? This is mainly for easier debug in the
> future, so each feature/bugfix/commit can be tested separately.
Originally I sent a patch that corrected the evaluation of the return 
code within the TPM's response and introduced the sysfs entry. I did not 
scale the MEDIUM and LONG timeouts, which then caused the problems on 
the Infineon TPM and the patch was removed. I then introduced the 
scaling of the MEDIUM and LONG timeouts, which seems necessary as a 
consequence to working with the TPM-reported timeouts. Then adding a 
sysfs entry to it is an additional feature. I'd split this patch in 2 
patches where the 2nd one introduces the sysfs entry. Is this ok with you?

    Stefan